GNOME Bugzilla – Bug 657639
Junk visible_pixels_region
Last modified: 2012-02-04 01:01:00 UTC
<owen> magcius: oh, that whole visible_pixels_region thing was unnecessary - the place it was used was just doing a small amount of drawing optimization and could just have been removed... well, anyways, we have it now :-) I was told not to create this patch, but I already did by the time you told me not to :)
Created attachment 195095 [details] [review] Junk visible_pixels_region It isn't necessary -- it was a minor optimization that's now invalid because we can paint outside our own shape_region.
Created attachment 195146 [details] [review] Junk visible_pixels_region It isn't necessary -- it was a minor optimization that's now invalid because we can paint outside our own shape_region. Fix compile error and weird "window tiling" issue.
Created attachment 196455 [details] [review] Junk visible_pixels_region It isn't necessary -- it was a minor optimization that's now invalid because we can paint outside our own shape_region. Rebased to master after a silly crasher typo.
Created attachment 201191 [details] [review] Junk visible_pixels_region It isn't necessary -- it was a minor optimization that's now invalid because we can paint outside our own shape_region. Rebased to master.
Review of attachment 201191 [details] [review]: MetaShapedTexture: remove visible_pixels_region When we were shaping the window with a cairo region, there was an easy optimization to restrict painting only to the pixels we were going to actually draw. With rounded corners, the amount of work we have to do figure out what pixels isn't worth the small savings of not drawing the completely transparent parts of the corners, so remove this optimization, and the supporting meta_shaped_texture_get_visible_pixels_region() ::: src/compositor/meta-shaped-texture.c @@ +442,3 @@ + rect.height = tex_height; + + cairo_region_intersect_rectangle (priv->clip_region, &rect); I think doing this breaks encapsulation - because you are assuming that the clip region is set and thrown away. That may be true with how we use clip region, but it isn't *necessarily* true. Probably the best thing to do is to use gdk_rectangle_intersect() on each rectangle of the region directly after we fetch it. cairo_region_intersect() is unlikely to be much faster than that. The alternative would be to do cairo_region_intersect() higher in the stack and not in this actor - possibly in MetaWindowGroup. ::: src/compositor/meta-window-actor.c @@ +1757,3 @@ /* Assumes ownership */ meta_shaped_texture_set_clip_region (META_SHAPED_TEXTURE (priv->actor), + cairo_region_copy (visible_region)); If you are calling it like this, there's no reason to leave the "assumes ownership" thing - which was there as a specific optimization to avoid an unnecessary copy. Just move the copy inside meta_shaped_texture_set_clip_region().
Created attachment 206731 [details] [review] MetaShapedTexture: Remove visible_pixels_region When we were shaping the window with a cairo region, there was an easy optimization to restrict painting only to the pixels we were going to actually draw. With rounded corners, the amount of work we have to do figure out what pixels isn't worth the small savings of not drawing the completely transparent parts of the corners, so remove this optimization, and the supporting meta_shaped_texture_get_visible_pixels_region() OK, I used your commit message, and applied the rest of the comments.
Comment on attachment 206731 [details] [review] MetaShapedTexture: Remove visible_pixels_region Wait... nevermind. This seems to cause issues with the overview for some reason.
Created attachment 206733 [details] [review] MetaShapedTexture: Remove visible_pixels_region When we were shaping the window with a cairo region, there was an easy optimization to restrict painting only to the pixels we were going to actually draw. With rounded corners, the amount of work we have to do figure out what pixels isn't worth the small savings of not drawing the completely transparent parts of the corners, so remove this optimization, and the supporting meta_shaped_texture_get_visible_pixels_region() Fixed the issue with the overview.
Review of attachment 206733 [details] [review]: Looks fine except for one small cleanup ::: src/compositor/meta-shaped-texture.c @@ +442,3 @@ + gdk_rectangle_intersect (&tex_rect, &rect, &rect); + + if (rect.width == 0 || rect.height == 0) gdk_rectangle_intersect has a boolean return, so you can do: if (!gdk_rectangle_intersect (...)) continue;
(git-bz was having some trouble, this is fixed)