[PATCH wayland-protocols 1/2] stable: add viewporter draft

[Pekka Paalanen]

On Mon, 18 Apr 2016 15:16:39 +0100
Daniel Stone <daniel at fooishbar.org> wrote:

> Hi,
> 
> On 15 April 2016 at 15:53, Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > +      This interface allows to define the source rectangle (src_x,
> > +      src_y, src_width, src_height) from where to take the wl_buffer
> > +      contents, and scale that to destination size (dst_width,
> > +      dst_height). This state is double-buffered, and is applied on the
> > +      next wl_surface.commit.  
> 
> I like Yong's rewording, except with an additional bikeshed of 'allows
> the client to control the [...]' perhaps.

I ended up rewriting it too, please see if it's better in my reply to
Yong.

> > +      If the source rectangle is set, it defines what area of the
> > +      wl_buffer is taken as the source. If the source rectangle is set and
> > +      the destination size is not set, the surface size becomes the source
> > +      rectangle size rounded up to the nearest integer. If the source size
> > +      is already exactly integers, this results in cropping without scaling.  
> 
> Ugh, I really intensely dislike that middle section. The cropping and
> scaling are completely independent ... except where they aren't,
> because sometimes we'll magically do it for you. I'd very much prefer
> to see fractional crop + no scale defined as a protocol error, i.e.
> the user must always explicitly set a configuration that results in an
> integral destination size.

Ok, so it is an error to have destination size unset and use
non-integer source size. However, integer source size is allowed.

I'll change that and add a error code for it.

> > +      If the source rectangle is partially or completely outside of the
> > +      wl_buffer, then the surface contents are undefined (not void), and
> > +      the surface size is still dst_width, dst_height.  
> 
> Again, I'd rather see this defined as a protocol error. Without
> explicit border-colour/mode controls (e.g. 'sample this colour for
> anything outside buffer bounds'), the result of sampling outside the
> source image can never be useful (at least AFAICT). So, why would you
> ever want to do so? To achieve more perfect scaling factors, at the
> expense of having to display some fractional garbage?

Yup, I'll add an error code for it and make it an error.

> > +      If the wl_surface associated with the wl_viewport is destroyed,
> > +      the wl_viewport object becomes inert.  
> 
> Should attempting to call wl_viewport::set_* then result in an error?

Inert follows the example of wl_subsurface. I'll make this an error,
too. There is no reason to assume that different code modules would be
in charge of the wp_viewport and wl_surface, unlike with sub-surfaces.

> > +       Arguments dst_x and dst_y do not exist here, use the x and y
> > +       arguments to wl_surface.attach.  
> 
> The x and y arguments perform relative translation of the surface, and
> as such I don't think are really analagous. I'd suggest just deleting
> this to avoid confusion.

Ok. This sentence was here to silence any questions about "how do I
control where the destination rectangle goes on the wl_surface?" - a
question that doesn't make sense on closer inspection.

> > +    <request name="set_source" since="2">
> > +      <description summary="set the source rectangle for cropping">
> > +       Set the source rectangle of the associated wl_surface. See
> > +       wl_viewport for the description, and relation to the wl_buffer
> > +       size.
> > +
> > +       If width is -1.0 and height is -1.0, the source rectangle is unset
> > +       instead. Any other pair of values for width and height that
> > +       contains zero or negative values raises the bad_value protocol
> > +       error.  
> 
> This is different to the bare 'set' request, which doesn't (at least
> in its documentation) allow for this. But that's being removed anyway,
> so no problem. Should we require a pair of fixed values for x and y
> here, either (0,0) as the state they'll be fixed to, or (-1,-1) for
> symmetry?

Right, these are new requests that supersede 'set'.

(-1, -1) for x,y would make sense. They are illegal values otherwise,
so they obviously do something special. So rather than ignore x,y for
unsetting, I will require them to be -1.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160419/71c81ce9/attachment-0001.sig>