<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, May 2, 2013 at 5:51 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>On Tue, 30 Apr 2013 10:54:25 -0500<br>
Jason Ekstrand <<a href="mailto:jason@jlekstrand.net" target="_blank">jason@jlekstrand.net</a>> wrote:<br>
> On Tue, Apr 30, 2013 at 9:06 AM, John Kåre Alsaker <<br>
> <a href="mailto:john.kare.alsaker@gmail.com" target="_blank">john.kare.alsaker@gmail.com</a>> wrote:<br>
> > I'd say we should split the cropping and scaling request into two.<br>
><br>
> How would you suggest doing that? I actually really like the<br>
> simplicity of this box maps to that box.<br>
<br>
</div>The boxes approach also does not have rounding issues. The<br>
destination size will be exactly what you specify, not some vague<br>
number multiplied by number, then rounded somehow.<br></blockquote><div><br></div><div>Agreed. Sending transform matrices or the like has HUGE rounding problems. Particularly when we're using wl_fixed which is 24.8. Other methods would require adding rounding conventions etc.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
> > Specifying a scaling with a NULL buffer should still set the<br>
> > surface size, so we can have surfaces with only an input region.<br>
> ><br>
><br>
> I really don't think this is the place for that. I want input-only<br>
> surfaces too, but I don't think this is the place for that<br>
<br>
</div>I also don't think this is the place for that. If any...<br>
<div><br>
> > I don't see a way to disable scaling and cropping, could passing 0<br>
> > as width and height do that?<br>
> ><br>
><br>
> Can't the client just set it to 0, 0, width, height, width, height<br>
> easily enough? Or just destroy the scaler?<br>
<br>
</div>Yeah, that's two ways to disable scaling and cropping, should be<br>
enough. The first one just has to be used repeatedly, if the buffer<br>
size changes. Destroying the surface_scaler returns the original 1:1<br>
mapping.<br>
<div><div><br>
> On Tue, Apr 30, 2013 at 2:33 PM, Pekka Paalanen <<a href="mailto:ppaalanen@gmail.com" target="_blank">ppaalanen@gmail.com</a>><br>
> wrote:<br>
><br>
> > Hi all,<br>
> ><br>
> > below is my first draft for a wl_surface scaling and cropping<br>
> > extension. I called it wl_scaler in the lack of a better name. It is<br>
> > designed similarly to the other wl_surface extensions<br>
> > wl_shell_surface and wl_subsurface.<br>
> ><br>
> > There probably isn't any interesting details to debate, right? ;-)<br>
> ><br>
> > I'd like to have a better name for it, and you might want the set<br>
> > request split into two or three, or not, but otherwise I'm quite<br>
> > satisfied with it. I tried to take into account all existing<br>
> > protocol that interacts with this.<br>
> ><br>
> > Comments? Is the language clear?<br>
> ><br>
> ><br>
> > Thanks,<br>
> > pq<br>
> ><br>
> ><br>
> > <?xml version="1.0" encoding="UTF-8"?><br>
> > <protocol name="scaler"><br>
> ><br>
> > <copyright><br>
> > Copyright © 2013 Collabora, Ltd.<br>
> ><br>
> > Permission to use, copy, modify, distribute, and sell this<br>
> > software and its documentation for any purpose is hereby granted<br>
> > without fee, provided that the above copyright notice appear in<br>
> > all copies and that both that copyright notice and this<br>
> > permission notice appear in supporting documentation, and that the<br>
> > name of the copyright holders not be used in advertising or<br>
> > publicity pertaining to distribution of the software without<br>
> > specific, written prior permission. The copyright holders make no<br>
> > representations about the suitability of this software for any<br>
> > purpose. It is provided "as is" without express or implied<br>
> > warranty.<br>
> ><br>
> > THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO<br>
> > THIS SOFTWARE, INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY<br>
> > AND FITNESS, IN NO EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR<br>
> > ANY SPECIAL, INDIRECT OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES<br>
> > WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER<br>
> > IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,<br>
> > ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF<br>
> > THIS SOFTWARE.<br>
> > </copyright><br>
> ><br>
> > <interface name="wl_scaler" version="1"><br>
> > <description summary="surface cropping and scaling"><br>
> > The global interface exposing surface cropping and scaling<br>
> > capabilities is used to instantiate an interface extension<br>
> > for a wl_surface object. This extended interface will then allow<br>
> > cropping and scaling the surface contents, effectively<br>
> > disconnecting the 1:1 relationship between the buffer and the<br>
> > surface size.<br>
> > </description><br>
> ><br>
> > <request name="destroy" type="destructor"><br>
> > <description summary="unbind from the cropping and scaling<br>
> > interface"><br>
> > Informs the server that the client will not be using this<br>
> > protocol object anymore. This does not affect any other<br>
> > objects, wl_surface_scaler objects included.<br>
> > </description><br>
> > </request><br>
> ><br>
> > <enum name="error"><br>
> > <entry name="scaler_exists" value="0"<br>
> > summary="the surface already has a scaler object<br>
> > associated"/> </enum><br>
> ><br>
> > <request name="get_surface_scaler"><br>
> > <description summary="extend surface interface for crop and<br>
> > scale"> Instantiate an interface extension for the given wl_surface<br>
> > to crop and scale its content. If the given wl_surface already has<br>
> > a wl_surface_scaler object associated, the scaler_exists<br>
> > protocol error is raised.<br>
> > </description><br>
> ><br>
> > <arg name="id" type="new_id" interface="wl_surface_scaler"<br>
> > summary="the new scaler interface id"/><br>
> > <arg name="surface" type="object" interface="wl_surface"<br>
> > summary="the surface"/><br>
> > </request><br>
> > </interface><br>
> ><br>
> > <interface name="wl_surface_scaler" version="1"><br>
> > <description summary="crop and scale interface to a wl_surface"><br>
> > An additional interface to a wl_surface object, allowing<br>
> > cropping and scaling of the surface contents.<br>
> ><br>
><br>
> "allows you" or "allows the client" would be better here<br>
<br>
</div></div>I changed the paragraph to:<br>
<br>
" An additional interface to a wl_surface object, which allows the<br>
client to specify the cropping and scaling of the surface<br>
contents."<br></blockquote><div><br></div><div>Looks Good!<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> > This interface allows to define the source rectangle (src_x,<br>
> > src_y, src_width, src_height) from where to take the wl_buffer<br>
> > contents, and scale that to destination size (dst_width,<br>
> > dst_height). This state is double-buffered, and is applied on<br>
> > the next wl_surface.commit.<br>
> ><br>
> > Before the first set request, the wl_surface still behaves as<br>
> > if there was no crop and scale state. That is, no scaling is<br>
> > applied, and the surface size is the buffer size.<br>
> ><br>
> > On compositing, source rectangle coordinates are evaluated<br>
> > after wl_surface.set_buffer_transform is evaluated. This means that<br>
> > changing the buffer transform and correspondingly the client<br>
> > rendering does not require sending new source rectangle<br>
> > coordinates to keep the exact same image source rectangle. In<br>
> > other words, the source rectangle is given in the<br>
> > not-scaled-and-cropped surface coordinates, not buffer data<br>
> > coordinates.<br>
> ><br>
><br>
> I agree with Zhi, this needs to be re-worded<br>
<br>
</div>Yeah, did you understand what I was trying to explain? Any suggestions?<br><div></div></blockquote><div><br></div><div>If I understood correctly, you meant the sensible thing. i.e., buffers are in (possibly transformed) buffer coordinates while surfaces are in surface coordinates. In other words, the crop/scale is applied after the buffer transform. How about this (actually a paragraph higher):<br>
<br></div><div>The source rectangle is specified in (possibly transformed) buffer coordinates. This means that changing the buffer transform and correspondingly the client rendering does not require sending new source rectangle coordinates. In other words, the source rectangle is given in the coordinates that the surface would have without the scaled-and-cropped transformation.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>
> > The crop and scale state causes the surface size to become<br>
> > dst_width, dst_height. This overrides whatever the attached<br>
> > wl_buffer size is, unless the wl_buffer is NULL.<br>
> ><br>
> > The surface-local coordinate system is tied to the space of<br>
> > dst_width and dst_height. That is, surface-local coordinates<br>
> > are relative to the scaled and cropped surface, not the wl_buffer<br>
> > providing the content.<br>
> ><br>
> > If the source rectangle is partially or completely outside of<br>
> > the wl_buffer, then the surface contents are undefined, and the<br>
> > surface size is still dst_width, dst_height.<br>
> ><br>
> > The x, y arguments of wl_surface.attach are applied as normal<br>
> > to the surface. They indicate how many pixels to remove from the<br>
> > surface size from the left and the top. In other words, they<br>
> > are still in the surface-local coordinate system, just like<br>
> > dst_width and dst_height are.<br>
> ><br>
<br>
</div>This above paragraph...<br>
<div><div><br>
> > If the wl_surface associated with the wl_surface_scaler is<br>
> > destroyed, the wl_surface_scaler object becomes inert.<br>
> ><br>
> > If the wl_surface_scaler object is destroyed, the crop and<br>
> > scale state is removed from the wl_surface. The change will be<br>
> > applied on the next wl_surface.commit.<br>
> > </description><br>
> ><br>
> > <request name="destroy" type="destructor"><br>
> > <description summary="remove scaling and cropping from the<br>
> > surface"> The associated wl_surface's crop and scale state is<br>
> > removed. The change is applied on the next wl_surface.commit.<br>
> > </description><br>
> > </request><br>
> ><br>
> > <enum name="error"><br>
> > <entry name="bad_value" value="0"<br>
> > summary="negative values in width or height"/><br>
> > </enum><br>
> ><br>
> > <request name="set"><br>
> > <description summary="set the crop and scale state"><br>
> > Set the crop and scale state of the associated wl_surface.<br>
> > See wl_surface_scaler for the description, and relation to the<br>
> > wl_buffer size.<br>
> ><br>
> > If any of src_width, src_height, dst_width, and dst_height<br>
> > is negative, the bad_value protocol error is raised.<br>
> > </description><br>
> ><br>
> > <arg name="src_x" type="fixed" summary="source rectangle x"/><br>
> > <arg name="src_y" type="fixed" summary="source rectangle y"/><br>
> > <arg name="src_width" type="fixed" summary="source rectangle<br>
> > width"/> <arg name="src_height" type="fixed" summary="source<br>
> > rectangle height"/><br>
> > <arg name="dst_width" type="int" summary="surface width"/><br>
> > <arg name="dst_height" type="int" summary="surface height"/><br>
> > </request><br>
> ><br>
> > </interface><br>
> > </protocol><br>
> ><br>
><br>
> There's only one gaping hole that I see here and that is attach<br>
> behaviour. Currently, wl_surface.attach takes an (x, y) position<br>
> argument telling the compositor where to place the new buffer<br>
> relative to the old one. How is this going to interact with this<br>
> extension?<br>
<br>
</div></div>...should explain the interaction precicely. Is it not understandable?<br>
<br>
I explained it via adding/removing pixel rows or columns, as that<br>
is the fundamental intention. Moving the surface is just a consequence.<br><div></div></blockquote><div><br></div><div>Ok, I see it now. Sorry, but I missed it on my first read-through. Yes, it fixes the problem, but in an extremely confusing way. The reason I say it is confusing is because it inherently mixes buffer and surface coordinate systems. I really think we need to isolate buffer coordinates from surface coordinates more. Perhaps what we need is two requests: set_source_rect and set_dest_rect and completely ignore the x and y from attach. This both provides clarity to the coordinate systems and provides a little separation between crop and scale.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
> I think the problem here is the same one that we've seen before:<br>
> Surfaces inherently take their size etc. from the buffer attached to<br>
> them. While this works great in the "single image exactly the size<br>
> it should be on screen" case, I'm not sure it's working well when we<br>
> go beyond that. Unfortunately, I'm not 100% sure how to solve it.<br>
<br>
</div>I hope I managed to solve it. :-)<br>
<br>
However, my proposed solution relies on the wl_surface_scaler interface<br>
being controlled by the same entity who renders and commits the<br>
wl_buffers. If it is not supposed to be the same entity (library,<br>
component), then we have more problems than just defining what<br>
attach(x,y) does.<br>
<br>
What I mean is, that this extension is not designed for an independent<br>
component to force the size of another component's (sub-)surface<br>
behind its back. wl_surface_scaler interface and wl_surface.attach<br>
request must be used together for coherent and fully controlled<br>
results. This idea also makes sampling from outside of a buffer just a<br>
silent, non-fatal client error, rather than trying to come up with a<br>
scheme to transparently fix such mismatches.<br></blockquote><br></div><div class="gmail_quote">Agreed. We don't want parents trying to use it to force their children unless their children are explicitly forceable (i.e. constant-size video surface).<br>
</div><br></div></div>