From 00a3c698686f25e193d0311ad25c903f0ad71e8b Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Fri, 9 Jan 2009 12:06:46 +0000 Subject: [PATCH] [x11] Fix a race condition when resizing a stage There is a race condition when we resize a stage before showing it on X11. The race goes like this: - clutter_init() creates the default stage and realize it, which will cause a 640x480 Window to be created - call set_size(800, 600) on the stage will cause the Window to be resized to 800x600 - call show() on the stage for the first time will cause COGL to set up an 800 by 600 GL viewport - the Window will be mapped, which will cause X to notify the window manager that the Window should be resized to 800x600 - the window manager will approve the resize - X resizes the drawable to 800x600 To fix the race, we need to defer COGL from setting up the viewport until we receive a ConfigureNotify event and the X server has resized the Drawable. In order to defer the call to cogl_setup_viewport() we add a new private flag, CLUTTER_STAGE_IN_RESIZE; the flag is checked whenever we need to change the viewport size along with the SYNC_MATRICES private flag. Thus, cogl_setup_viewport() will be called only if SYNC_MATRICES is set and IN_RESIZE is not set. --- clutter/clutter-main.c | 3 ++- clutter/clutter-private.h | 3 ++- clutter/x11/clutter-event-x11.c | 6 ++++++ clutter/x11/clutter-stage-x11.c | 35 ++++++++++++++++++++++----------- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/clutter/clutter-main.c b/clutter/clutter-main.c index 745fcbbd7..edcb44cf0 100644 --- a/clutter/clutter-main.c +++ b/clutter/clutter-main.c @@ -147,7 +147,8 @@ _clutter_stage_maybe_relayout (ClutterActor *stage) void _clutter_stage_maybe_setup_viewport (ClutterStage *stage) { - if (CLUTTER_PRIVATE_FLAGS (stage) & CLUTTER_ACTOR_SYNC_MATRICES) + if ((CLUTTER_PRIVATE_FLAGS (stage) & CLUTTER_ACTOR_SYNC_MATRICES) && + !(CLUTTER_PRIVATE_FLAGS (stage) & CLUTTER_STAGE_IN_RESIZE)) { ClutterPerspective perspective; guint width, height; diff --git a/clutter/clutter-private.h b/clutter/clutter-private.h index c65bfb066..b1f141bf7 100644 --- a/clutter/clutter-private.h +++ b/clutter/clutter-private.h @@ -62,7 +62,8 @@ typedef enum { */ CLUTTER_ACTOR_IN_PAINT = 1 << 4, /* Used to avoid recursion */ CLUTTER_ACTOR_IN_RELAYOUT = 1 << 5, /* Used to avoid recursion */ - CLUTTER_TEXTURE_IN_CLONE_PAINT = 1 << 6 /* Used for safety in clones */ + CLUTTER_TEXTURE_IN_CLONE_PAINT = 1 << 6, /* Used for safety in clones */ + CLUTTER_STAGE_IN_RESIZE = 1 << 7 /* Used to mark stage resizes */ } ClutterPrivateFlags; typedef enum { diff --git a/clutter/x11/clutter-event-x11.c b/clutter/x11/clutter-event-x11.c index 06e27e766..090e2c28c 100644 --- a/clutter/x11/clutter-event-x11.c +++ b/clutter/x11/clutter-event-x11.c @@ -438,6 +438,12 @@ event_translate (ClutterBackend *backend, xevent->xconfigure.height); stage_x11->handling_configure = FALSE; + + /* the resize process is complete, so we can remove the + * in-resize flag and allow the viewport to be resized + */ + CLUTTER_UNSET_PRIVATE_FLAGS (CLUTTER_ACTOR (stage_x11->wrapper), + CLUTTER_STAGE_IN_RESIZE); } res = FALSE; break; diff --git a/clutter/x11/clutter-stage-x11.c b/clutter/x11/clutter-stage-x11.c index 010063951..bfc86ba9f 100644 --- a/clutter/x11/clutter-stage-x11.c +++ b/clutter/x11/clutter-stage-x11.c @@ -282,13 +282,30 @@ clutter_stage_x11_allocate (ClutterActor *self, queue. Handling the first event will undo the work of setting the second property which will cause it to keep generating events in an infinite loop. See bug #810 */ - if (stage_x11->xwin != None - && !stage_x11->is_foreign_xwin - && !stage_x11->handling_configure) - XResizeWindow (stage_x11->xdpy, - stage_x11->xwin, - stage_x11->xwin_width, - stage_x11->xwin_height); + if (stage_x11->xwin != None && + !stage_x11->is_foreign_xwin && + !stage_x11->handling_configure) + { + XResizeWindow (stage_x11->xdpy, + stage_x11->xwin, + stage_x11->xwin_width, + stage_x11->xwin_height); + + /* resizing is an asynchronous process; to avoid races + * with the window manager, we flag the wrapper as being + * "in resize", so that the SYNC_MATRICES flag will not + * cause a call to cogl_get_viewport(). + * + * the flag is unset inside clutter-event-x11.c, after + * we receive a ConfigureNotify event. XXX - need to + * check what happens when running without a window manager + */ + CLUTTER_SET_PRIVATE_FLAGS (CLUTTER_ACTOR (stage_x11->wrapper), + CLUTTER_STAGE_IN_RESIZE); + } + + CLUTTER_SET_PRIVATE_FLAGS (CLUTTER_ACTOR (stage_x11->wrapper), + CLUTTER_ACTOR_SYNC_MATRICES); clutter_stage_x11_fix_window_size (stage_x11); @@ -298,9 +315,6 @@ clutter_stage_x11_allocate (ClutterActor *self, clutter_actor_unrealize (self); clutter_actor_realize (self); } - - CLUTTER_SET_PRIVATE_FLAGS (CLUTTER_ACTOR (stage_x11->wrapper), - CLUTTER_ACTOR_SYNC_MATRICES); } /* chain up to fill in actor->priv->allocation */ @@ -769,4 +783,3 @@ clutter_stage_x11_unmap (ClutterStageX11 *stage_x11) CLUTTER_ACTOR_UNSET_FLAGS (stage_x11, CLUTTER_ACTOR_MAPPED); CLUTTER_ACTOR_UNSET_FLAGS (stage_x11->wrapper, CLUTTER_ACTOR_MAPPED); } -