[PATCH weston 0/8] wl_viewport enhancements

Thu Mar 20 12:17:25 PDT 2014

On Thu, Mar 20, 2014 at 8:04 AM, Pekka Paalanen <ppaalanen at gmail.com> wrote:

> On Thu, 20 Mar 2014 07:22:33 -0500
> Jason Ekstrand <jason at jlekstrand.net> wrote:
>
> > Hi Pekka,
> > I've been meaning to get around to this one.  Sorry it took me so long.
>
> Hi Jason,
>
> no worries.
>
> > On Mar 20, 2014 2:46 AM, "Pekka Paalanen" <
> pekka.paalanen at collabora.co.uk>
> > wrote:
> > >
> > > On Fri, 14 Mar 2014 14:38:10 +0200
> > > Pekka Paalanen <ppaalanen at gmail.com> wrote:
> > >
> > > > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> > > >
> > > > Hi,
> > > >
> > > > this series replaces the first 5 patches from
> > > >
> >
> http://lists.freedesktop.org/archives/wayland-devel/2014-March/013580.html
> > > >
> > > > Compared to the old series, this series carries the same 5 patches
> > > > rebased on top of the current master branch. It adds a tiny fix
> > > > (patch 4).
> > > >
> > > > The big thing added in this series is patch 7, which adds two new
> > > > wl_viewport requests for setting the source rectangle and destination
> > > > size separately. This will be needed by the Presentation extension,
> > > > which classifies source rectangle as buffer state and destination
> size
> > > > as surface state, hence going through different commit paths. Clients
> > > > will need a way to set these separately.
> > > >
> > > > We now also define what it means if source rectangle is set, but
> > > > destination size is not set, and vice versa.
> > > >
> > > > This patch series does not yet change the wl_surface.commit
> behaviour.
> > > >
> > > > When wl_scaler/wl_viewport eventually migrates to Wayland, my
> intention
> > > > is to drop the wl_viewport.set request and the error codes. I guess
> > > > this would also need to rename the global interface, which gives us
> > > > the opportunity to change the interface style from factory to
> > > > something else, if we want.
> > > >
> > > > Pekka Paalanen (8):
> > > >   compositor: refactor more into weston_surface_attach
> > > >   compositor: buffer can be non-NULL only if newly_attached
> > > >   compositor: refactor code into
> weston_surface_reset_pending_buffer()
> > > >   compositor: use surface_set_size() in one more case
> > > >   compositor: reorganize struct weston_buffer_viewport
> > > >   compositor: replace weston_buffer_viewport::viewport_set
> > > >   protocol,compositor: split wl_viewport setters
> > > >   clients/scaler: add modes to test wl_scaler version 2
> > > >
> > > >  clients/scaler.c      | 143 +++++++++++++++++++++++++++++-----
> > > >  desktop-shell/shell.c |   6 +-
> > > >  protocol/scaler.xml   |  77 +++++++++++++++----
> > > >  src/compositor-drm.c  |  12 +--
> > > >  src/compositor.c      | 208
> > ++++++++++++++++++++++++++++++++++----------------
> > > >  src/compositor.h      |  31 +++++---
> > > >  src/gl-renderer.c     |   2 +-
> > > >  src/pixman-renderer.c |  59 ++++++++------
> > > >  8 files changed, 400 insertions(+), 138 deletions(-)
> > >
> > > Hi,
> > >
> > > anyone else than Bill got any comments?
> > >
> > > Would you agree with Bill that 0x0 source rect should not be legal?
> > > Or if we should raise an error on negative sizes rather than just take
> > > it as "disable"?
> >
> > As far as 0x0 goes, I don't see why we would want to allow 0-sized
> > surfaces.  As a client, I would expect the compositor to sample exactly
> the
> > rectangle I gave it and no further.  If I wanted to sample just one
> pixel,
> > I would give it a 1x1 source, not a 0x0.  Also, making both require > 0
> > would add some nice symmetry.  That said, I'm not going to be insistant
> on
> > the point.
>
> Ok, I should remember to change that when I revisit this again.
>
> The sampling really goes into hair-splitting. It depends on how you
> interpret a texel image; is each pixel a solid-colored tile, or does
> the color vary smoothly from texel to the next. Then you have the
> source rectangle, which is divided into dst_width x dst_height pixels
> (let's assume output_scale=1). For each pixel, do you take a point
> sample from its middle, or do you integrate an average over the pixel's
> area in the source texture. In any case, I see that point sampling is
> well-defined always, and a theoretical prerequisite for the integration
> since we can and will sample "between texels". In that respect, a 0x0
> source rectangle would just mean that all pixels will be point samples
> at src_x,src_y.
>

Ok, I think I see what you're getting at: what happens if you upscale a
single pixel?  I'm not 100% sure what to say here.  Part of me is inclined
to say that you treat the crop as a hard crop and don't use anything
outside of the specified rectangle.  This would mean a 1x1 crop is is
always a solid color.  However, I can also see where this requirement may
not always make sense and, for the benefit of implementers with strange
hardware to work on, we probably don't want to specify this too closely.
If compositors are going to do some sort of bilinear sampling and you're
upscaling (so it may go outside of the middles of the source pixels), I
guess allowing 0x0 source makes some sense.  I have no idea if that's going
to cause hardware problems anywhere or not.  Another option would be to say
negative removes the source rectangle and leave 0x0 undefined for now.

I guess the short version of what I'm trying to say is that if someone is
going to use both crop and scale together, something funny is liable to
happen at the edges.  I don't think we can get around it.  If you want to
guarantee edge behaviour, don't crop.

Thanks,
--Jason Ekstrand

> So, better forget that and do what seems natural. The minimum sampling
> area (source rect) will be 1/256 x 1/256 texels per surface pixel,
> then. Output_scale would make that respectively smaller, too.
>
> > As far as negative size -> disable goes, I like it.  We need some way of
> > disabling them, and that works fine.  You could make an argument about
> how
> > we should send an actual error and kill the client, but I think just
> > turning off crop-and-scale is sufficient.
>
> Cool.
>
> Kristian, there are no changes planned for patches 1-6, so those should
> be good to go.
>
>
> Thanks,
> pq
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140320/95dbff15/attachment.html>