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