fix tile recomp issue

splitting request from collect stops recomp cancelling itself all the
time

Author: jcupitt

TODO

@@ -1,17 +1,8 @@
-- nip4 ~/pics/a1.bmp, free rotate, open imagewindow, drag slider
+- investigate tile recomp for test.ws, are we optimal now?
 
-	updates often hitch badly, what's it doing?
 
-	perhaps it's not discarding out of date updates quickly enough?
 
-	test nip2 on the same task
-
-		always seems to be making useful pixels
-
-  in nip4, drag alisder a lot, end on -61, open image view window, never
-  updates
-
-	you need to press 1, wait for update, then press 0 to see anything
+- very long formula (eg. Group [G1, G2, etc]) are not truncated
 
 - check all batch mode flags
 

src/tilecache.c

@@ -94,7 +94,7 @@ tilecache_dispose(GObject *object)
 
 	FREESID(tilecache->tilesource_changed_sid, tilecache->tilesource);
 	FREESID(tilecache->tilesource_tiles_changed_sid, tilecache->tilesource);
-	FREESID(tilecache->tilesource_area_changed_sid, tilecache->tilesource);
+	FREESID(tilecache->tilesource_collect_sid, tilecache->tilesource);
 	VIPS_UNREF(tilecache->tilesource);
 	VIPS_UNREF(tilecache->background_texture);
 
@@ -334,11 +334,11 @@ tilecache_find(Tilecache *tilecache, VipsRect *tile_rect, int z)
 	return NULL;
 }
 
-/* Fetch a single tile. If we have this tile already, refresh if there are new
+/* Request a single tile. If we have this tile already, refresh if there are new
  * pixels available.
  */
 static void
-tilecache_get(Tilecache *tilecache, VipsRect *tile_rect, int z)
+tilecache_request(Tilecache *tilecache, VipsRect *tile_rect, int z)
 {
 	Tile *tile;
 
@@ -356,29 +356,30 @@ tilecache_get(Tilecache *tilecache, VipsRect *tile_rect, int z)
 	}
 
 	if (!tile->valid) {
-		/* The tile might have no pixels, or might need refreshing
-		 * because the bg render has finished with it.
-		 */
 #ifdef DEBUG_VERBOSE
-		printf("tilecache_get: fetching left = %d, top = %d, "
+		printf("tilecache_request: fetching left = %d, top = %d, "
 			   "width = %d, height = %d, z = %d\n",
 			tile_rect->left, tile_rect->top,
 			tile_rect->width, tile_rect->height,
 			z);
 #endif /*DEBUG_VERBOSE*/
 
-		tilesource_fill_tile(tilecache->tilesource, tile);
+		tilesource_request_tile(tilecache->tilesource, tile);
 	}
 }
 
-/* Fetch the tiles in an area.
+/* Request tiles from an area. If they are not in cache, they will be computed
+ * in the bg and delivered via _collect().
  *
  * render processes tiles in FIFO order, so we need to add in reverse order
  * of processing. We want repaint to happen in a spiral from the centre out,
  * so we have to add in a spiral from the outside in.
+ *
+ * We must be careful not to change tilesource if we have all these tiles
+ * already (very common for thumbnails, for example).
  */
 static void
-tilecache_fetch_area(Tilecache *tilecache, VipsRect *viewport, int z)
+tilecache_request_area(Tilecache *tilecache, VipsRect *viewport, int z)
 {
 	int size0 = TILE_SIZE << z;
 
@@ -412,7 +413,7 @@ tilecache_fetch_area(Tilecache *tilecache, VipsRect *viewport, int z)
 		for (x = left; x < right; x += size0) {
 			tile_rect.left = x;
 			tile_rect.top = top;
-			tilecache_get(tilecache, &tile_rect, z);
+			tilecache_request(tilecache, &tile_rect, z);
 		}
 
 		top += size0;
@@ -425,7 +426,7 @@ tilecache_fetch_area(Tilecache *tilecache, VipsRect *viewport, int z)
 		for (x = left; x < right; x += size0) {
 			tile_rect.left = x;
 			tile_rect.top = bottom - size0;
-			tilecache_get(tilecache, &tile_rect, z);
+			tilecache_request(tilecache, &tile_rect, z);
 		}
 
 		bottom -= size0;
@@ -438,7 +439,7 @@ tilecache_fetch_area(Tilecache *tilecache, VipsRect *viewport, int z)
 		for (y = top; y < bottom; y += size0) {
 			tile_rect.left = left;
 			tile_rect.top = y;
-			tilecache_get(tilecache, &tile_rect, z);
+			tilecache_request(tilecache, &tile_rect, z);
 		}
 
 		left += size0;
@@ -451,7 +452,7 @@ tilecache_fetch_area(Tilecache *tilecache, VipsRect *viewport, int z)
 		for (y = top; y < bottom; y += size0) {
 			tile_rect.left = right - size0;
 			tile_rect.top = y;
-			tilecache_get(tilecache, &tile_rect, z);
+			tilecache_request(tilecache, &tile_rect, z);
 		}
 
 		right -= size0;
@@ -461,33 +462,35 @@ tilecache_fetch_area(Tilecache *tilecache, VipsRect *viewport, int z)
 	}
 }
 
-/* The bg render thread says some tiles have fresh pixels.
+/* A new tile is available from the bg render and must be collected.
  */
 static void
-tilecache_source_area_changed(Tilesource *tilesource,
+tilecache_source_collect(Tilesource *tilesource,
 	VipsRect *dirty, int z, Tilecache *tilecache)
 {
 #ifdef DEBUG_VERBOSE
-	printf("tilecache_source_area_changed: left = %d, top = %d, "
+	printf("tilecache_source_collect: left = %d, top = %d, "
 		   "width = %d, height = %d, z = %d\n",
 		dirty->left, dirty->top,
 		dirty->width, dirty->height, z);
 #endif /*DEBUG_VERBOSE*/
 
-	/* Immediately fetch the updated tile. If we wait for snapshot, the
-	 * animation page may have changed.
-	 */
-	tilecache_fetch_area(tilecache, dirty, z);
+	Tile *tile = tilecache_find(tilecache, dirty, z);
+	if (tile &&
+		!tile->valid) {
+		tilesource_collect_tile(tilecache->tilesource, tile);
 
-	tilecache_area_changed(tilecache, dirty, z);
+		// things displaying us will need to redraw
+		tilecache_area_changed(tilecache, dirty, z);
+	}
 }
 
 static void
 tilecache_set_tilesource(Tilecache *tilecache, Tilesource *tilesource)
 {
 	FREESID(tilecache->tilesource_changed_sid, tilecache->tilesource);
 	FREESID(tilecache->tilesource_tiles_changed_sid, tilecache->tilesource);
-	FREESID(tilecache->tilesource_area_changed_sid, tilecache->tilesource);
+	FREESID(tilecache->tilesource_collect_sid, tilecache->tilesource);
 	VIPS_UNREF(tilecache->tilesource);
 
 	tilecache->tilesource = tilesource;
@@ -503,9 +506,9 @@ tilecache_set_tilesource(Tilecache *tilecache, Tilesource *tilesource)
 		tilecache->tilesource_tiles_changed_sid =
 			g_signal_connect(tilesource, "tiles-changed",
 				G_CALLBACK(tilecache_source_tiles_changed), tilecache);
-		tilecache->tilesource_area_changed_sid =
-			g_signal_connect(tilesource, "area-changed",
-				G_CALLBACK(tilecache_source_area_changed), tilecache);
+		tilecache->tilesource_collect_sid =
+			g_signal_connect(tilesource, "collect",
+				G_CALLBACK(tilecache_source_collect), tilecache);
 
 		/* Any tiles must be invalidated.
 		 */
@@ -946,7 +949,7 @@ tilecache_snapshot(Tilecache *tilecache, GtkSnapshot *snapshot,
 	/* Fetch any tiles we are missing, update any tiles we have that have
 	 * been flagged as having pixels ready for fetching.
 	 */
-	tilecache_fetch_area(tilecache, &viewport, z);
+	tilecache_request_area(tilecache, &viewport, z);
 
 	/* Find the set of visible tiles, sorted back to front.
 	 *

src/tilecache.h

@@ -87,11 +87,11 @@ typedef struct _Tilecache {
 	 */
 	GdkTexture *background_texture;
 
-	/* The signals we watcgh tilesource with.
+	/* The signals we watch tilesource with.
 	 */
 	guint tilesource_changed_sid;
 	guint tilesource_tiles_changed_sid;
-	guint tilesource_area_changed_sid;
+	guint tilesource_collect_sid;
 
 } Tilecache;
 

src/tilesource.c

@@ -62,7 +62,7 @@ enum {
 	SIG_POSTEVAL,
 	SIG_CHANGED,
 	SIG_TILES_CHANGED,
-	SIG_AREA_CHANGED,
+	SIG_COLLECT,
 	SIG_PAGE_CHANGED,
 
 	SIG_LAST
@@ -111,10 +111,9 @@ tilesource_tiles_changed(Tilesource *tilesource)
 }
 
 static void
-tilesource_area_changed(Tilesource *tilesource, VipsRect *dirty, int z)
+tilesource_collect(Tilesource *tilesource, VipsRect *dirty, int z)
 {
-	g_signal_emit(tilesource,
-		tilesource_signals[SIG_AREA_CHANGED], 0, dirty, z);
+	g_signal_emit(tilesource, tilesource_signals[SIG_COLLECT], 0, dirty, z);
 }
 
 static void
@@ -230,7 +229,7 @@ tilesource_render_notify_idle(void *user_data)
 	 * pipeline.
 	 */
 	if (update->image == tilesource->display)
-		tilesource_area_changed(tilesource, &update->rect, update->z);
+		tilesource_collect(tilesource, &update->rect, update->z);
 
 	/* The update that's just for this one event needs freeing.
 	 */
@@ -253,9 +252,13 @@ tilesource_render_notify(VipsImage *image, VipsRect *rect, void *client)
 	 */
 	TilesourceUpdate *new_update = g_new(TilesourceUpdate, 1);
 
+	/* From image cods to level0 cods.
+	 */
 	*new_update = *update;
-	new_update->rect = *rect;
-	new_update->z = update->z;
+	new_update->rect.left = rect->left << update->z;
+	new_update->rect.top = rect->top << update->z;
+	new_update->rect.width = rect->width << update->z;
+	new_update->rect.height = rect->height << update->z;
 
 	g_idle_add(tilesource_render_notify_idle, new_update);
 }
@@ -1237,10 +1240,10 @@ tilesource_class_init(TilesourceClass *class)
 		g_cclosure_marshal_VOID__VOID,
 		G_TYPE_NONE, 0);
 
-	tilesource_signals[SIG_AREA_CHANGED] = g_signal_new("area-changed",
+	tilesource_signals[SIG_COLLECT] = g_signal_new("collect",
 		G_TYPE_FROM_CLASS(class),
 		G_SIGNAL_RUN_LAST,
-		G_STRUCT_OFFSET(TilesourceClass, area_changed),
+		G_STRUCT_OFFSET(TilesourceClass, collect),
 		NULL, NULL,
 		nip4_VOID__POINTER_INT,
 		G_TYPE_NONE, 2,
@@ -1821,11 +1824,15 @@ tilesource_background_load(Tilesource *tilesource)
 		tilesource, NULL);
 }
 
+/* Request a tile from the pipeline. The tile might be already there (in
+ * cache), and we are all done, or it might need to be computed and collected
+ * later.
+ */
 int
-tilesource_fill_tile(Tilesource *tilesource, Tile *tile)
+tilesource_request_tile(Tilesource *tilesource, Tile *tile)
 {
 #ifdef DEBUG_VERBOSE
-	printf("tilesource_fill_tile: %d x %d\n",
+	printf("tilesource_request_tile: %d x %d\n",
 		tile->region->valid.left, tile->region->valid.top);
 #endif /*DEBUG_VERBOSE*/
 
@@ -1850,8 +1857,10 @@ tilesource_fill_tile(Tilesource *tilesource, Tile *tile)
 	printf("  valid = %d\n", tile->valid);
 #endif /*DEBUG_VERBOSE*/
 
-	/* We must always prepare the region, even if we know it's blank,
-	 * since this will trigger the background render.
+	/* If the tile is not in cache, we must fetch to trigger a bg recomp.
+	 *
+	 * If it is in cache, we also fetch the pixels and set a texture ready
+	 * for the cache.
 	 */
 	if (vips_region_prepare_to(tilesource->rgb_region, tile->region,
 			&tile->region->valid,
@@ -1870,6 +1879,46 @@ tilesource_fill_tile(Tilesource *tilesource, Tile *tile)
 	return 0;
 }
 
+/* Try to collect a computed tile.
+ */
+int
+tilesource_collect_tile(Tilesource *tilesource, Tile *tile)
+{
+#ifdef DEBUG_VERBOSE
+	printf("tilesource_collect_tile: %d x %d\n",
+		tile->region->valid.left, tile->region->valid.top);
+#endif /*DEBUG_VERBOSE*/
+
+	if (vips_region_prepare(tilesource->mask_region, &tile->region->valid))
+		return -1;
+
+	/* tile is within a single tile, so we only need to test the first byte
+	 * of the mask.
+	 */
+	tile->valid = VIPS_REGION_ADDR(tilesource->mask_region,
+		tile->region->valid.left, tile->region->valid.top)[0];
+
+#ifdef DEBUG_VERBOSE
+	printf("  valid = %d\n", tile->valid);
+#endif /*DEBUG_VERBOSE*/
+
+	/* Only read out the tile if it's valid. We don't want to trigger another
+	 * compute.
+	 */
+	if (tile->valid) {
+		if (vips_region_prepare_to(tilesource->rgb_region, tile->region,
+				&tile->region->valid,
+				tile->region->valid.left,
+				tile->region->valid.top))
+			return -1;
+
+		tile_free_texture(tile);
+		tile_get_texture(tile);
+	}
+
+	return 0;
+}
+
 const char *
 tilesource_get_path(Tilesource *tilesource)
 {

src/tilesource.h

@@ -258,10 +258,10 @@ typedef struct _TilesourceClass {
 	 */
 	void (*tiles_changed)(Tilesource *tilesource);
 
-	/* A set of tiles on a certain level have new pixels now that a
-	 * background render has completed.
+	/* A tile has been computed by a bg worker and must be collected from the
+	 * end of the pipeline.
 	 */
-	void (*area_changed)(Tilesource *tilesource, VipsRect *area, int z);
+	void (*collect)(Tilesource *tilesource, VipsRect *area, int z);
 
 	/* The page has changed. Just for updating the page number display.
 	 */
@@ -280,7 +280,8 @@ gboolean tilesource_has_imageinfo(Tilesource *tilesource, Imageinfo *ii);
 
 void tilesource_background_load(Tilesource *tilesource);
 
-int tilesource_fill_tile(Tilesource *tilesource, Tile *tile);
+int tilesource_request_tile(Tilesource *tilesource, Tile *tile);
+int tilesource_collect_tile(Tilesource *tilesource, Tile *tile);
 
 const char *tilesource_get_path(Tilesource *tilesource);
 GFile *tilesource_get_file(Tilesource *tilesource);