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 719669 - lingering shadows
lingering shadows
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2013-12-02 02:04 UTC by Matthias Clasen
Modified: 2013-12-05 14:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
MetaWindowGroup: fix paint volume (1.89 KB, patch)
2013-12-03 05:29 UTC, Owen Taylor
committed Details | Review

Description Matthias Clasen 2013-12-02 02:04:43 UTC
I am seeing shadows that linger after override-redirect windows such as tooltips or menus are unmapped. This is with the gnome-shell/mutter/clutter in f20. In particular, the clutter patches from bug 719368 and bug 719367 are present, and might be to blame.

Screen recording makes the artifacts go away, which seems to indicate that clipped redraws might be at fault.
Comment 1 Owen Taylor 2013-12-02 14:03:45 UTC
(In reply to comment #0)
> I am seeing shadows that linger after override-redirect windows such as
> tooltips or menus are unmapped. This is with the gnome-shell/mutter/clutter in
> f20. In particular, the clutter patches from bug 719368 and bug 719367 are
> present, and might be to blame.
> 
> Screen recording makes the artifacts go away, which seems to indicate that
> clipped redraws might be at fault.

Does this reliably always happen or just sometimes?
Comment 2 Owen Taylor 2013-12-03 05:29:10 UTC
Created attachment 263368 [details] [review]
MetaWindowGroup: fix paint volume

In the past, MetaWindowGroup was allocated the size of the screen and
painted the size of the screen because it contained the screen background,
but now we also have the "top window group" which contains only popup
windows, so the allocation doens't properly reflect the paint bounds
of the window group. Compute the paint bounds accurately from the
children.
Comment 3 Jasper St. Pierre (not reading bugmail) 2013-12-03 05:53:48 UTC
Review of attachment 263368 [details] [review]:

So, when I was doing the MetaCullable stuff, I noticed that painting_untransformed references meta_screen_get_size and compares the absolute vertices of the actor to the screen size. Is that also depending on the allocation being stage/screen-sized?

The commit message should also mention that this is only a problem for popup windows, even if the window_group was not screen-sized, simply because for most other windows, the invisible borders also contain the shadow size. We should look into making that allocation cleanup we mentioned so that the window actor's allocation is equal to the "frame rect" of the window, always, and the shadow/invisible borders are part of the paint volume.

Nit: "doens't" in the commit message.

::: src/compositor/meta-window-group.c
@@ +217,3 @@
+        continue;
+
+      child_volume = clutter_actor_get_transformed_paint_volume (child, actor);

I don't like how (child, actor); reads. I think it's a bit unclear that the second argument is the parent of child. Perhaps change it to (child, self); ?

@@ +220,3 @@
+      if (child_volume == NULL)
+        {
+          res = FALSE;

return FALSE; might be a bit easier to read?
Comment 4 Owen Taylor 2013-12-05 14:04:24 UTC
Attachment 263368 [details] pushed as 55226ad - MetaWindowGroup: fix paint volume
Comment 5 Owen Taylor 2013-12-05 14:07:51 UTC
(In reply to comment #3)
> Review of attachment 263368 [details] [review]:
> 
> So, when I was doing the MetaCullable stuff, I noticed that
> painting_untransformed references meta_screen_get_size and compares the
> absolute vertices of the actor to the screen size. Is that also depending on
> the allocation being stage/screen-sized?

This isn't really an issue. When we are asking "are we painting untransformed?" we have to deal with floating-point imprecision - if we have a matrix that is [1.0001 0][0.001 0.999] is that transformed or not?

For a large enough rectangle, any amount of imprecision will eventually shift the contents enough to no longer align with the pixel grid. So we have to say how large a rectangle we are painting before we can answer the question of whether we are transformed. For the window group, we just input a rectangle the size of the screen. That should be fine, even if the window group isn't *actually* occupying the whole screen.

> The commit message should also mention that this is only a problem for popup
> windows, even if the window_group was not screen-sized, simply because for most
> other windows, the invisible borders also contain the shadow size

Shadows *do* extend outside of the invisible borders in many cases. The reason it was only a problem for popup windows was that for normal windows in the main window group, the allocation of the window group included the background actor, which is screen sized, but popup windows are in the top window group, which shrinks to the size of its contents. This is what is mentioned in the commit message.

>  We should
> look into making that allocation cleanup we mentioned so that the window
> actor's allocation is equal to the "frame rect" of the window, always, and the
> shadow/invisible borders are part of the paint volume.

Yeah.

> Nit: "doens't" in the commit message.

Fixed.

> ::: src/compositor/meta-window-group.c
> @@ +217,3 @@
> +        continue;
> +
> +      child_volume = clutter_actor_get_transformed_paint_volume (child,
> actor);
> 
> I don't like how (child, actor); reads. I think it's a bit unclear that the
> second argument is the parent of child. Perhaps change it to (child, self); ?

OK.

> @@ +220,3 @@
> +      if (child_volume == NULL)
> +        {
> +          res = FALSE;
> 
> return FALSE; might be a bit easier to read?

OK.