[PATCH v5 xserver 5/7] xf86Cursor: Fix xf86_crtc_rotate_coord_back using width/height wrongly

[Michel Dänzer]

On 08/09/16 02:32 PM, Hans de Goede wrote:
> On 08-09-16 03:10, Michel Dänzer wrote:
>> On 08/09/16 12:43 AM, Hans de Goede wrote:
>>>
>>> xf86_crtc_rotate_coord_back() is described as "Given a cursor source
>>> coordinate, rotate to a screen coordinate" which is exactly what we
>>> need to properly position the cursor on a slave output, we start with
>>> cursor coordinates and need to adjust those for the screen's rotation.
>>
>> No, transform_(gpu_)cursor_position[0] take screen space coordinates and
>> transform them to CRTC space.
> 
> Ah, ok. I'm not all that familiar with this code, so I'm going to take your
> word for this.

I was hoping my explanation would be clear enough that you don't have to
take my word. :}


> I'll do a new patch fixing xf86_crtc_rotate_coord instead of
> xf86_crtc_rotate_coord_back, and also fix patch 6/7 to do things
> differently.

Thanks.


>> [0] BTW, do we really need two functions? The method used in
>> xf86_crtc_transform_gpu_cursor_position should work for non-GPU screens
>> as well, doesn't it?
> 
> That uses crtc->f_framebuffer_to_crtc which gets setup by xf86CrtcRotate(),
> which at the very top has:
> 
>     if (pScreen->isGPU)
>         return TRUE;
> 
> Which is why I went the route I went. I'll see if instead I can safely
> modify xf86CrtcRotate() to still setup crtc->f_framebuffer_to_crtc, without
> it having any side-effects.

What I mean is making xf86_crtc_transform_cursor_position use the same
method as xf86_crtc_transform_gpu_cursor_position in your patch, instead
of using crtc->f_framebuffer_to_crtc. Then it should be usable for both
cases?


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer