From 35eef7db3248843c17de23c5636ff9c8610ea191 Mon Sep 17 00:00:00 2001 From: Andy Spencer Date: Mon, 9 Nov 2009 13:40:49 +0000 Subject: [PATCH] Fix UI hangs with parallel processing and callbacks * Multi-threading * Thread protection * Priorities on callbacks --- src/gis-opengl.c | 19 +++++++++++-- src/gis_test.c | 7 ++++- src/plugins/bmng.c | 53 ++++++++++++++++++++++++++-------- src/plugins/bmng.h | 1 + src/plugins/srtm.c | 71 +++++++++++++++++++++++++++++++++------------- src/plugins/srtm.h | 1 + 6 files changed, 118 insertions(+), 34 deletions(-) diff --git a/src/gis-opengl.c b/src/gis-opengl.c index d1c845f..86fba80 100644 --- a/src/gis-opengl.c +++ b/src/gis-opengl.c @@ -195,6 +195,7 @@ static gboolean on_key_press(GisOpenGL *self, GdkEventKey *event, gpointer _) gis_view_get_location(self->view, &lat, &lon, &elev); pan = MIN(elev/(EARTH_R/2), 30); guint kv = event->keyval; + gdk_threads_leave(); if (kv == GDK_Left || kv == GDK_h) gis_view_pan(self->view, 0, -pan, 0); else if (kv == GDK_Down || kv == GDK_j) gis_view_pan(self->view, -pan, 0, 0); else if (kv == GDK_Up || kv == GDK_k) gis_view_pan(self->view, pan, 0, 0); @@ -213,32 +214,45 @@ static gboolean on_key_press(GisOpenGL *self, GdkEventKey *event, gpointer _) else if (kv == GDK_p) roam_sphere_merge_one(self->sphere); else if (kv == GDK_r) roam_sphere_split_merge(self->sphere); else if (kv == GDK_u) roam_sphere_update_errors(self->sphere); + gdk_threads_enter(); gtk_widget_queue_draw(GTK_WIDGET(self)); +#else + gdk_threads_enter(); #endif return TRUE; } +static gboolean _update_errors_cb(gpointer sphere) +{ + roam_sphere_update_errors(sphere); + return FALSE; +} static void on_view_changed(GisView *view, gdouble _1, gdouble _2, gdouble _3, GisOpenGL *self) { g_debug("GisOpenGL: on_view_changed"); + gdk_threads_enter(); gis_opengl_begin(self); set_visuals(self); #ifndef ROAM_DEBUG - roam_sphere_update_errors(self->sphere); + g_idle_add_full(G_PRIORITY_HIGH_IDLE+30, _update_errors_cb, self->sphere, NULL); + //roam_sphere_update_errors(self->sphere); #endif gis_opengl_redraw(self); gis_opengl_end(self); + gdk_threads_leave(); } static gboolean on_idle(GisOpenGL *self) { //g_debug("GisOpenGL: on_idle"); + gdk_threads_enter(); gis_opengl_begin(self); if (roam_sphere_split_merge(self->sphere)) gis_opengl_redraw(self); gis_opengl_end(self); + gdk_threads_leave(); return TRUE; } @@ -376,6 +390,7 @@ void gis_opengl_end(GisOpenGL *self) g_assert(GIS_IS_OPENGL(self)); GdkGLDrawable *gldrawable = gtk_widget_get_gl_drawable(GTK_WIDGET(self)); gdk_gl_drawable_gl_end(gldrawable); + gdk_threads_leave(); } void gis_opengl_flush(GisOpenGL *self) { @@ -415,7 +430,7 @@ static void gis_opengl_init(GisOpenGL *self) g_object_set(self, "can-focus", TRUE, NULL); #ifndef ROAM_DEBUG - self->sm_source = g_timeout_add(10, (GSourceFunc)on_idle, self); + self->sm_source = g_timeout_add_full(G_PRIORITY_HIGH_IDLE+30, 33, (GSourceFunc)on_idle, self, NULL); #endif g_signal_connect(self, "realize", G_CALLBACK(on_realize), NULL); diff --git a/src/gis_test.c b/src/gis_test.c index df2692f..efd9218 100644 --- a/src/gis_test.c +++ b/src/gis_test.c @@ -51,16 +51,20 @@ int main(int argc, char **argv) GisView *view = gis_view_new(); GisOpenGL *opengl = gis_opengl_new(world, view, plugins); + gdk_threads_enter(); GtkWidget *window = gtk_window_new(GTK_WINDOW_TOPLEVEL); g_signal_connect(window, "destroy", G_CALLBACK(gtk_main_quit), NULL); g_signal_connect(window, "key-press-event", G_CALLBACK(on_key_press), NULL); gtk_container_add(GTK_CONTAINER(window), GTK_WIDGET(opengl)); gtk_widget_show_all(window); - //gis_plugins_load(plugins, "bmng", world, view, opengl, prefs); + gdk_threads_leave(); + + gis_plugins_load(plugins, "bmng", world, view, opengl, prefs); gis_plugins_load(plugins, "srtm", world, view, opengl, prefs); gis_view_set_site(view, "KLSX"); + gdk_threads_enter(); gtk_main(); g_object_unref(prefs); @@ -68,5 +72,6 @@ int main(int argc, char **argv) g_object_unref(view); g_object_unref(opengl); gis_plugins_free(plugins); + gdk_threads_leave(); return 0; } diff --git a/src/plugins/bmng.c b/src/plugins/bmng.c index a1499c6..8b02749 100644 --- a/src/plugins/bmng.c +++ b/src/plugins/bmng.c @@ -26,15 +26,21 @@ #define TILE_WIDTH 1024 #define TILE_HEIGHT 512 -static void _load_tile(GisTile *tile, gpointer _self) +struct _LoadTileData { + GisPluginBmng *self; + GisTile *tile; + GdkPixbuf *pixbuf; +}; +static gboolean _load_tile_cb(gpointer _data) { - GisPluginBmng *self = _self; - g_debug("GisPluginBmng: _load_tile start"); - - char *path = gis_wms_make_local(self->wms, tile); - GdkPixbuf *pixbuf = gdk_pixbuf_new_from_file(path, NULL); - g_free(path); + struct _LoadTileData *data = _data; + GisPluginBmng *self = data->self; + GisTile *tile = data->tile; + GdkPixbuf *pixbuf = data->pixbuf; + g_free(data); + /* Create Texture */ + g_debug("GisPluginBmng: _load_tile_cb start"); guchar *pixels = gdk_pixbuf_get_pixels(pixbuf); gboolean alpha = gdk_pixbuf_get_has_alpha(pixbuf); gint width = gdk_pixbuf_get_width(pixbuf); @@ -59,21 +65,41 @@ static void _load_tile(GisTile *tile, gpointer _self) tile->data = tex; gis_opengl_redraw(self->opengl); g_object_unref(pixbuf); + return FALSE; } -static void _free_tile(GisTile *tile, gpointer _self) +static void _load_tile(GisTile *tile, gpointer _self) { GisPluginBmng *self = _self; - g_debug("GisPluginBmng: _free_tile: %p=%d", tile->data, *(guint*)tile->data); - guint *data = tile->data; + g_debug("GisPluginBmng: _load_tile start %p", g_thread_self()); + char *path = gis_wms_make_local(self->wms, tile); + struct _LoadTileData *data = g_new0(struct _LoadTileData, 1); + data->self = self; + data->tile = tile; + data->pixbuf = gdk_pixbuf_new_from_file(path, NULL); + g_free(path); + g_idle_add_full(G_PRIORITY_LOW, _load_tile_cb, data, NULL); + g_debug("GisPluginBmng: _load_tile end %p", g_thread_self()); +} + +static gboolean _free_tile_cb(gpointer data) +{ glDeleteTextures(1, data); g_free(data); + return FALSE; +} +static void _free_tile(GisTile *tile, gpointer _self) +{ + GisPluginBmng *self = _self; + g_debug("GisPluginBmng: _free_tile: %p=%d", tile->data, *(guint*)tile->data); + g_idle_add_full(G_PRIORITY_LOW, _free_tile_cb, tile->data, NULL); } static gpointer _update_tiles(gpointer _self) { g_debug("GisPluginBmng: _update_tiles"); GisPluginBmng *self = _self; + g_mutex_lock(self->mutex); gdouble lat, lon, elev; gis_view_get_location(self->view, &lat, &lon, &elev); gis_tile_update(self->tiles, @@ -82,6 +108,7 @@ static gpointer _update_tiles(gpointer _self) _load_tile, self); gis_tile_gc(self->tiles, time(NULL)-10, _free_tile, self); + g_mutex_unlock(self->mutex); return NULL; } @@ -91,7 +118,7 @@ static gpointer _update_tiles(gpointer _self) static void _on_location_changed(GisView *view, gdouble lat, gdouble lon, gdouble elev, GisPluginBmng *self) { - _update_tiles(self); + g_thread_create(_update_tiles, self, FALSE, NULL); } /*********** @@ -106,7 +133,7 @@ GisPluginBmng *gis_plugin_bmng_new(GisWorld *world, GisView *view, GisOpenGL *op /* Load initial tiles */ _load_tile(self->tiles, self); - _update_tiles(self); + g_thread_create(_update_tiles, self, FALSE, NULL); /* Connect signals */ g_signal_connect(self->view, "location-changed", G_CALLBACK(_on_location_changed), self); @@ -141,6 +168,7 @@ static void gis_plugin_bmng_init(GisPluginBmng *self) { g_debug("GisPluginBmng: init"); /* Set defaults */ + self->mutex = g_mutex_new(); self->tiles = gis_tile_new(NULL, NORTH, SOUTH, EAST, WEST); self->wms = gis_wms_new( "http://www.nasa.network.com/wms", "bmng200406", "image/jpeg", @@ -160,6 +188,7 @@ static void gis_plugin_bmng_finalize(GObject *gobject) /* Free data */ gis_tile_free(self->tiles, _free_tile, self); gis_wms_free(self->wms); + g_mutex_free(self->mutex); G_OBJECT_CLASS(gis_plugin_bmng_parent_class)->finalize(gobject); } diff --git a/src/plugins/bmng.h b/src/plugins/bmng.h index 21c562a..5ebed78 100644 --- a/src/plugins/bmng.h +++ b/src/plugins/bmng.h @@ -38,6 +38,7 @@ struct _GisPluginBmng { GisOpenGL *opengl; GisTile *tiles; GisWms *wms; + GMutex *mutex; }; struct _GisPluginBmngClass { diff --git a/src/plugins/srtm.c b/src/plugins/srtm.c index 6960241..67dd7e0 100644 --- a/src/plugins/srtm.c +++ b/src/plugins/srtm.c @@ -85,7 +85,13 @@ static gdouble _height_func(gdouble lat, gdouble lon, gpointer _self) * Loader and Freeers * **********************/ #define LOAD_BIL TRUE -#define LOAD_OPENGL TRUE +#define LOAD_OPENGL FALSE +struct _LoadTileData { + GisPluginSrtm *self; + GisTile *tile; + GdkPixbuf *pixbuf; + struct _TileData *data; +}; static guint16 *_load_bil(gchar *path) { gchar *data; @@ -141,23 +147,20 @@ static guint _load_opengl(GdkPixbuf *pixbuf) g_debug("GisPluginSrtm: load_opengl %d", opengl); return opengl; } -static void _load_tile(GisTile *tile, gpointer _self) +static gboolean _load_tile_cb(gpointer _load) { - GisPluginSrtm *self = _self; - g_debug("GisPluginSrtm: _load_tile"); + struct _LoadTileData *load = _load; + GisPluginSrtm *self = load->self; + GisTile *tile = load->tile; + GdkPixbuf *pixbuf = load->pixbuf; + struct _TileData *data = load->data; + g_free(load); - struct _TileData *data = g_new0(struct _TileData, 1); - gchar *path = gis_wms_make_local(self->wms, tile); - if (LOAD_BIL || LOAD_OPENGL) - data->bil = _load_bil(path); - g_free(path); - if (LOAD_OPENGL) { - GdkPixbuf *pixbuf = _load_pixbuf(data->bil); + if (LOAD_OPENGL) data->opengl = _load_opengl(pixbuf); - g_object_unref(pixbuf); - } /* Do necessasairy processing */ + /* TODO: Lock this and move to thread, can remove self from _load then */ if (LOAD_BIL) gis_opengl_set_height_func(self->opengl, tile, _height_func, self, TRUE); @@ -165,25 +168,52 @@ static void _load_tile(GisTile *tile, gpointer _self) /* Cleanup unneeded things */ if (!LOAD_BIL) g_free(data->bil); + if (LOAD_OPENGL) + g_object_unref(pixbuf); tile->data = data; + return FALSE; } - -static void _free_tile(GisTile *tile, gpointer _self) +static void _load_tile(GisTile *tile, gpointer _self) { GisPluginSrtm *self = _self; - struct _TileData *data = tile->data; - g_debug("GisPluginSrtm: _free_tile: %p=%d", data, data->opengl); + + g_debug("GisPluginSrtm: _load_tile"); + gchar *path = gis_wms_make_local(self->wms, tile); + struct _LoadTileData *load = g_new0(struct _LoadTileData, 1); + load->self = self; + load->tile = tile; + load->data = g_new0(struct _TileData, 1); + if (LOAD_BIL || LOAD_OPENGL) + load->data->bil = _load_bil(path); + if (LOAD_OPENGL) + load->pixbuf = _load_pixbuf(load->data->bil); + + g_idle_add_full(G_PRIORITY_LOW, _load_tile_cb, load, NULL); + g_free(path); +} + +static gboolean _free_tile_cb(gpointer _data) +{ + struct _TileData *data = _data; if (LOAD_BIL) g_free(data->bil); if (LOAD_OPENGL) glDeleteTextures(1, &data->opengl); g_free(data); + return FALSE; +} +static void _free_tile(GisTile *tile, gpointer _self) +{ + GisPluginSrtm *self = _self; + g_debug("GisPluginSrtm: _free_tile: %p=%d", tile->data, *(guint*)tile->data); + g_idle_add_full(G_PRIORITY_LOW, _free_tile_cb, tile->data, NULL); } static gpointer _update_tiles(gpointer _self) { GisPluginSrtm *self = _self; + g_mutex_lock(self->mutex); gdouble lat, lon, elev; gis_view_get_location(self->view, &lat, &lon, &elev); gis_tile_update(self->tiles, @@ -192,6 +222,7 @@ static gpointer _update_tiles(gpointer _self) _load_tile, self); gis_tile_gc(self->tiles, time(NULL)-10, _free_tile, self); + g_mutex_unlock(self->mutex); return NULL; } @@ -201,7 +232,7 @@ static gpointer _update_tiles(gpointer _self) static void _on_location_changed(GisView *view, gdouble lat, gdouble lon, gdouble elev, GisPluginSrtm *self) { - _update_tiles(self); + g_thread_create(_update_tiles, self, FALSE, NULL); } /*********** @@ -216,7 +247,7 @@ GisPluginSrtm *gis_plugin_srtm_new(GisWorld *world, GisView *view, GisOpenGL *op /* Load initial tiles */ _load_tile(self->tiles, self); - _update_tiles(self); + g_thread_create(_update_tiles, self, FALSE, NULL); /* Connect signals */ g_signal_connect(view, "location-changed", G_CALLBACK(_on_location_changed), self); @@ -253,6 +284,7 @@ static void gis_plugin_srtm_init(GisPluginSrtm *self) { g_debug("GisPluginSrtm: init"); /* Set defaults */ + self->mutex = g_mutex_new(); self->tiles = gis_tile_new(NULL, NORTH, SOUTH, EAST, WEST); self->wms = gis_wms_new( "http://www.nasa.network.com/elev", "srtm30", "application/bil", @@ -272,6 +304,7 @@ static void gis_plugin_srtm_finalize(GObject *gobject) /* Free data */ gis_tile_free(self->tiles, _free_tile, self); gis_wms_free(self->wms); + g_mutex_free(self->mutex); G_OBJECT_CLASS(gis_plugin_srtm_parent_class)->finalize(gobject); } diff --git a/src/plugins/srtm.h b/src/plugins/srtm.h index 7af659e..32fc08a 100644 --- a/src/plugins/srtm.h +++ b/src/plugins/srtm.h @@ -38,6 +38,7 @@ struct _GisPluginSrtm { GisOpenGL *opengl; GisTile *tiles; GisWms *wms; + GMutex *mutex; }; struct _GisPluginSrtmClass { -- 2.43.2