[PATCH weston] xwm: Fix opaque region system.

[Scott Moreau]

Hi Pekka,

Thanks for the review and detailed explanation.

On Fri, Nov 23, 2012 at 12:35 AM, Pekka Paalanen <ppaalanen at gmail.com>wrote:

> On Thu, 22 Nov 2012 15:35:13 -0700
> Scott Moreau <oreaus at gmail.com> 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20121123/492808b2/attachment.html>