After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 659523 - ClutterOffscreenEffect doesn't work for cloned actors
ClutterOffscreenEffect doesn't work for cloned actors
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: ClutterEffect
git master
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2011-09-19 21:37 UTC by drago01
Modified: 2019-01-24 17:10 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
shader-effect: Use clutter_effect_queue_redraw() (970 bytes, patch)
2011-09-20 10:19 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
shader-effect: Use clutter_effect_queue_redraw() (983 bytes, patch)
2011-09-20 10:23 UTC, Emmanuele Bassi (:ebassi)
none Details | Review
MetaWindowActor: Skip paint optimization for actors with an effect (1.48 KB, patch)
2011-09-20 14:58 UTC, drago01
none Details | Review
MetaWindowActor: Skip paint optimization for actors with an effect (1.48 KB, patch)
2011-09-20 18:39 UTC, drago01
none Details | Review
MetaWindowActor: Skip paint optimization for actors with an effect (1.94 KB, patch)
2011-09-20 18:45 UTC, Owen Taylor
committed Details | Review

Description drago01 2011-09-19 21:37:37 UTC
When using ClutterShaderEffect to do a window dimming effect in gnome-shell it ended up causing drawing artifacts (contents of other actors in the fbo or wrong alpha).

I looked a bit through the code and found that a patch like:

--------------
diff --git a/clutter/clutter-offscreen-effect.c b/clutter/clutter-offscreen-effect.c
index bb5268a..34014b2 100644
--- a/clutter/clutter-offscreen-effect.c
+++ b/clutter/clutter-offscreen-effect.c
@@ -419,9 +419,7 @@ clutter_offscreen_effect_paint (ClutterEffect           *effect,
   /* If we've already got a cached image for the same matrix and the
      actor hasn't been redrawn then we can just use the cached image
      in the fbo */
-  if (priv->offscreen == NULL ||
-      (flags & CLUTTER_EFFECT_PAINT_ACTOR_DIRTY) ||
-      !cogl_matrix_equal (&matrix, &priv->last_matrix_drawn))
+  if (priv->offscreen == NULL || TRUE)
     {
       /* Chain up to the parent paint method which will call the pre and
          post paint functions to update the image */

------------

Fixes it so there must be something fishy going on with that check (I don't know what though).

Here is a videos demonstrating the issue http://magcius.mecheye.net/shell/modal-dialog-overview.webm and http://magcius.mecheye.net/shell/modal-dialog-overview-2.webm
Comment 1 Jasper St. Pierre (not reading bugmail) 2011-09-19 21:44:27 UTC
(In reply to comment #0)
> Here is a videos demonstrating the issue
> http://magcius.mecheye.net/shell/modal-dialog-overview.webm and
> http://magcius.mecheye.net/shell/modal-dialog-overview-2.webm

I doubt this is the same issue. This is showing the dialog being clipped strangely when entering the overview in gnome-shell. A more appropriate picture:

http://i.imgur.com/AF9T5.png

Steps for gnome-shell:

1. Open a modal dialog.
2. Drag a window over it.
3. ???
4. Corruption.
Comment 2 Emmanuele Bassi (:ebassi) 2011-09-20 10:19:58 UTC
Created attachment 197029 [details] [review]
shader-effect: Use clutter_effect_queue_redraw()

When changing the value of a uniform we must tell Clutter to mark the
effect as dirty, to avoid the OffscreenEffect cache kicking in.
Comment 3 Emmanuele Bassi (:ebassi) 2011-09-20 10:23:17 UTC
Created attachment 197030 [details] [review]
shader-effect: Use clutter_effect_queue_redraw()

When changing the value of a uniform we must tell Clutter to mark the
effect as dirty, to avoid the OffscreenEffect cache kicking in.
Comment 4 drago01 2011-09-20 10:36:46 UTC
(In reply to comment #3)
> Created an attachment (id=197030) [details] [review]
> shader-effect: Use clutter_effect_queue_redraw()
> 
> When changing the value of a uniform we must tell Clutter to mark the
> effect as dirty, to avoid the OffscreenEffect cache kicking in.

<drago01> ebassi: does not fix it
<ebassi> mmh, I think the queue_redraw() caused by add_effect() later will override this call
<drago01> when is clutter_shader_effect_add_uniform called?
<drago01> just when setting uniforms? 
<ebassi> whenever you change/add a uniform value
<drago01> well the corruption happens later then that
<drago01> i.e long after the tween finishes
<drago01> and it stays in the "last position"
<ebassi> yes, because the offscreen buffer gets cached, and if the effect is still in place, it'll be what gets painted
<ebassi> queue_repaint() on the effect is the function that's supposed to set the DIRTY flag and go through the offscreen creation code
<drago01> oh found something else ... a clone of the actor with the effect (the thumbnail in altTab) is just empty
<drago01> i.e nothing drawn at all
<ebassi> okay, this'll need bpeel as he wrote the effect invalidation code
<drago01> ebassi: the clone issue can be "fixed" using this.actor.connect("paint", Lang.bind(this, function() { this.effect.queue_repaint();  }));
<drago01> but that's a gross hack
<drago01> (and does not fix the other issue)
<ebassi> and once again, Clones prove to be a pain. news at 11. ;-)
<drago01> ;)
Comment 5 drago01 2011-09-20 14:58:09 UTC
Created attachment 197070 [details] [review]
MetaWindowActor: Skip paint optimization for actors with an effect

There is no point in trying to do any optimizations here, as
we don't (and currently can't know) whether the actor will be painted
in an offscreen buffer or not.

So we simply skip actors with an effect applied here, otherwise we might
end up with rendering corruption in when the effects gets painted.
Comment 6 drago01 2011-09-20 18:39:41 UTC
Created attachment 197106 [details] [review]
MetaWindowActor: Skip paint optimization for actors with an effect

There is no point in trying to do any optimizations here, as
we don't (and currently can't know) whether the actor will be painted
in an offscreen buffer or not.

So we simply skip actors with an effect applied here, otherwise we might
end up with rendering corruption in when the effects gets painted.

----

The old check was backwards ..
Comment 7 Owen Taylor 2011-09-20 18:45:08 UTC
Created attachment 197107 [details] [review]
MetaWindowActor: Skip paint optimization for actors with an effect

Same patch, with improved (I think) commit message and comment.
Comment 8 drago01 2011-09-20 18:54:00 UTC
Attachment 197107 [details] pushed as 7c6bc73 - MetaWindowActor: Skip paint optimization for actors with an effect
Comment 9 Owen Taylor 2011-09-20 19:18:27 UTC
Retitling and reopening - though the Mutter fix is basically outside the realm of what Clutter can do anything about, much of the problems we were having came from another problem: ClutterOffscreenEffect simply doesn't work when cloned, since it does set up for the fbo using screen space coordinate for the original actor.

 - Simple fix: when inside a clone paint, always allocate a one-off buffer, and repaint and discard it.

 - Complex fix: somehow try to keep one fbo per paint site?

We worked around this for GNOME Shell by moving the effect up to a different actor from the on we were cloning.
Comment 10 Daniel van Vugt 2018-05-16 10:07:46 UTC
I feel comment 9 might be better dealt with as bug 789050.

And maybe we can call this one closed at comment 8?
Comment 11 Marco Trevisan (Treviño) 2019-01-24 17:10:51 UTC
Fixed with commit 8655bc5d8de6a969e0ca83eff8e450f62d28fbee