GNOME Bugzilla – Bug 714706
Add a MetaCullable interface
Last modified: 2013-11-25 20:12:29 UTC
This is something I worked on for Wayland subsurfaces support, but it's complex enough and a big enough cleanup that I want Owen to review it...
Created attachment 261117 [details] [review] window-group: Decompose culling operations into two methods We will add an interface for this soon, so we can recursively cull like this...
Created attachment 261118 [details] [review] window-actor: Move the operations we need to do when we cull out here Soon, we'll move this into a generic MetaCullable interface, but for now, just put hardcoded knowledge in MetaWindowGroup.
Created attachment 261119 [details] [review] cullable: Turn cull_out / reset_culling into a separate interface Instead of hardcoded knowledge of certain classes in MetaWindowGroup, create a generic interface that all actors can implement to get parts of their regions culled out during redraw, without needing any special knowledge of how to handle a specific actor. When MetaWindowGroup is painted, we walk over every actor and ask themselves to cull out the opaque parts of the region. A MetaCullable implementation should do this by subtracting the opaque parts of its actor out of the passed in region. We actually pass along two regions: the "unobscured" region, which starts off with the full stage window, and the "clip" region, which only contains the stage's paint clip. Actors should copy the "clip" region and clip themselves to it during painting, as painting won't automatically be culled... After the painting is done, we call a reset function. MetaCullable implementations should clear their copied clip regions at this part. MetaCullable also has a simple recursive default implementation that is used by MetaBackgroundGroup and friends. MetaBackgroundGroup is still needed as a 'tagging' class to determine whether it is a background when syncing the actor stacking order, but this may go away entirely in the future.
Review of attachment 261119 [details] [review]: Mostly seems like a good reorganization. I'd like to see docs for the interface (including things like coordinate systems) in the interface, and not in the commit message. I would at least suggest that perhaps the default implementations should be helper functions and not default implementations - I'm not completely comfortable with the idea that interfaces can have default implementations :-) Would also a) save trickiness with chaining up b) potentially allow culling through containers that don't implement MetaCullable - if you e.g., got rid of the MetaBackgroundGroup class.
Review of attachment 261118 [details] [review]: Seems fine. (Assuming that docs get added to MetaCullable about the interface contract for these functions.)
Review of attachment 261117 [details] [review]: Seems fine
Created attachment 261242 [details] [review] cullable: Turn cull_out / reset_culling into a separate interface Instead of hardcoded knowledge of certain classes in MetaWindowGroup, create a generic interface that all actors can implement to get parts of their regions culled out during redraw, without needing any special knowledge of how to handle a specific actor.
Created attachment 261469 [details] [review] window-group: Decompose culling operations into two methods We will add an interface for this soon, so we can recursively cull like this...
Created attachment 261470 [details] [review] window-actor: Move the operations we need to do when we cull out here Soon, we'll move this into a generic MetaCullable interface, but for now, just put hardcoded knowledge in MetaWindowGroup.
Created attachment 261471 [details] [review] cullable: Turn cull_out / reset_culling into a separate interface Instead of hardcoded knowledge of certain classes in MetaWindowGroup, create a generic interface that all actors can implement to get parts of their regions culled out during redraw, without needing any special knowledge of how to handle a specific actor. The names now are a bit suspect. MetaBackgroundGroup is a simple MetaCullable that knows how to cull children, and MetaWindowGroup is the "toplevel" cullable that computes the initial two regions. A future cleanup here could be to merge MetaWindowGroup / MetaBackgroundGroup so that we only have a generic MetaSimpleCullable, and move the "toplevel" cullability to be a MetaCullableToplevel.
Review of attachment 261471 [details] [review]: I like the improvements here over the last version. The only problem I see are in one area of code that doesn't get exercised because when we clone the window group we either: - Scale it - Show it at 0x0 But never show it at an offset without scaling. ::: src/compositor/meta-window-group.c @@ +122,3 @@ + cairo_region_translate (unobscured_region, paint_x_offset, paint_y_offset); + cairo_region_translate (clip_region, paint_x_offset, paint_y_offset); There's actually a bug in the current code - unobscured_region shouldn't be affected by paint_x_offset, paint_y_offset. (I didn't catch this when unobscured_region was introduced.) Plus, the sign here is wrong. Say that we're painting the window group with paint_x_origin/paint_y_origin at +10+10, and the clip_region is +10+10x100x100 - then paint_x_offset/paint_y_offset are 10,10 - and we want to offset clip_region *the other way* to get a clip in actor coordinates of +0+0x100x100. I also think this should be done in the wrapper code not in cull_out - because it's in essence a fixup on the way we compute clip_region.
Created attachment 261483 [details] [review] window-group: Decompose culling operations into two methods This also fixes a bug in the translation of clip_region. We will add an interface for this soon, so we can recursively cull like this...
Created attachment 261484 [details] [review] window-actor: Move the operations we need to do when we cull out here Soon, we'll move this into a generic MetaCullable interface, but for now, just put hardcoded knowledge in MetaWindowGroup.
Created attachment 261485 [details] [review] cullable: Turn cull_out / reset_culling into a separate interface Instead of hardcoded knowledge of certain classes in MetaWindowGroup, create a generic interface that all actors can implement to get parts of their regions culled out during redraw, without needing any special knowledge of how to handle a specific actor. The names now are a bit suspect. MetaBackgroundGroup is a simple MetaCullable that knows how to cull children, and MetaWindowGroup is the "toplevel" cullable that computes the initial two regions. A future cleanup here could be to merge MetaWindowGroup / MetaBackgroundGroup so that we only have a generic MetaSimpleCullable, and move the "toplevel" cullability to be a MetaCullableToplevel.
Review of attachment 261483 [details] [review]: Looks good
Review of attachment 261484 [details] [review]: Should be OK
Review of attachment 261485 [details] [review]: Looks good
Attachment 261483 [details] pushed as 59a0113 - window-group: Decompose culling operations into two methods Attachment 261484 [details] pushed as d8c6607 - window-actor: Move the operations we need to do when we cull out here Attachment 261485 [details] pushed as 4714425 - cullable: Turn cull_out / reset_culling into a separate interface