[RFC weston 13/16] compositor: Add a function to test if images transformed by a matrix should be bilinearly filtered

[Derek Foreman]

On 03/10/14 02:04 AM, Pekka Paalanen wrote:
> On Thu, 02 Oct 2014 17:21:04 -0500
> Derek Foreman <derekf at osg.samsung.com> wrote:
> 
>> On 02/10/14 02:37 PM, Jason Ekstrand wrote:
>>>
>>> On Oct 2, 2014 12:37 AM, "Pekka Paalanen" <ppaalanen at gmail.com
>>> <mailto:ppaalanen at gmail.com>> wrote:
>>>>
>>>> On Wed, 1 Oct 2014 18:09:32 -0700
>>>> Jason Ekstrand <jason at jlekstrand.net <mailto:jason at jlekstrand.net>> wrote:
> 
>>>> (Total transformation is between buffer pixel coords and output/scanout
>>>> pixel coords, i.e. buffer-to-output.)
>>
>> btw, what exactly is the buffer-to-output transform?  I think in the
>> pixman renderer that's already calculated in a convenient location (in
>> "matrix" in repaint_region()
> 
> Yes, like Jason already replied. It is from pixel coordinates in buffer
> to pixel coordinates in output. Unnormalized. Raw in the sense that you
> can theoretically use them directly with pointer arithmetic to address
> pixels.
> 
>> For gl-renderer, I suspect I need to build it myself in draw_view():
>> weston_matrix_init(&foo);
>> weston_matrix_multiply(&foo, &ev->surface->buffer_to_surface_matrix);
>> if (ev->transform.enabled)
>> 	weston_matrix_multiply(&foo, &ev->transform.matrix)
>> weston_matrix_multiply(&foo, &output->matrix);
>>
>> Is that right?  Do I have the order backwards?
>>
>> I'd like to test just that one matrix and no additional if
>> (ev->tranform.enabled) etc to decide on whether to use linear or nearest...
> 
> I can't bother to think far enough to say anything about the order
> right now, but all this is what Weston core should be computing when
> you're done (maybe a helper function called from renderers). So the
> generic computation would be moved from pixman to core, and then only
> pixman specifics are left in pixman renderer.

The really lovely part about pixman is that (if I'm "getting" it
correctly) you transform the source image instead of transforming
destination vertices.  So pixman needs things exactly backwards.

> For the gl-renderer this means much bigger changes: the renderer
> receives a pixels-to-pixels transformation matrix which is the
> canonical form, and then it needs to adapt it to the normalized GL
> coordinate system. You have various options on how to implement that:
> do you compute all coordinates on CPU and feed both vertex and texture
> coords to GL, or do you feed a matrix and either vertex or texture
> coords to GL? I can't say off-hand what is best, but I think you'll
> know when writing the code. (Maybe you already did? Sorry, I still
> haven't looked at the patches.)

I think Jason did that change before I came along - the projection
matrix for the GL renderer is no longer output->matrix, it's now
output->matrix translated and scaled (so output->matrix can be in pixels).

The whole surface -> <magic> -> buffer chain is split in two for the
gl-renderer (part of it is on the projection matrix, part of it comes in
vertices), and inverted for pixman. :)

I'll see if I can think of a way to unify these a little more.  leaving
only the pixels->glspace transformation on the projection matrix might
help...

>>> I'm sorry I mistyped but I meant the transform to be an output
>>> parameter.  That way you know of the matrix is a 90-degree rotation or
>>> flip.  Not sure if this is needed but for figuring out GL_LINEAR vs
>>> GL_NEAREST we don't want to fail if there is a 90-degree rotation.
>>>>
>>>>> (do we use "bool" in weston?  Maybe just return int).  We may need
>>
>> I'm kind of interested in the answer to the question "do we use bool in
>> weston?" - It's used in some places and not others - do we care? :)
> 
> Hmm... it's used in so many optional places, that I guess we can just
> use it also in the core.
> 
> Just make sure the semantics are logical. A test function of the form
> foo_is_bar() or is_foo() can return bool and it makes sense, but an
> imperative function usually returns success/failure. If success/failure
> is returned as int, we use -1 for failure and 0 for success, which is
> the opposite of bool.

I'm of the opinion that 0 for success and non-zero for error is more
useful when you're returning distinct error conditions...

> Except the places where we use int return with 1=success and 0=fail...
> 
> After a pause, I always need to think three times what the return value
> for weston_output_repaint() means. :-P
> 
> All I'm saying is that an imperative function (do_something())
> returning bool vs. returning int can be easily confused...
>
> Eh, cleanups to bring consistency would be appreciated. And first
> determining what the common convention actually is. ;-)

Sure. :)

>>> both x
>>>>> and y scales and it may be useful to get those as floats.  I'm not
>>> sure on
>>>>> that.  Pekka, what would the RPi backend use?
>>>>
>>>> The rpi-renderer uses pretty much the same as what DRM planes/overlays
>>>> offer wrt. coordinates, IIRC: integer position and size on the output,
>>>> and 16.16 fixed point position and size on input (buffer).
>>>>
>>>> Whether scaling factor is integer or not is irrelevant there. I do not
>>>> recall there being an option for sampling (nearest/linear/...) in
>>>> either DispmanX nor DRM.
>>
>> I think rpi has horizontal and vertical flip capabilities as well?
>>
>> I don't think that's exposed by drmModeSetPlane.
> 
> You clip by setting the source rectangle to smaller than the buffer.

Ahem.  Flip.  Not clip.  :D

> extern int drmModeSetPlane(int fd, uint32_t plane_id, uint32_t crtc_id,
>                            uint32_t fb_id, uint32_t flags,
>                            uint32_t crtc_x, uint32_t crtc_y,
>                            uint32_t crtc_w, uint32_t crtc_h,
>                            uint32_t src_x, uint32_t src_y,
>                            uint32_t src_w, uint32_t src_h);
> 
> Thanks,
> pq
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>