[RFC v2] surface crop & scale protocol extension

[Pekka Paalanen]

On Mon, 11 Nov 2013 10:47:47 -0800
Bill Spitzak <spitzak at gmail.com> wrote:

> Pekka Paalanen wrote:
> 
> > Or would you rather have src_x, src_y, src_width, src_height always
> > defined in buffer coordinates, and then buffer_transform and
> > buffer_scale (with their requirement of size being a
> > multiple of buffer_scale) applied to dst_width and dst_height instead?
> > 
> > In the latter case, the surface size would be affected by all of
> > buffer_transform, buffer_scale, dst_width and dst_height.
> 
> The source rectangle *must* be in buffer pixels. Putting it in 
> buffer_scale units makes absolutely no sense, as the buffer_scale is in 
> effect ignored for this surface, overridden entirely by the scaling.

That means that dst_width and dst_height will be required to be in
multiples of buffer_scale.

In the original proposal however, a client can simply choose to set
buffer_scale=1 regardless of output_scale, and so use buffer-pixel
units in the src rectangle (although buffer_transform will still affect
it). You understood right, buffer_scale is in effect ignored.

> It does not matter whether buffer_transform is applied first, as it does 
> not move where integers are.

But it does matter? Buffer_transform rotates in 90-deg steps, and can
do a flip. Using a 20x20 area at 0,0 from a 100x100 buffer, if
buffer_transform is applied before crop & scale then buffer_transform
will decide from which corner it actually is.

If you were talking only about avoiding fractional units, then yes,
buffer_transform always keeps integers as integers.

> Also if integers are used it is impossible to scale an image that is not 
> a multiple of buffer_scale in both dimensions. Using fixed for the 
> source rectangle works around this problem but is very misleading.

That can be remedied by simply choosing buffer_scale=1 in the original
proposal to make the src image always a multiple of buffer_scale.

I agree, that using wl_fixed_t does not solve this problem. Setting
buffer_scale=1 does.

> At the moment it sounds like the destination has to be in buffer_scale 
> units, rather than in pixels, in order to match the rest of the Wayland 
> api. I think this is a mistake but it should be fixed everywhere, not 
> just here.

In the original proposal, dst_width and dst_height are in surface
coordinate units and not restricted to multiples of buffer_scale. They
are just any positive integers.

In the "src rect in buffer pixel units" suggestion, dst_width and
dst_height must be multiples of buffer_scale, so that they can be
divided by buffer_scale to get the surface dimensions in surface
coordinate units.

(Surface coordinate units and coordinate frame are the one used by
almost all of the Wayland protocols: input, child surface
positioning, ...)

> > In the end, I see no other difference between the transformation orders
> > than convenience: calculations you might be able to avoid, some lines
> > of code you can skip writing. The same piece of code has to control both
> > the wl_surface interface and the wl_surface_scaler interface anyway.
> 
> In theory yes, but unfortunately in the real world there is finite 
> precision in our representation of numbers, making the differences 
> relevant and important.

The only place that may require rounding is encoding the information
into the protocol requests. Everything else can be computed exactly by
using ratios of integers, given exact input values.

***

There is a third option: specify that when crop & scale state is set,
buffer_scale gets ignored. I really don't know if this would better or
worse for future-proofing, but a new extension that disables state that
was used in the past feels a bit strange here. And disabling
buffer_scale would not be explicitly visible in the protocol, it would
be a rule that you just have to know, if you set crop & scale.

In this particular case, we have seemingly equal impacts whether to go
with the original proposal or disable buffer_scale. In the original
proposal clients can effectively disable buffer_scale by setting it to
one (which is the default value anyway, so no need to explicitly set
it).

Therefore I would not propose the third option, but I'm mentioning it
for completeness.


In my opinion, buffer_scale is not an issue either way. The issue is
buffer_transform. Does buffer_transform apply to the buffer, or to the
cut-out rectangle?

Even the name "buffer_transform" implies that it applies to the buffer,
just like it originally does.


Thanks,
pq