From 2fcb644e4fb02375bdf8b1f54677bcde7abdceba Mon Sep 17 00:00:00 2001 From: Chris Lord Date: Mon, 15 Feb 2010 18:53:58 +0000 Subject: [PATCH] [stage] Fix some races to do with window resizing When we resize, we relied on the stage's allocate to re-initialise the GL viewport. Unfortunately, if we resized within Clutter, the new size was cached before the window is actually resized, so glViewport wasn't being called after resizing (some of the time, it's a race condition). Change the way resizing works slightly so that we only resize when the geometry size doesn't match our preferred size, and queue a relayout on ConfigureNotify so the glViewport gets called. Also change window creation slightly so that setting the size of a window before it's realized works correctly. --- clutter/clutter-stage.c | 36 ++++++++++++++------------------- clutter/glx/clutter-stage-glx.c | 19 +++++++++++++++-- clutter/x11/clutter-event-x11.c | 13 ++++++++++++ clutter/x11/clutter-stage-x11.c | 24 +++++++++------------- 4 files changed, 55 insertions(+), 37 deletions(-) diff --git a/clutter/clutter-stage.c b/clutter/clutter-stage.c index 00fb26c1a..6df81efc3 100644 --- a/clutter/clutter-stage.c +++ b/clutter/clutter-stage.c @@ -182,6 +182,7 @@ clutter_stage_allocate (ClutterActor *self, ClutterAllocationFlags flags) { ClutterStagePrivate *priv = CLUTTER_STAGE (self)->priv; + ClutterGeometry geom = { 0, }; gboolean origin_changed; gint width, height; @@ -192,6 +193,11 @@ clutter_stage_allocate (ClutterActor *self, width = clutter_actor_box_get_width (box); height = clutter_actor_box_get_height (box); + _clutter_stage_window_get_geometry (priv->impl, &geom); + + /* XXX: Until Cogl becomes fully responsible for backend windows Clutter + * need to manually keep it informed of the current window size */ + _cogl_onscreen_clutter_backend_set_size (geom.width, geom.height); /* if the stage is fixed size (for instance, it's using a frame-buffer) * then we simply ignore any allocation request and override the @@ -201,10 +207,6 @@ clutter_stage_allocate (ClutterActor *self, { ClutterActorClass *klass; - /* XXX: Until Cogl becomes fully responsible for backend windows Clutter - * need to manually keep it informed of the current window size */ - _cogl_onscreen_clutter_backend_set_size (width, height); - CLUTTER_NOTE (LAYOUT, "Following allocation to %dx%d (origin %s)", width, height, @@ -213,20 +215,15 @@ clutter_stage_allocate (ClutterActor *self, klass = CLUTTER_ACTOR_CLASS (clutter_stage_parent_class); klass->allocate (self, box, flags); - _clutter_stage_window_resize (priv->impl, width, height); + /* Ensure the window is sized correctly */ + if ((geom.width != width) || (geom.height != height)) + _clutter_stage_window_resize (priv->impl, width, height); } else { ClutterActorBox override = { 0, }; - ClutterGeometry geom = { 0, }; ClutterActorClass *klass; - _clutter_stage_window_get_geometry (priv->impl, &geom); - - /* XXX: Until Cogl becomes fully responsible for backend windows Clutter - * need to manually keep it informed of the current window size */ - _cogl_onscreen_clutter_backend_set_size (geom.width, geom.height); - override.x1 = 0; override.y1 = 0; override.x2 = geom.width; @@ -2335,7 +2332,6 @@ clutter_stage_set_minimum_size (ClutterStage *stage, guint width, guint height) { - gboolean resize; ClutterGeometry geom; g_return_if_fail (CLUTTER_IS_STAGE (stage)); @@ -2347,20 +2343,18 @@ clutter_stage_set_minimum_size (ClutterStage *stage, if (stage->priv->impl == NULL) return; - resize = FALSE; _clutter_stage_window_get_geometry (stage->priv->impl, &geom); - if (geom.width < width) - resize = TRUE; - else + if (geom.width > width) width = geom.width; - if (geom.height < height) - resize = TRUE; - else + if (geom.height > height) height = geom.height; - if (resize) + /* Call resize to ensure that the minimum size is enforced by + * the backend. + */ + if (CLUTTER_ACTOR_IS_MAPPED (stage)) _clutter_stage_window_resize (stage->priv->impl, width, height); } diff --git a/clutter/glx/clutter-stage-glx.c b/clutter/glx/clutter-stage-glx.c index 70e682aae..045a002d4 100644 --- a/clutter/glx/clutter-stage-glx.c +++ b/clutter/glx/clutter-stage-glx.c @@ -114,6 +114,7 @@ clutter_stage_glx_realize (ClutterStageWindow *stage_window) XSetWindowAttributes xattr; unsigned long mask; XVisualInfo *xvisinfo; + gfloat width, height; CLUTTER_NOTE (MISC, "Creating stage X window"); @@ -133,6 +134,18 @@ clutter_stage_glx_realize (ClutterStageWindow *stage_window) xvisinfo->visual, AllocNone); mask = CWBorderPixel | CWColormap; + + /* Call get_size - this will either get the geometry size (which + * before we create the window is set to 640x480), or if a size + * is set, it will get that. This lets you set a size on the + * stage before it's realized. + */ + clutter_actor_get_size (CLUTTER_ACTOR (stage_x11->wrapper), + &width, + &height); + stage_x11->xwin_width = (gint)width; + stage_x11->xwin_height = (gint)height; + stage_x11->xwin = XCreateWindow (backend_x11->xdpy, backend_x11->xwin_root, 0, 0, @@ -144,9 +157,11 @@ clutter_stage_glx_realize (ClutterStageWindow *stage_window) xvisinfo->visual, mask, &xattr); - CLUTTER_NOTE (BACKEND, "Stage [%p], window: 0x%x", + CLUTTER_NOTE (BACKEND, "Stage [%p], window: 0x%x, size: %dx%d", stage_window, - (unsigned int) stage_x11->xwin); + (unsigned int) stage_x11->xwin, + stage_x11->xwin_width, + stage_x11->xwin_height); XFree (xvisinfo); } diff --git a/clutter/x11/clutter-event-x11.c b/clutter/x11/clutter-event-x11.c index 9f9993814..6a74cf249 100644 --- a/clutter/x11/clutter-event-x11.c +++ b/clutter/x11/clutter-event-x11.c @@ -482,6 +482,9 @@ event_translate (ClutterBackend *backend, xevent->xconfigure.width, xevent->xconfigure.height); + stage_x11->xwin_width = xevent->xconfigure.width; + stage_x11->xwin_height = xevent->xconfigure.height; + clutter_actor_set_size (CLUTTER_ACTOR (stage), xevent->xconfigure.width, xevent->xconfigure.height); @@ -493,6 +496,16 @@ event_translate (ClutterBackend *backend, * to set up the GL viewport with the new size */ clutter_stage_ensure_viewport (stage); + + /* Also queue a relayout - we want glViewport to be called + * with the correct values, and this is done in ClutterStage + * via _cogl_onscreen_clutter_backend_set_size (). + * + * We queue a relayout, because if this ConfigureNotify is + * in response to a size we set in the application, the + * set_size above is essentially a null-op. + */ + clutter_actor_queue_relayout (CLUTTER_ACTOR (stage)); } res = FALSE; break; diff --git a/clutter/x11/clutter-stage-x11.c b/clutter/x11/clutter-stage-x11.c index 38229edc2..3bd0ecde6 100644 --- a/clutter/x11/clutter-stage-x11.c +++ b/clutter/x11/clutter-stage-x11.c @@ -219,27 +219,26 @@ clutter_stage_x11_resize (ClutterStageWindow *stage_window, CLUTTER_NOTE (BACKEND, "New size received: (%d, %d)", width, height); - if (width != stage_x11->xwin_width || - height != stage_x11->xwin_height) + if (stage_x11->xwin != None && !stage_x11->is_foreign_xwin) { - stage_x11->xwin_width = width; - stage_x11->xwin_height = height; + clutter_stage_x11_fix_window_size (stage_x11, width, height); - if (stage_x11->xwin != None && !stage_x11->is_foreign_xwin) + if (width != stage_x11->xwin_width || + height != stage_x11->xwin_height) { CLUTTER_NOTE (BACKEND, "%s: XResizeWindow[%x] (%d, %d)", G_STRLOC, (unsigned int) stage_x11->xwin, - stage_x11->xwin_width, - stage_x11->xwin_height); + width, + height); CLUTTER_SET_PRIVATE_FLAGS (stage_x11->wrapper, CLUTTER_STAGE_IN_RESIZE); XResizeWindow (backend_x11->xdpy, stage_x11->xwin, - stage_x11->xwin_width, - stage_x11->xwin_height); + width, + height); /* If the viewport hasn't previously been initialized then even * though we can't guarantee that the server will honour our request @@ -249,17 +248,14 @@ clutter_stage_x11_resize (ClutterStageWindow *stage_window, { ClutterPerspective perspective; clutter_stage_get_perspective (stage, &perspective); - _cogl_setup_viewport (stage_x11->xwin_width, - stage_x11->xwin_height, + _cogl_setup_viewport (width, + height, perspective.fovy, perspective.aspect, perspective.z_near, perspective.z_far); } } - - if (!resize) - clutter_stage_x11_fix_window_size (stage_x11, width, height); } }