[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.
> 
> 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'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.
> 
> 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. ;-)

I've sent a patch that just does the 1/0 test_something() stuff...

Looks like there's a lot of -1/0, but there's also some -1/1 lying around.

And even a few -1/fd where it obviously makes sense to keep using int.

And some 1/0 (like weston_compositor_run_axis_binding) that doesn't have
the test_something() look to its function name.

Some 1/0 stuff is in callbacks for libinput and the like too.

Not sure where to go from here.  :)