[PATCH] Fix lightbox signal handling (#1000927)
Vratislav Podzimek
vpodzime at redhat.com
Tue Sep 10 11:30:36 UTC 2013
On Fri, 2013-09-06 at 17:34 -0400, David Shea wrote:
> As part of this, I also:
> - Removed anaconda_lb_destroy, since it didn't do anything useful that
> a signal handler couldn't
> - Bumped the soversion of libAnacondaWidgets since this removes a
> public interface
> - Switched to using destroy handlers to cleanup the lightbox
> references that are set in other objects
> - Renamed some variables and added some comments to hopefully make it
> easier to tell what's actually going on
> ---
> pyanaconda/ui/gui/tools/run-spoke.py | 2 +-
> pyanaconda/ui/gui/utils.py | 2 +-
> scripts/makeupdates | 2 +-
> widgets/src/Makefile.am | 2 +-
> widgets/src/lightbox.c | 290 ++++++++++++++++++++++++-----------
> widgets/src/lightbox.h | 1 -
> 6 files changed, 206 insertions(+), 93 deletions(-)
>
> diff --git a/pyanaconda/ui/gui/tools/run-spoke.py b/pyanaconda/ui/gui/tools/run-spoke.py
> index d3a83a4..d7101cf 100755
> --- a/pyanaconda/ui/gui/tools/run-spoke.py
> +++ b/pyanaconda/ui/gui/tools/run-spoke.py
> @@ -20,7 +20,7 @@ if len(sys.argv)<2:
> sys.exit(1)
>
> # This is a hack to make sure the AnacondaWidgets library gets loaded
> -ctypes.CDLL("libAnacondaWidgets.so.0", ctypes.RTLD_GLOBAL)
> +ctypes.CDLL("libAnacondaWidgets.so.1", ctypes.RTLD_GLOBAL)
>
> # Logging always needs to be set up first thing, or there'll be tracebacks.
> from pyanaconda import anaconda_log
> diff --git a/pyanaconda/ui/gui/utils.py b/pyanaconda/ui/gui/utils.py
> index 38e0f4c..06847dc 100644
> --- a/pyanaconda/ui/gui/utils.py
> +++ b/pyanaconda/ui/gui/utils.py
> @@ -101,7 +101,7 @@ def enlightbox(mainWindow, dialog):
> ANACONDA_WINDOW_GROUP.add_window(lightbox)
> dialog.set_transient_for(lightbox)
> yield
> - AnacondaWidgets.lb_destroy(lightbox)
> + lightbox.destroy()
>
> def ignoreEscape(dlg):
> """Prevent a dialog from accepting the escape keybinding, which emits a
> diff --git a/scripts/makeupdates b/scripts/makeupdates
> index 6e291fd..273428a 100755
> --- a/scripts/makeupdates
> +++ b/scripts/makeupdates
> @@ -358,7 +358,7 @@ def copyUpdatedWidgets(updates, cwd):
>
> os.system('make')
>
> - files = ["libAnacondaWidgets.so", "libAnacondaWidgets.so.0", "libAnacondaWidgets.so.0.0.0"]
> + files = ["libAnacondaWidgets.so", "libAnacondaWidgets.so.1", "libAnacondaWidgets.so.1.0.0"]
> for f in files:
> path = os.path.normpath(cwd + "/widgets/src/.libs/" + f)
> if os.path.islink(path) and not os.path.exists(updates + libdir + os.path.basename(path)):
> diff --git a/widgets/src/Makefile.am b/widgets/src/Makefile.am
> index 4b3bfde..aaab626 100644
> --- a/widgets/src/Makefile.am
> +++ b/widgets/src/Makefile.am
> @@ -65,7 +65,7 @@ libAnacondaWidgets_la_CFLAGS = $(GTK_CFLAGS) $(GLADEUI_CFLAGS) $(LIBXKLAVIER_CFL
> -DWIDGETS_DATADIR=$(WIDGETSDATA)\
> -DTZMAP_DATADIR=$(TZMAPDATA)
> libAnacondaWidgets_la_LIBADD = $(GTK_LIBS) $(GLADEUI_LIBS) $(LIBXKLAVIER_LIBS)
> -libAnacondaWidgets_la_LDFLAGS = $(LTLIBINTL)
> +libAnacondaWidgets_la_LDFLAGS = $(LTLIBINTL) -version-info 1:0:0
> libAnacondaWidgets_la_SOURCES = $(SOURCES) $(HDRS) \
> glade-adaptor.c
>
> diff --git a/widgets/src/lightbox.c b/widgets/src/lightbox.c
> index 43edeeb..41b01db 100644
> --- a/widgets/src/lightbox.c
> +++ b/widgets/src/lightbox.c
> @@ -32,48 +32,150 @@
> */
>
> #include <cairo.h>
> -#include <gdk/gdk.h>
> #include <gtk/gtk.h>
> +#include <string.h>
>
> #include "lightbox.h"
>
> -/* GObject ID for the parent window's configure-event signal handler */
> +/* GObject IDs for the parent window's signal handlers */
> #define ANACONDA_LB_PARENT_CONFIGURE_EVENT "anaconda-configure-event"
> +#define ANACONDA_LB_PARENT_DESTROY_EVENT "anaconda-destroy-event"
>
> -static void anaconda_lb_move_window_to_parent(GtkWidget *parent,
> - GdkEventConfigure *e,
> - GtkWindow *window)
> +/*
> + * Restack the lightbox and its parent any time we receive a configure-event
> + * on the lightbox
> + */
> +static gboolean anaconda_lb_configure_event(
> + GtkWidget *lightbox,
> + GdkEvent *event,
> + gpointer parent
> + )
> {
> - GdkWindow *p_window, *w_window;
> - int pwidth, pheight, width, height, px, py, x, y, nx, ny;
> + GdkWindow *g_parent_window;
> + GdkWindow *g_lightbox_window;
> +
> + if ((event->type != GDK_CONFIGURE) ||
> + !GTK_IS_WIDGET(parent) ||
> + !GTK_IS_WINDOW(lightbox))
> + {
> + return FALSE;
> + }
> +
> + g_lightbox_window = gtk_widget_get_window(lightbox);
> + if (NULL == g_lightbox_window)
> + {
> + return FALSE;
> + }
> +
> + g_parent_window = gtk_widget_get_window(GTK_WIDGET(parent));
> + if (NULL == g_parent_window)
> + {
> + return FALSE;
> + }
> +
> + gdk_window_restack(g_lightbox_window, g_parent_window, TRUE);
> + return FALSE;
> +}
> +
> +/*
> + * Adjust the lightbox any time the parent window's size, position or stacking
> + * changes. Returns FALSE to allow signal processing to continue.
> + */
> +static gboolean anaconda_lb_parent_configure_event(
> + GtkWidget *parent,
> + GdkEvent *event,
> + gpointer lightbox
> + )
> +{
> + /* Always return FALSE to continue processing for this signal. */
> + GdkWindow *g_parent_window;
> + GdkWindow *g_lightbox_window;
> +
> + if ((event->type != GDK_CONFIGURE) ||
> + !GTK_IS_WIDGET(parent) ||
> + !GTK_IS_WINDOW(lightbox))
> + {
> + return FALSE;
> + }
>
> - if (!GTK_IS_WIDGET(parent) || !GTK_IS_WINDOW(window))
> - return;
> + g_lightbox_window = gtk_widget_get_window(GTK_WIDGET(lightbox));
> + if (NULL == g_lightbox_window)
> + {
> + /*
> + * No underlying GdkWindow. This may mean the lightbox is not yet
> + * realized, but whatever the cause, there's nothing we can do here.
> + */
> + return FALSE;
> + }
>
> - p_window = gtk_widget_get_window (parent);
> - w_window = gtk_widget_get_window (GTK_WIDGET(window));
> + /* Resize and move the window according to the event data */
> + gdk_window_move_resize(g_lightbox_window,
> + event->configure.x,
> + event->configure.y,
> + event->configure.width,
> + event->configure.height
> + );
>
> - if (!GDK_IS_WINDOW(p_window) || !GDK_IS_WINDOW(w_window))
> - return;
> + g_parent_window = gtk_widget_get_window(parent);
> + if (NULL == g_parent_window)
> + {
> + return FALSE;
> + }
>
> - pwidth = gdk_window_get_width (p_window);
> - pheight = gdk_window_get_height (p_window);
> - gdk_window_get_position(p_window, &px, &py);
> + /* Stack the lightbox above the parent */
> + gdk_window_restack(g_lightbox_window, g_parent_window, TRUE);
>
> - width = gdk_window_get_width (w_window);
> - height = gdk_window_get_height (w_window);
> - gdk_window_get_position(w_window, &x, &y);
> + return FALSE;
> +}
>
> - nx = px + pwidth / 2 - width / 2;
> - ny = py + pheight / 2 - height / 2;
> +/* Clean up references to lightbox held by the parent window */
> +static void anaconda_lb_cleanup(GtkWidget *lightbox, gpointer user_data)
> +{
> + guint *p_configure_event_handler;
> + guint *p_destroy_event_handler;
> + GtkWindow *parent;
>
> - if (x != nx || y != ny)
> + /* Remove the signal handlers set on the parent window */
> + if (GTK_IS_WINDOW(lightbox))
> {
> - gdk_window_move (w_window, nx, ny);
> - gdk_window_restack(w_window, p_window, TRUE);
> + parent = gtk_window_get_transient_for(GTK_WINDOW(lightbox));
> + if (GTK_IS_WINDOW(parent))
> + {
> + p_configure_event_handler = g_object_get_data(G_OBJECT(lightbox),
> + ANACONDA_LB_PARENT_CONFIGURE_EVENT);
> + p_destroy_event_handler = g_object_get_data(G_OBJECT(lightbox),
> + ANACONDA_LB_PARENT_DESTROY_EVENT);
> +
> + if (NULL != p_configure_event_handler)
> + {
> + g_signal_handler_disconnect(parent, GPOINTER_TO_UINT(p_configure_event_handler));
> + }
> +
> + if (NULL != p_destroy_event_handler)
> + {
> + g_signal_handler_disconnect(parent, GPOINTER_TO_UINT(p_destroy_event_handler));
> + }
> + }
> }
> +}
> +
> +static void anaconda_lb_cleanup_parent(GtkWidget *parent, gpointer user_data)
> +{
> + GtkWidget *lightbox;
>
> - g_object_set_data(G_OBJECT(window), ANACONDA_LB_PARENT_CONFIGURE_EVENT, NULL);
> + /* If the parent window is destoryed, destroy the lightbox */
> + if (GTK_IS_WIDGET(user_data))
> + {
> + lightbox = GTK_WIDGET(user_data);
> +
> + /*
> + * Clear the destroy id so we don't try disconnecting a signal handler
> + * while the handler is being called.
> + */
> + g_object_set_data(G_OBJECT(lightbox), ANACONDA_LB_PARENT_DESTROY_EVENT, NULL);
> +
> + gtk_widget_destroy(lightbox);
> + }
> }
>
> /**
> @@ -89,87 +191,99 @@ static void anaconda_lb_move_window_to_parent(GtkWidget *parent,
> GtkWindow *anaconda_lb_show_over(GtkWindow *window)
> {
> GtkWindow *lightbox;
> - GdkWindow *w_window;
> - GdkWindow *l_window;
> - int width, height;
> - cairo_t *cr;
> - cairo_pattern_t *pattern;
> + GdkWindow *g_window;
> +
> + GdkWindow *g_lightbox_window;
> + GdkWindow *g_parent_window;
> cairo_surface_t *surface;
> - guint signal_handler;
> + cairo_pattern_t *pattern;
> + cairo_t *cr;
>
> + guint configure_event_handler;
> + guint destroy_event_handler;
> +
> + /* Create a new window on top of the parent */
> lightbox = (GTK_WINDOW(gtk_window_new(GTK_WINDOW_TOPLEVEL)));
> gtk_window_set_transient_for(lightbox, window);
> +
> + /* Disable the window decorations */
> gtk_window_set_decorated(lightbox, FALSE);
> gtk_window_set_has_resize_grip(lightbox, FALSE);
> +
> + /* Set the initial position to the center of the parent */
> gtk_window_set_position(lightbox, GTK_WIN_POS_CENTER_ON_PARENT);
> - gtk_window_set_type_hint (lightbox, GDK_WINDOW_TYPE_HINT_SPLASHSCREEN);
> +
> + /* Indicate we will handle drawing the widget so no theme is applied */
> gtk_widget_set_app_paintable(GTK_WIDGET(lightbox), TRUE);
> + gtk_window_set_type_hint(lightbox, GDK_WINDOW_TYPE_HINT_SPLASHSCREEN);
>
> - w_window = gtk_widget_get_window (GTK_WIDGET(window));
> - width = gdk_window_get_width(w_window);
> - height = gdk_window_get_height(w_window);
> - gtk_window_set_default_size(lightbox, width, height);
> - gtk_widget_realize(GTK_WIDGET(lightbox));
> - l_window = gtk_widget_get_window (GTK_WIDGET(lightbox));
> - gdk_window_set_background_pattern (l_window, NULL);
> - gtk_widget_show(GTK_WIDGET(lightbox));
> - surface = gdk_window_create_similar_surface(l_window,
> - CAIRO_CONTENT_COLOR_ALPHA,
> - width, height);
> -
> - cr = cairo_create (surface);
> - gdk_cairo_set_source_window(cr, w_window, 0, 0);
> - cairo_paint(cr);
> - cairo_set_source_rgba(cr, 0.0, 0.0, 0.0, 0.5);
> - cairo_paint(cr);
> - cairo_destroy(cr);
> + /* Set the lightbox to the parent window's dimensions */
> + g_window = gtk_widget_get_window(GTK_WIDGET(window));
> + gtk_window_set_default_size(lightbox,
> + gdk_window_get_width(g_window),
> + gdk_window_get_height(g_window)
> + );
>
> - pattern = cairo_pattern_create_for_surface (surface);
> - gdk_window_set_background_pattern(l_window, pattern);
> - cairo_pattern_destroy (pattern);
> + /* handle restacking events */
> + g_signal_connect(lightbox, "configure-event",
> + G_CALLBACK(anaconda_lb_configure_event), window);
>
> /* make the shade move with the parent window */
> - signal_handler = g_signal_connect(window, "configure-event",
> - G_CALLBACK (anaconda_lb_move_window_to_parent), lightbox);
> + configure_event_handler = g_signal_connect(window, "configure-event",
> + G_CALLBACK(anaconda_lb_parent_configure_event), lightbox);
> +
> + /* cleanup */
> + g_signal_connect(lightbox, "destroy", G_CALLBACK(anaconda_lb_cleanup), NULL);
> + destroy_event_handler = g_signal_connect(window, "destroy",
> + G_CALLBACK(anaconda_lb_cleanup_parent), lightbox);
>
> - /* Save the signal handler in the lightbox so we can remove it later */
> + /*
> + * Save the signal handlers set on the parent so they can be removed when
> + * the lightbox is destroyed.
> + */
> g_object_set_data(G_OBJECT(lightbox), ANACONDA_LB_PARENT_CONFIGURE_EVENT,
> - GUINT_TO_POINTER(signal_handler));
> + GUINT_TO_POINTER(configure_event_handler));
>
> - return lightbox;
> -}
> + g_object_set_data(G_OBJECT(lightbox), ANACONDA_LB_PARENT_DESTROY_EVENT,
> + GUINT_TO_POINTER(destroy_event_handler));
>
> -/**
> - * anaconda_lb_destroy:
> - * @lightbox: a #GtkWindow
> - *
> - * Destroys the previously used lightbox.
> - *
> - * Since: 1.0
> - */
> -void anaconda_lb_destroy(GtkWindow *lightbox)
> -{
> - GtkWindow *window;
> - gpointer p_signal_handler;
> + /*
> + * NB: We should probably be handling the draw signal in order to refresh
> + * the transparent pattern from the parent window whenver something
> + * changes, but by the time things get to the signal handler the surface is
> + * already set up and doesn't support alpha channels and replacing it
> + * doesn't seem to work quite right. Besides, none of these windows are
> + * supposed to move anyway.
> + */
>
> - /* Disconnect the configure-event from the contained window */
> - if (GTK_IS_WINDOW(lightbox))
> - {
> - window = gtk_window_get_transient_for(GTK_WINDOW(lightbox));
> + /* Realize the window to initialize the Gdk objects */
> + gtk_widget_realize(GTK_WIDGET(lightbox));
> + g_lightbox_window = gtk_widget_get_window(GTK_WIDGET(lightbox));
> + g_parent_window = gtk_widget_get_window(GTK_WIDGET(window));
>
> - p_signal_handler = g_object_get_data(G_OBJECT(lightbox),
> - ANACONDA_LB_PARENT_CONFIGURE_EVENT);
> - if ((NULL != p_signal_handler) && GTK_IS_WINDOW(window))
> - {
> - /* XXX HAAAAAAACK:
> - * If the configure-event signal handler for the contained window
> - * hasn't fired yet, do it now.
> - */
> - g_signal_emit_by_name(window, "configure-event", window);
> + /* Create a new surface that supports alpha content */
> + surface = gdk_window_create_similar_surface(g_lightbox_window,
> + CAIRO_CONTENT_COLOR_ALPHA,
> + gdk_window_get_width(g_parent_window),
> + gdk_window_get_height(g_parent_window));
> + cr = cairo_create(surface);
>
> - g_signal_handler_disconnect(window, GPOINTER_TO_UINT(p_signal_handler));
> - }
> - }
> + /* Use the parent window as a pattern and paint it on the surface */
> + gdk_cairo_set_source_window(cr, g_parent_window, 0, 0);
> + cairo_paint(cr);
>
> - gtk_widget_destroy(GTK_WIDGET(lightbox));
> + /* Paint a black, 50% transparent shade */
> + cairo_set_source_rgba(cr, 0.0, 0.0, 0.0, 0.5);
> + cairo_paint(cr);
> +
> + cairo_destroy(cr);
> +
> + /* Use the surface we painted as the window background */
> + pattern = cairo_pattern_create_for_surface(surface);
> + gdk_window_set_background_pattern(g_lightbox_window, pattern);
> + cairo_pattern_destroy(pattern);
> +
> + gtk_widget_show_all(GTK_WIDGET(lightbox));
> +
> + return lightbox;
> }
> diff --git a/widgets/src/lightbox.h b/widgets/src/lightbox.h
> index 3a97010..db83bd5 100644
> --- a/widgets/src/lightbox.h
> +++ b/widgets/src/lightbox.h
> @@ -23,6 +23,5 @@
> #include <gtk/gtk.h>
>
> GtkWindow *anaconda_lb_show_over(GtkWindow *window);
> -void anaconda_lb_destroy(GtkWindow *lightbox);
>
> #endif
This looks good to me. If there is no implementation of the "proper
object-oriented" solution knocking at the doors, we should apply this
patch to prevent hangs described in the bug.
--
Vratislav Podzimek
Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic
More information about the anaconda-patches
mailing list