Hi Pekka, Thanks for the review and detailed explanation.
On Fri, Nov 23, 2012 at 12:35 AM, Pekka Paalanen <[email protected]>wrote: > On Thu, 22 Nov 2012 15:35:13 -0700 > Scott Moreau <[email protected]> wrote: > > > Since surface.commit was introduced, opqaue regions are stored in a > pending > > variable that isn't used until surface.commit. Xwayland uses the surface > opaque > > region as a way to tell weston what region of the surface should be > opaque. > > However when this pending opaque region was introduced, xwm was not > updated > > and so we have the 'black = transparent' problem again. This patch fixes > the > > problem by having xwm use the pending opaque regions. > > --- > > > > Hi, I noticed this broke again and fixed it here. However, now I'm > wondering > > if weston_surface needs an opaque region at all anymore. It seems the > only thing > > that uses it is weston_surface_update_transform_disable(). I'm not sure > if this > > is the best fix but the opaque region system could use a closer > examination > > now that surface.commit is in place. > > Hi Scott, > > I believe it is still needed. The opaque region is needed in global > coordinate frame, but weston_surface:opaque is in surface-local > coordinate frame. Just like the bounding box, > weston_surface:transform.opaque is computed in > weston_surface_update_transform(), and it is used in clipping repaint > regions. > > Weston_surface:opaque is referenced only in the transform_disable() > path, because we never bothered to write an algorithm for the > transform_enable() path. A pixman region deals with axis-aligned > rectangles, and the conversion from an arbitrary rectangle to an > approximating set of axis-aligned rectangles is not trivial nor > unique. In the transform enabled case, we just imagine that nothing is > opaque, and compute repaint regions accordingly. The only downside is > that we may repaint more than absolutely necessary. It should not cause > misrendering, though. > > Weston_surface:opaque is also used in the gl-renderer, where the > surface tessellation is split into two parts: blended and opaque. Ah yes, this is probably the main reason to have it. I overlooked this case when thinking about where the opaque region is used. > This > is possible, because the tessellation is not limited to axis-aligned > rectangles, and the opaque region in surface-local coordinates can be > handled properly. > > The patch looks ok to me in the sense, that it is now modifying the > pending.opaque region, which a following wl_surface.commit will take > into use. I do wonder, how that interacts with possible > wl_surface.set_opaque_region requests from the client. Maybe such > requests do not occur? This will probably be rewritten in the > xwm-as-client work, anyway, so it's fine if it works well enough for > now. > > Thanks, Scott > > Thanks, > pq >
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
