From a28440eafbfc126020fc83544eb89d06069df3e2 Mon Sep 17 00:00:00 2001 From: Neil Roberts Date: Thu, 15 Apr 2010 13:57:45 +0100 Subject: [PATCH] clutter-texture: Remove confusing comments about realization In 125bded81 some comments were introduced to ClutterTexture complaining that it can have a Cogl texture before being realized. Clutter always assumes that the single GL context is current so there is no need to wait until the actor is realized before setting a texture. This patch replaces the comments with clarification that this should not be a problem. The patch also changes the documentation about the realized state in various places to clarify that it is acceptable to create any Cogl resources before the actor is realized. http://bugzilla.openedhand.com/show_bug.cgi?id=2075 --- clutter/clutter-actor.c | 49 +++++++++++++++++++++++----------------- clutter/clutter-texture.c | 30 ++++++++++-------------- doc/clutter-actor-invariants.txt | 5 +++- 3 files changed, 44 insertions(+), 40 deletions(-) diff --git a/clutter/clutter-actor.c b/clutter/clutter-actor.c index d7d4b0b..4452671 100644 --- a/clutter/clutter-actor.c +++ b/clutter/clutter-actor.c @@ -180,14 +180,18 @@ * * Evaluates to %TRUE if the %CLUTTER_ACTOR_REALIZED flag is set. * - * Whether GL resources such as textures are allocated; - * if an actor is mapped it must also be realized, but an actor - * can be realized and unmapped (this is so hiding an actor temporarily - * doesn't do an expensive unrealize/realize). + * The realized state has an actor-dependant interpretation. If an + * actor wants to delay allocating resources until it is attached to a + * stage, it may use the realize state to do so. However it is + * perfectly acceptable for an actor to allocate Cogl resources before + * being realized because there is only one GL context used by Clutter + * so any resources will work on any stage. If an actor is mapped it + * must also be realized, but an actor can be realized and unmapped + * (this is so hiding an actor temporarily doesn't do an expensive + * unrealize/realize). * * To be realized an actor must be inside a stage, and all its parents - * must be realized. The stage is required so the actor knows the - * correct GL context and window system resources to use. + * must be realized. * * Since: 0.2 */ @@ -1232,11 +1236,12 @@ clutter_actor_hide_all (ClutterActor *self) * clutter_actor_realize: * @self: A #ClutterActor * - * Creates any underlying graphics resources needed by the actor to be - * displayed. - * - * Realization means the actor is now tied to a specific rendering context - * (that is, a specific toplevel stage). + * Realization informs the actor that it is attached to a stage. It + * can use this to allocate resources if it wanted to delay allocation + * until it would be rendered. However it is perfectly acceptable for + * an actor to create resources before being realized because Clutter + * only ever has a single rendering context so that actor is free to + * be moved from one stage to another. * * This function does nothing if the actor is already realized. * @@ -1321,11 +1326,12 @@ clutter_actor_real_unrealize (ClutterActor *self) * clutter_actor_unrealize: * @self: A #ClutterActor * - * Frees up any underlying graphics resources needed by the actor to - * be displayed. - * - * Unrealization means the actor is now independent of any specific - * rendering context (is not attached to a specific toplevel stage). + * Unrealization informs the actor that it may be being destroyed or + * moved to another stage. The actor may want to destroy any + * underlying graphics resources at this point. However it is + * perfectly acceptable for it to retain the resources until the actor + * is destroyed because Clutter only ever uses a single rendering + * context and all of the graphics resources are valid on any stage. * * Because mapped actors must be realized, actors may not be * unrealized if they are mapped. This function hides the actor to be @@ -1368,11 +1374,12 @@ clutter_actor_unrealize (ClutterActor *self) * clutter_actor_unrealize_not_hiding: * @self: A #ClutterActor * - * Frees up any underlying graphics resources needed by the actor to - * be displayed. - * - * Unrealization means the actor is now independent of any specific - * rendering context (is not attached to a specific toplevel stage). + * Unrealization informs the actor that it may be being destroyed or + * moved to another stage. The actor may want to destroy any + * underlying graphics resources at this point. However it is + * perfectly acceptable for it to retain the resources until the actor + * is destroyed because Clutter only ever uses a single rendering + * context and all of the graphics resources are valid on any stage. * * Because mapped actors must be realized, actors may not be * unrealized if they are mapped. You must hide the actor or one of diff --git a/clutter/clutter-texture.c b/clutter/clutter-texture.c index bb93da6..d133aa2 100644 --- a/clutter/clutter-texture.c +++ b/clutter/clutter-texture.c @@ -282,13 +282,14 @@ clutter_texture_realize (ClutterActor *actor) return; } - /* If the texture is not a FBO, then realization is a no-op but - * we still want to be in REALIZED state to maintain invariants. - * We may have already created the texture if someone set some - * data earlier, or we may create it later if someone sets some - * data later. The fact that we may have created it earlier is - * really a bug, since it means ClutterTexture can have GL - * resources without being realized. + /* If the texture is not a FBO, then realization is a no-op but we + * still want to be in REALIZED state to maintain invariants. + * ClutterTexture doesn't need to be realized to have a Cogl texture + * because Clutter assumes that a GL context is always current so + * there is no need to wait to realization time to create the + * texture. Although this is slightly odd it would be wasteful to + * redundantly store a copy of the texture data in local memory just + * so that we can make a texture during realize. */ CLUTTER_NOTE (TEXTURE, "Texture realized"); @@ -1210,12 +1211,6 @@ clutter_texture_get_cogl_texture (ClutterTexture *texture) * @cogl_tex. A reference to the texture is taken so if the handle is * no longer needed it should be deref'd with cogl_handle_unref. * - * This should not be called on an unrealizable texture (one that - * isn't inside a stage). (Currently the ClutterTexture - * implementation relies on being able to have a GL texture while - * unrealized, which means you can get away with it, but it's - * not correct and may change in the future.) - * * Since: 0.8 */ void @@ -1229,11 +1224,10 @@ clutter_texture_set_cogl_texture (ClutterTexture *texture, g_return_if_fail (CLUTTER_IS_TEXTURE (texture)); g_return_if_fail (cogl_is_texture (cogl_tex)); - /* FIXME this implementation should realize the actor if it's in a - * stage, and warn and return if not in a stage yet. However, right - * now everything would break if we did that, so we just fudge it - * and we're broken: we can have a texture without being realized. - */ + /* This function can set the texture without the actor being + realized. This is ok because Clutter requires that the GL context + always be current so there is no point in waiting to realization + to set the texture. */ priv = texture->priv; diff --git a/doc/clutter-actor-invariants.txt b/doc/clutter-actor-invariants.txt index b7c7465..cb28206 100644 --- a/doc/clutter-actor-invariants.txt +++ b/doc/clutter-actor-invariants.txt @@ -29,7 +29,10 @@ ClutterActor. CLUTTER_ACTOR_REALIZED Means: the actor has GPU resources associated to its paint - cycle. + cycle. Note however that an actor is allowed to allocate Cogl + resources before being realized because Clutter only ever uses + one rendering context which is always current. An actor is + free to create resources at any time. Set by clutter_actor_realize(), unset by clutter_actor_unrealize(). Generally set implicitly when the -- 2.7.4