[PATCH weston 0/8] wl_viewport enhancements

[Pekka Paalanen]

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.

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