vips_cache() + sequential seems to be broken in 8.15

[jcupitt]

With this test program:

#!/usr/bin/env python3

import sys
import pyvips

tile_size = 512

for filename in sys.argv[1:]:
    image = pyvips.Image.new_from_file(filename, access="sequential")
    image = image.cache()
    tiles = [image.crop(x * tile_size, y * tile_size, tile_size, tile_size)
             for y in range(0, image.height // tile_size)
             for x in range(0, image.width // tile_size)]
    averages = [tile.avg() for tile in tiles]

    print(f"cut {filename} into {len(tiles)} tiles")
    print(f"tile averages = {averages}")

And 8.15 master I see:

$ ./try346.py ~/pics/k2.jpg ~/pics/wtc.jpg
cut /home/john/pics/k2.jpg into 8 tiles
tile averages = [53.93897374471029, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0, 0.0]
cut /home/john/pics/wtc.jpg into 324 tiles
tile averages = [2.8246485392252603, 0.0, 0.0, ...

Which is obviously not correct! Remove the access="sequential" and it starts working:

$ ./try346.py ~/pics/k2.jpg ~/pics/wtc.jpg
cut /home/john/pics/k2.jpg into 8 tiles
tile averages = [53.93897374471029, 68.08555348714192, 84.35875574747722, 148.9688924153646, 94.37240091959636, 189.3188451131185, 75.03674570719402, 116.65000788370769]
cut /home/john/pics/wtc.jpg into 324 tiles
tile averages = [2.8246485392252603, 47.18035761515299, 83.10053380330403, 65.48446400960286, 72.40385055541992, ...

Perhaps the jpeg loader is not flagging out of order correctly?

[kleisauke]

This doesn't seem to be a regression; I can reproduce this with libvips 8.2.x (the minimum libvips requirement of pyvips).

Note that the error in tile-warnings are swallowed by default in pyvips, you can make these appear with:

@@ -3,6 +3,9 @@
 import sys
 import pyvips
 
+import logging
+logging.basicConfig(level=logging.WARNING)
+
 tile_size = 512
 
 for filename in sys.argv[1:]:

libvips/libvips/conversion/tilecache.c

Lines 714 to 715 in 2be083a

	g_warning(_("error in tile %d x %d"),
	tile->pos.left, tile->pos.top);

[jcupitt]

Sorry, I should have updated this issue, I've been looking at it.

The problem is that cache is based on sinkscreen, and that swallows errors (you don't want screen repainting to stop on a dead tile). We should probably deprecate cache, ignoring errors makes it worse than useless for non-interactive work.

I might have found some improvements to tilecache, still poking at it ...