Hi Pekka, Thanks for the review and detailed explanation. <div class="gmail_quote">On Fri, Nov 23, 2012 at 12:35 AM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im">On Thu, 22 Nov 2012 15:35:13 -0700 Scott Moreau <<a href="mailto:oreaus@gmail.com">oreaus@gmail.com</a>> 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. </div>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.</blockquote><div> Ah yes, this is probably the main reason to have it. I overlooked this case when thinking about where the opaque region is used. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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. </blockquote><div> Thanks, Scott </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Thanks, pq </blockquote></div>