GNOME.org
 

[grits] Move threading out of tile update/gc functions

commit c07abad157dd10d3f8cc229b2f616c3a33f6c900
Author: Andy Spencer <andy753421 gmail com>
Date:   Thu Dec 27 08:45:40 2012 +0000

    Move threading out of tile update/gc functions
    
    This prevents all sorts of race conditions when unloading plugins.
    
    I hope.

 src/grits-opengl.c       |    2 +
 src/objects/grits-tile.c |   50 +++++++++++++++++++++++----------------------
 src/plugins/elev.c       |   39 +++++++++++++++++++----------------
 src/plugins/map.c        |   44 ++++++++++++++++++++++------------------
 src/plugins/sat.c        |   44 ++++++++++++++++++++++------------------
 5 files changed, 97 insertions(+), 82 deletions(-)
---
diff --git a/src/grits-opengl.c b/src/grits-opengl.c
index a17ddda..90e830b 100644
--- a/src/grits-opengl.c
+++ b/src/grits-opengl.c
@@ -371,8 +371,10 @@ static gboolean on_expose(GritsOpenGL *opengl, GdkEventExpose *event, gpointer _
 	glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
 
 #ifndef ROAM_DEBUG
+	g_mutex_lock(&opengl->sphere_lock);
 	roam_sphere_update_errors(opengl->sphere);
 	roam_sphere_split_merge(opengl->sphere);
+	g_mutex_unlock(&opengl->sphere_lock);
 #endif
 
 #ifdef ROAM_DEBUG
diff --git a/src/objects/grits-tile.c b/src/objects/grits-tile.c
index b903492..e42a160 100644
--- a/src/objects/grits-tile.c
+++ b/src/objects/grits-tile.c
@@ -224,7 +224,7 @@ void grits_tile_update(GritsTile *tile, GritsPoint *eye,
 	}
 
 	/* Load the tile */
-	if (!tile->load && !tile->pixels && !tile->tex)
+	if (!tile->load && !tile->data && !tile->tex && !tile->pixels && !tile->pixbuf)
 		load_func(tile, user_data);
 	tile->atime = time(NULL);
 	tile->load  = TRUE;
@@ -299,7 +299,6 @@ gboolean grits_tile_load_pixbuf(GritsTile *tile, GdkPixbuf *pixbuf)
 	tile->width  = gdk_pixbuf_get_width(pixbuf);
 	tile->height = gdk_pixbuf_get_height(pixbuf);
 	tile->alpha  = gdk_pixbuf_get_has_alpha(pixbuf);
-	tile->pixels = gdk_pixbuf_get_pixels(pixbuf);
 
 	/* Queue OpenGL texture load/draw */
 	grits_object_queue_draw(GRITS_OBJECT(tile));
@@ -321,17 +320,13 @@ gboolean grits_tile_load_file(GritsTile *tile, const gchar *file)
 {
 	g_debug("GritsTile: load_file %p -> %s", tile, file);
 
-	/* Load pixbuf */
-	GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(file, NULL);
-	if (!pixbuf)
-		return FALSE;
-
 	/* Copy pixbuf data for callback */
-	tile->pixbuf = pixbuf;
-	tile->width  = gdk_pixbuf_get_width(pixbuf);
-	tile->height = gdk_pixbuf_get_height(pixbuf);
-	tile->alpha  = gdk_pixbuf_get_has_alpha(pixbuf);
-	tile->pixels = gdk_pixbuf_get_pixels(pixbuf);
+	tile->pixbuf = gdk_pixbuf_new_from_file(file, NULL);
+	if (!tile->pixbuf)
+		return FALSE;
+	tile->width  = gdk_pixbuf_get_width(tile->pixbuf);
+	tile->height = gdk_pixbuf_get_height(tile->pixbuf);
+	tile->alpha  = gdk_pixbuf_get_has_alpha(tile->pixbuf);
 
 	/* Queue OpenGL texture load/draw */
 	grits_object_queue_draw(GRITS_OBJECT(tile));
@@ -407,19 +402,20 @@ GritsTile *grits_tile_gc(GritsTile *root, time_t atime,
 	}
 	//g_debug("GritsTile: gc - %p kids=%d time=%d data=%d load=%d",
 	//	root, !!has_children, root->atime < atime, !!root->data, !!root->load);
-	if (root->parent && !has_children && root->atime < atime &&
-			(root->data || !root->load)) {
+	int thread_safe = !root->load || root->data || root->tex || root->pixels || root->pixbuf;
+	if (root->parent && !has_children && root->atime < atime && thread_safe) {
 		//g_debug("GritsTile: gc/free - %p", root);
-		if (root->tex) {
+		if (root->pixbuf)
+			g_object_unref(root->pixbuf);
+		if (root->pixels)
+			g_free(root->pixels);
+		if (root->tex)
 			glDeleteTextures(1, &root->tex);
-			root->tex = 0;
-		}
 		if (root->data) {
 			if (free_func)
 				free_func(root, user_data);
 			else
 				g_free(root->data);
-			root->data = NULL;
 		}
 		g_object_unref(root);
 		return NULL;
@@ -486,9 +482,13 @@ static gboolean _grits_tile_load_tex(GritsTile *tile)
 		return TRUE;
 
 	/* Check if the tile has data yet */
-	if (!tile->pixels)
+	if (!tile->pixels && !tile->pixbuf)
 		return FALSE;
 
+	/* Get correct pixel buffer */
+	guchar *pixels = tile->pixels ?:
+		gdk_pixbuf_get_pixels(tile->pixbuf);
+
 	/* Create texture */
 	g_debug("GritsTile: load_tex");
 	glGenTextures(1, &tile->tex);
@@ -497,19 +497,21 @@ static gboolean _grits_tile_load_tex(GritsTile *tile)
 	glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
 	glPixelStorei(GL_PACK_ALIGNMENT, 1);
 	glTexImage2D(GL_TEXTURE_2D, 0, 4, tile->width, tile->height, 0,
-			(tile->alpha ? GL_RGBA : GL_RGB), GL_UNSIGNED_BYTE, tile->pixels);
+			(tile->alpha ? GL_RGBA : GL_RGB), GL_UNSIGNED_BYTE, pixels);
 	glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
 	glTexParameterf(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
 	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
 
 	/* Free data */
-	if (tile->pixbuf)
+	if (tile->pixbuf) {
 		g_object_unref(tile->pixbuf);
-	else
+		tile->pixbuf = NULL;
+	}
+	if (tile->pixels) {
 		g_free(tile->pixels);
-	tile->pixbuf = NULL;
-	tile->pixels = NULL;
+		tile->pixels = NULL;
+	}
 
 	return TRUE;
 
diff --git a/src/plugins/elev.c b/src/plugins/elev.c
index 3f7b2a1..3fbeb87 100644
--- a/src/plugins/elev.c
+++ b/src/plugins/elev.c
@@ -133,13 +133,15 @@ static guchar *_load_pixels(guint16 *bil)
 	return (guchar*)pixels;
 }
 
-static void _load_tile(GritsTile *tile, gpointer _elev)
+static void _load_tile_thread(gpointer _tile, gpointer _elev)
 {
+	GritsTile       *tile = _tile;
 	GritsPluginElev *elev = _elev;
 
-	g_debug("GritsPluginElev: _load_tile start %p", g_thread_self());
+	g_debug("GritsPluginElev: _load_tile_thread start %p - tile=%p",
+			g_thread_self(), tile);
 	if (elev->aborted) {
-		g_debug("GritsPluginElev: _load_tile - aborted");
+		g_debug("GritsPluginElev: _load_tile_thread - aborted");
 		return;
 	}
 
@@ -173,19 +175,14 @@ static void _load_tile(GritsTile *tile, gpointer _elev)
 		g_free(bil);
 
 	/* Load the GL texture from the main thread */
-	g_debug("GritsPluginElev: _load_tile end %p", g_thread_self());
+	g_debug("GritsPluginElev: _load_tile_thread end %p", g_thread_self());
 }
 
-static void _update_tiles(gpointer _, gpointer _elev)
+static void _load_tile_func(GritsTile *tile, gpointer _elev)
 {
-	g_debug("GritsPluginElev: _update_tiles");
+	g_debug("GritsPluginElev: _load_tile_func - tile=%p", tile);
 	GritsPluginElev *elev = _elev;
-	GritsPoint eye;
-	grits_viewer_get_location(elev->viewer, &eye.lat, &eye.lon, &eye.elev);
-	grits_tile_update(elev->tiles, &eye,
-			MAX_RESOLUTION, TILE_WIDTH, TILE_WIDTH,
-			_load_tile, elev);
-	grits_tile_gc(elev->tiles, time(NULL)-10, NULL, elev);
+	g_thread_pool_push(elev->threads, tile, NULL);
 }
 
 /*************
@@ -194,7 +191,11 @@ static void _update_tiles(gpointer _, gpointer _elev)
 static void _on_location_changed(GritsViewer *viewer,
 		gdouble lat, gdouble lon, gdouble elevation, GritsPluginElev *elev)
 {
-	g_thread_pool_push(elev->threads, NULL+1, NULL);
+	GritsPoint eye = {lat, lon, elevation};
+	grits_tile_update(elev->tiles, &eye,
+			MAX_RESOLUTION, TILE_WIDTH, TILE_WIDTH,
+			_load_tile_func, elev);
+	grits_tile_gc(elev->tiles, time(NULL)-10, NULL, elev);
 }
 
 /***********
@@ -215,7 +216,9 @@ GritsPluginElev *grits_plugin_elev_new(GritsViewer *viewer)
 	elev->viewer = g_object_ref(viewer);
 
 	/* Load initial tiles */
-	_update_tiles(NULL, elev);
+	gdouble lat, lon, elevation;
+	grits_viewer_get_location(viewer, &lat, &lon, &elevation);
+	_on_location_changed(viewer, lat, lon, elevation, elev);
 
 	/* Connect signals */
 	elev->sigid = g_signal_connect(elev->viewer, "location-changed",
@@ -247,7 +250,7 @@ static void grits_plugin_elev_init(GritsPluginElev *elev)
 {
 	g_debug("GritsPluginElev: init");
 	/* Set defaults */
-	elev->threads = g_thread_pool_new(_update_tiles, elev, 1, FALSE, NULL);
+	elev->threads = g_thread_pool_new(_load_tile_thread, elev, 1, FALSE, NULL);
 	elev->tiles = grits_tile_new(NULL, NORTH, SOUTH, EAST, WEST);
 	elev->wms   = grits_wms_new(
 		"http://www.nasa.network.com/elev";, "mergedSrtm", "application/bil",
@@ -262,15 +265,15 @@ static void grits_plugin_elev_dispose(GObject *gobject)
 	/* Drop references */
 	if (elev->viewer) {
 		GritsViewer *viewer = elev->viewer;
-		elev->viewer = NULL;
 		g_signal_handler_disconnect(viewer, elev->sigid);
+		soup_session_abort(elev->wms->http->soup);
+		g_thread_pool_free(elev->threads, TRUE, TRUE);
+		elev->viewer = NULL;
 		if (LOAD_BIL)
 			grits_viewer_clear_height_func(viewer);
 		if (LOAD_TEX)
 			grits_viewer_remove(viewer, GRITS_OBJECT(elev->tiles));
 		g_object_unref(elev->tiles);
-		soup_session_abort(elev->wms->http->soup);
-		g_thread_pool_free(elev->threads, TRUE, TRUE);
 		g_object_unref(viewer);
 	}
 	G_OBJECT_CLASS(grits_plugin_elev_parent_class)->dispose(gobject);
diff --git a/src/plugins/map.c b/src/plugins/map.c
index a573dcb..255f67c 100644
--- a/src/plugins/map.c
+++ b/src/plugins/map.c
@@ -46,12 +46,15 @@ static const guchar colormap[][2][4] = {
 	{{0xff, 0xe1, 0x80}, {0xff, 0xe1, 0x80, 0x60}}, // Cities
 };
 
-static void _load_tile(GritsTile *tile, gpointer _map)
+static void _load_tile_thread(gpointer _tile, gpointer _map)
 {
-	GritsPluginMap *map = _map;
-	g_debug("GritsPluginMap: _load_tile start %p", g_thread_self());
+	GritsTile      *tile = _tile;
+	GritsPluginMap *map  = _map;
+
+	g_debug("GritsPluginMap: _load_tile_thread start %p - tile=%p",
+			g_thread_self(), tile);
 	if (map->aborted) {
-		g_debug("GritsPluginMap: _load_tile - aborted");
+		g_debug("GritsPluginMap: _load_tile_thread - aborted");
 		return;
 	}
 
@@ -63,7 +66,7 @@ static void _load_tile(GritsTile *tile, gpointer _map)
 	/* Load pixbuf */
 	GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(path, NULL);
 	if (!pixbuf) {
-		g_warning("GritsPluginMap: _load_tile - Error loading pixbuf %s", path);
+		g_warning("GritsPluginMap: _load_tile_thread - Error loading pixbuf %s", path);
 		g_remove(path);
 		g_free(path);
 		return;
@@ -92,19 +95,14 @@ static void _load_tile(GritsTile *tile, gpointer _map)
 
 	/* Load the GL texture from the main thread */
 	grits_tile_load_pixbuf(tile, pixbuf);
-	g_debug("GritsPluginMap: _load_tile end %p", g_thread_self());
+	g_debug("GritsPluginMap: _load_tile_thread end %p", g_thread_self());
 }
 
-static void _update_tiles(gpointer _, gpointer _map)
+static void _load_tile_func(GritsTile *tile, gpointer _map)
 {
-	g_debug("GritsPluginMap: _update_tiles");
+	g_debug("GritsPluginMap: _load_tile_func - tile=%p", tile);
 	GritsPluginMap *map = _map;
-	GritsPoint eye;
-	grits_viewer_get_location(map->viewer, &eye.lat, &eye.lon, &eye.elev);
-	grits_tile_update(map->tiles, &eye,
-			MAX_RESOLUTION, TILE_WIDTH, TILE_WIDTH,
-			_load_tile, map);
-	grits_tile_gc(map->tiles, time(NULL)-10, NULL, map);
+	g_thread_pool_push(map->threads, tile, NULL);
 }
 
 /*************
@@ -113,7 +111,11 @@ static void _update_tiles(gpointer _, gpointer _map)
 static void _on_location_changed(GritsViewer *viewer,
 		gdouble lat, gdouble lon, gdouble elev, GritsPluginMap *map)
 {
-	g_thread_pool_push(map->threads, NULL+1, NULL);
+	GritsPoint eye = {lat, lon, elev};
+	grits_tile_update(map->tiles, &eye,
+			MAX_RESOLUTION, TILE_WIDTH, TILE_WIDTH,
+			_load_tile_func, map);
+	grits_tile_gc(map->tiles, time(NULL)-10, NULL, map);
 }
 
 /***********
@@ -134,7 +136,9 @@ GritsPluginMap *grits_plugin_map_new(GritsViewer *viewer)
 	map->viewer = g_object_ref(viewer);
 
 	/* Load initial tiles */
-	_update_tiles(NULL, map);
+	gdouble lat, lon, elev;
+	grits_viewer_get_location(viewer, &lat, &lon, &elev);
+	_on_location_changed(viewer, lat, lon, elev, map);
 
 	/* Connect signals */
 	map->sigid = g_signal_connect(map->viewer, "location-changed",
@@ -165,7 +169,7 @@ static void grits_plugin_map_init(GritsPluginMap *map)
 {
 	g_debug("GritsPluginMap: init");
 	/* Set defaults */
-	map->threads = g_thread_pool_new(_update_tiles, map, 1, FALSE, NULL);
+	map->threads = g_thread_pool_new(_load_tile_thread, map, 1, FALSE, NULL);
 	map->tiles = grits_tile_new(NULL, 85.0511, -85.0511, EAST, WEST);
 	map->tms   = grits_tms_new("http://tile.openstreetmap.org";,
 		"osmtile/", "png");
@@ -185,13 +189,13 @@ static void grits_plugin_map_dispose(GObject *gobject)
 	/* Drop references */
 	if (map->viewer) {
 		GritsViewer *viewer = map->viewer;
-		map->viewer = NULL;
 		g_signal_handler_disconnect(viewer, map->sigid);
-		grits_viewer_remove(viewer, GRITS_OBJECT(map->tiles));
-		g_object_unref(map->tiles);
 		soup_session_abort(map->tms->http->soup);
 		//soup_session_abort(map->wms->http->soup);
 		g_thread_pool_free(map->threads, TRUE, TRUE);
+		map->viewer = NULL;
+		grits_viewer_remove(viewer, GRITS_OBJECT(map->tiles));
+		g_object_unref(map->tiles);
 		g_object_unref(viewer);
 	}
 	G_OBJECT_CLASS(grits_plugin_map_parent_class)->dispose(gobject);
diff --git a/src/plugins/sat.c b/src/plugins/sat.c
index b43e4fa..5389ddb 100644
--- a/src/plugins/sat.c
+++ b/src/plugins/sat.c
@@ -35,12 +35,15 @@
 #define TILE_WIDTH     1024
 #define TILE_HEIGHT    512
 
-static void _load_tile(GritsTile *tile, gpointer _sat)
+static void _load_tile_thread(gpointer _tile, gpointer _sat)
 {
-	GritsPluginSat *sat = _sat;
-	g_debug("GritsPluginSat: _load_tile start %p", g_thread_self());
+	GritsTile      *tile = _tile;
+	GritsPluginSat *sat  = _sat;
+
+	g_debug("GritsPluginSat: _load_tile_thread start %p - tile=%p",
+			g_thread_self(), tile);
 	if (sat->aborted) {
-		g_debug("GritsPluginSat: _load_tile - aborted");
+		g_debug("GritsPluginSat: _load_tile_thread - aborted");
 		return;
 	}
 
@@ -51,7 +54,7 @@ static void _load_tile(GritsTile *tile, gpointer _sat)
 	/* Load pixbuf */
 	GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(path, NULL);
 	if (!pixbuf) {
-		g_warning("GritsPluginSat: _load_tile - Error loading pixbuf %s", path);
+		g_warning("GritsPluginSat: _load_tile_thread - Error loading pixbuf %s", path);
 		g_remove(path);
 		g_free(path);
 		return;
@@ -77,19 +80,14 @@ static void _load_tile(GritsTile *tile, gpointer _sat)
 
 	/* Load the GL texture from the main thread */
 	grits_tile_load_pixbuf(tile, pixbuf);
-	g_debug("GritsPluginSat: _load_tile end %p", g_thread_self());
+	g_debug("GritsPluginSat: _load_tile_thread end %p", g_thread_self());
 }
 
-static void _update_tiles(gpointer _, gpointer _sat)
+static void _load_tile_func(GritsTile *tile, gpointer _sat)
 {
-	g_debug("GritsPluginSat: _update_tiles");
+	g_debug("GritsPluginSat: __load_tile_func - tile=%p", tile);
 	GritsPluginSat *sat = _sat;
-	GritsPoint eye;
-	grits_viewer_get_location(sat->viewer, &eye.lat, &eye.lon, &eye.elev);
-	grits_tile_update(sat->tiles, &eye,
-			MAX_RESOLUTION, TILE_WIDTH, TILE_WIDTH,
-			_load_tile, sat);
-	grits_tile_gc(sat->tiles, time(NULL)-10, NULL, sat);
+	g_thread_pool_push(sat->threads, tile, NULL);
 }
 
 /*************
@@ -98,7 +96,11 @@ static void _update_tiles(gpointer _, gpointer _sat)
 static void _on_location_changed(GritsViewer *viewer,
 		gdouble lat, gdouble lon, gdouble elev, GritsPluginSat *sat)
 {
-	g_thread_pool_push(sat->threads, NULL+1, NULL);
+	GritsPoint eye = {lat, lon, elev};
+	grits_tile_update(sat->tiles, &eye,
+			MAX_RESOLUTION, TILE_WIDTH, TILE_WIDTH,
+			_load_tile_func, sat);
+	grits_tile_gc(sat->tiles, time(NULL)-10, NULL, sat);
 }
 
 /***********
@@ -119,7 +121,9 @@ GritsPluginSat *grits_plugin_sat_new(GritsViewer *viewer)
 	sat->viewer = g_object_ref(viewer);
 
 	/* Load initial tiles */
-	_update_tiles(NULL, sat);
+	gdouble lat, lon, elev;
+	grits_viewer_get_location(viewer, &lat, &lon, &elev);
+	_on_location_changed(viewer, lat, lon, elev, sat);
 
 	/* Connect signals */
 	sat->sigid = g_signal_connect(sat->viewer, "location-changed",
@@ -150,7 +154,7 @@ static void grits_plugin_sat_init(GritsPluginSat *sat)
 {
 	g_debug("GritsPluginSat: init");
 	/* Set defaults */
-	sat->threads = g_thread_pool_new(_update_tiles, sat, 1, FALSE, NULL);
+	sat->threads = g_thread_pool_new(_load_tile_thread, sat, 1, FALSE, NULL);
 	sat->tiles = grits_tile_new(NULL, NORTH, SOUTH, EAST, WEST);
 	sat->wms   = grits_wms_new(
 		"http://www.nasa.network.com/wms";, "bmng200406", "image/jpeg",
@@ -165,12 +169,12 @@ static void grits_plugin_sat_dispose(GObject *gobject)
 	/* Drop references */
 	if (sat->viewer) {
 		GritsViewer *viewer = sat->viewer;
-		sat->viewer = NULL;
 		g_signal_handler_disconnect(viewer, sat->sigid);
-		grits_viewer_remove(viewer, GRITS_OBJECT(sat->tiles));
-		g_object_unref(sat->tiles);
 		soup_session_abort(sat->wms->http->soup);
 		g_thread_pool_free(sat->threads, TRUE, TRUE);
+		sat->viewer = NULL;
+		grits_viewer_remove(viewer, GRITS_OBJECT(sat->tiles));
+		g_object_unref(sat->tiles);
 		g_object_unref(viewer);
 	}
 	G_OBJECT_CLASS(grits_plugin_sat_parent_class)->dispose(gobject);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]