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 654551 - meta-window-group: Use clutter_stage_get_redraw_clip_bounds
meta-window-group: Use clutter_stage_get_redraw_clip_bounds
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2011-07-13 13:06 UTC by Neil Roberts
Modified: 2011-07-13 15:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
meta-window-group: Use clutter_stage_get_redraw_clip_bounds (3.28 KB, patch)
2011-07-13 13:06 UTC, Neil Roberts
reviewed Details | Review
meta-window-group: Use clutter_stage_get_redraw_clip_bounds (3.62 KB, patch)
2011-07-13 15:13 UTC, Neil Roberts
accepted-commit_now Details | Review

Description Neil Roberts 2011-07-13 13:06:02 UTC
Clutter now has some API to get the redraw clip bounds so Mutter
should use it to avoid the layering violation of accessing GL
directly.

For reference see:

https://bugzilla.gnome.org/show_bug.cgi?id=634779
http://bugzilla.clutter-project.org/show_bug.cgi?id=2421
Comment 1 Neil Roberts 2011-07-13 13:06:04 UTC
Created attachment 191878 [details] [review]
meta-window-group: Use clutter_stage_get_redraw_clip_bounds

Clutter now has some API to get the bounds of the current redraw clip
so Mutter no longer needs to make direct GL calls to get the scissor
rect. This should make it more robust against Cogl or Clutter changing
how it does the clipping.
Comment 2 Owen Taylor 2011-07-13 13:53:42 UTC
Review of attachment 191878 [details] [review]:

Cool, good to get rid of the scissor stuff!

Would like to see the clutter version in configure.ac bumped

::: src/compositor/meta-window-group.c
@@ +124,3 @@
+  meta_screen_get_size (window_group->screen,
+                        &visible_rect.width,
+                        &visible_rect.height);

Do we need this? We're assuming here (because we're assuming stage coordinates == screen coordinates) that the stage is coextensive with the screen, so it seems like get_redraw_clip_bounds() always gives us the rectangle that we want to start with.

@@ +130,3 @@
+   * that don't need to be painted in this frame */
+  stage = clutter_actor_get_stage (actor);
+  if (stage)

How could we be painting an actor not in a stage? Can't we just assume a stage inside paint?
Comment 3 Florian Müllner 2011-07-13 14:01:26 UTC
(In reply to comment #1)
> Created an attachment (id=191878) [details] [review]
> meta-window-group: Use clutter_stage_get_redraw_clip_bounds

On a side note, quick testing suggests that this patch fixes some redraw issues I was experiencing since our switch to clutter master.
Comment 4 Neil Roberts 2011-07-13 15:13:20 UTC
Created attachment 191894 [details] [review]
meta-window-group: Use clutter_stage_get_redraw_clip_bounds

Here's an updated patch with the three changes that Owen suggested.
Comment 5 Owen Taylor 2011-07-13 15:26:53 UTC
Review of attachment 191894 [details] [review]:

::: src/compositor/meta-window-group.c
@@ +118,3 @@
+  /* Get the clipped redraw bounds from Clutter so that we can avoid
+   * painting shadows on windows that don't need to be painted in this
+   * frame */

Looks good. If you wanted, before committing, maybe add the note about monitors back here:

   frame. In the case of a multihead setup with mismatched monitor sizes, we could
   intersect this with an accurate union of the monitors to avoid painting shadows
   that are visible only in the holes.
Comment 6 Neil Roberts 2011-07-13 15:42:48 UTC
Ok, I've pushed it as 4167ef with the comment about monitors. Thanks.