Sub-surface protocol

[Daniel Stone]

Hi,

On 6 December 2012 01:32, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> Clipping
>
> The term sub-surface sounds like a sub-window, which may cause one to
> think, that it will be clipped to the parent surface area. I do not
> think we should force or even allow such clipping.
>
> Forcing the clipping would waste memory: for every pixel in
> sub-surfaces there would have to be a pixel in the parent surface.
> For instance, drawing window decorations in four sub-surfaces around
> the main content would not only waste memory, but also not solve the
> problem of GL applications, where one needs to reserve a margin for the
> decorations. (Hello, window decoration libraries? :-)
>

Right, so we're coming at this from completely different angles then.  I
approached this more or less purely from the embedding sense, where you
either have a video element (e.g. GStreamer's waylandvideosink) or a plugin
such as Flash in one parent window.  In this case, you do absolutely want
clipping.

If I was looking at it from the point of view of having the decorations and
window content have a subsurface relationship, I'd be much more tempted to
express it as a group of surfaces instead.  In this case, rather than
having a strict parent/child relationship, we'd have a group of surfaces
with an internal stacking order, and no concept of a parent at all.  This
avoids the need for either disabling occlusion to the parent's bounding box
(which is IMO dangerous and misleading with a parent/child scheme, even if
it does allow for a usecase it wasn't in any way designed for), or having
the parent wl_surface be effectively larger than it actually is.

Something to think about.


> Merely allowing clipping is just not the Wayland style. If the *client*
> wants to clip something, it should not draw it in the first place.
>

For my usecases, clipping is pretty important since you don't want your
video content creeping out over the window borders.  _Not_ allowing
clipping is wasteful in some cases: consider a Flash video in your browser,
which is scrolled half-offscreen.  You'd have to copy the entire decoded
buffer into an intermediate buffer, then copy only the visible part of your
data into a wl_buffer.  Every time you scrolled, you'd have to create a
differently-sized wl_buffer, involving still more copies.

That being said, I don't think clipping to the parent's bounds is
sufficient for this (since you also don't want the child surface drawing
all over your decorations).  I think we can get around this though: see
below.


> * The pending state of all sub-surfaces is applied only on the parent
>   surface commit. Sub-surface commit is a no-op.
>
> This will nicely guarantee atomicity. The set of surfaces forms a
> single application window, and all the surfaces must be updated in sync
> to avoid flickering a bad composite. Resizing will absolutely require
> this to work reliably and without glitches.
>
> The downside is, that e.g. a video sink component cannot just happily
> push buffers to its own sub-surface. It has to signal the application to
> commit the parent surface for every single frame. Some flash, plugin,
> or video APIs might not support that. Do we care?
>

It's some amount of overhead, and for the 99% case of embedded media,
unnecessary if you go with #3 below.


> * Implicit fallback to requiring parent commit on sub-surface
>   configuration change. (by daniels)
>
> This means, that sub-surface commit works like a normal surface's
> commit, if the sub-surface configuration (position, buffer size) does
> not change. When the configuration changes, the compositor will apply
> the new sub-surface state only on the parent surface commit, and until
> then it will not complete the sub-surface commit.
>
> This will allow a video sink to run on its own, as long as the
> sub-surface size or position, e.g. via wl_surface.attach, does not
> change. Resizing can be handled by the application first signalling a
> resize to the video sink, and then repainting and committing the parent
> surface. Both the parent and the sub-surface will be updated
> atomically to the screen.
>

(the application signals a resize to the video sink, the video sink
reconfigures and commits its child surface, signals the parent that it's
ready to continue, then the parent commits its own surface)


> For debugging, if the video sink does something unexpected, like pass
> non-zero x,y to wl_surface.attach, the problem will be clearly visible
> as the video will stop until the parent surface updates.
>
> The downside is, that the client cannot force a sync to the parent
> surface commit without changing the sub-surface configuration.
> Therefore sub-surfaces animating in sync will be very awkward to
> implement, though one can argue, that such applications should not be
> using sub-surfaces to begin with.
>

Yeah, I don't think there's any justification for applications wanting to
animate multiple subsurfaces in sync with each other, at least while they
retain the same configuration.


> * An explicit request toggling the behaviour of a sub-surface, whether
>   it requires the parent surface commit, or commits on its own.
>
> This gives the client explicit control on what happens on sub-surface
> commit. A video player can have its video sink as autonomous, and
> switch to synchronized to parent during resize. However, there might be
> downsides to be discovered.
>

The downside to this is that behaviour becomes non-deterministic.  In my
suggestion above, you could run WAYLAND_DEBUG=1 and instantly see that the
reason the commit never took effect is because the geometry was dirtied.
 With the global flag, you'd have to hunt through the entire context
looking for the configuration, and we'd also have to maintain two codepaths
inside the compositor, which adds to the testing burden.


> What commit behaviour should we choose?
>

I still like mine. :)


> Map and unmap
>
> Mapping and unmapping a sub-surface follows the normal wl_surface
> mapping rules, subject to the sub-surface commit behaviour, and
> conditional to the parent surface: a sub-surface can be mapped only if
> the parent surface is.
>
> Would we need explicit map/unmap requests? It might depend on the
> sub-surface commit behaviour with some corner cases, but I don't think
> we need them.
>

You're right - we can use set_position instead of explicit map/unmap
requests.  Much cleaner.


> Sub-surface configuration/geometry control
>
> All operations about positioning and sizing sub-surfaces take place in
> the parent surface's coordinate frame, and are limited to integer
> pixels. No fractional pixels are allowed, and no transformations apart
> from translation and scaling(?) are supported.
>
> We probably need a wl_subsurface.set_position(x, y) request. The
> initial position could be defined to be 0, 0. The position is affected
> by wl_surface.attach request's x,y parameters, as usual.
>
> The size of a normal wl_surface if defined by the wl_buffer last
> attached to it. This might be insufficient for sub-surfaces, since it
> does not allow using the scaling features of hardware overlays.
>
> Should we have a request, that can set a different size for a
> sub-surface, apart from the wl_buffer size?
> If we did, it should probably be double-buffered state like everything
> else, and follow the sub-surface commit behaviour.
>

Yep.  I think in addition to set_position, we also want a configuration
request specifying a source rect within the wl_buffer and a destination
rect, overriding the size given in the wl_buffer.  This could just go in
core wl_surface, allowing scaling for normal surfaces too.
 wl_surface_set_size or similar?


> What if the wl_buffer size changes without a corresponding sub-surface
> size change request? Do we just scale to the set surface size
> regardless of the buffer size? How would it interact with the
> sub-surface commit behaviour?
>

In that case we should be defaulting to unscaled, i.e. reset both source
and dest sizes to (0, 0) -> (buffer.w, buffer.h).


>     <request name="set_position">
>       <arg name="x" type="int"/>
>       <arg name="y" type="int"/>
>     </request>
>

If we want to enforce clipping and we have the set_size request as above,
then we should make these uint.


>     <request name="set_size">
>       <arg name="width" type="int"/>
>       <arg name="height" type="int"/>
>     </request>
>

I'd do this as:
  <arg name="src_x" type="uint"/>
  <arg name="src_y" type="uint"/>
  <arg name="src_w" type="uint"/>
  <arg name="src_h" type="uint"/>
  <arg name="dst_w" type="uint"/>
  <arg name="dst_h" type="uint"/>


> What should happen on wl_subsurface.get_subsurface if:
> - the parent is already a sub-surface itself?
>

I think this should be OK.  The easy case here is ARGB decorations,
containing YUV video, containing ARGB video controls.  A reasonable number
of embedded/media chips actually support this exact usecase in hardware,
and it's common enough that I think we should allow for it.  Not only does
it mean you don't have to stall your entire media pipeline to blend the
controls and the video, but it means you can have hardware-scaled video
with controls rendered at native resolution.


> - the surface is already a sub-surface of a different parent?
> - the surface is already a sub-surface of the same parent?
> - the surface has already been given a role, like pointer image or
>   top-level window?
>

These should be disallowed, I suspect.


> Should these be fatal errors, non-fatal errors, or allowed use?
> Non-fatal errors would be signalled with an additional event in
> wl_subsurface, that says it is now inert due to an error.
>
> I have not even thought about sub-surfaces' implications to input
> handling or the shell yet. Sub-surfaces probably need to be able to
> receive input. The shell perhaps needs a bounding box of the set of
> surfaces to be able to pick an initial position for the window, etc.
>

This is why I'm kind of uneasy about having children be able to wander
outside the parent's bounding box ... in this case it definitely becomes a
group rather than parent/child.

tl;dr: If we don't clip to the parent's bounding box then we shouldn't call
it parent/child subsurfaces but a group instead; having a wl_surface method
to clip and scale would be great; I think we shouldn't require parent
commits when only the buffer content, not configuration, has changed.

Cheers,
Daniel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20121212/bd3ba4b4/attachment-0001.html>