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

Michel Dänzer michel at daenzer.net
Thu Sep 8 07:24:30 UTC 2016


On 08/09/16 04:18 PM, Hans de Goede wrote:
> On 08-09-16 07:59, Michel Dänzer wrote:
>> 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:
>>>>>
>>>> [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.
> 
> I see, yes that is an interesting idea.
> 
>> Then it should be usable for both cases?
> 
> Well crtc->f_framebuffer_to_crtc also takes crtc->transform into account if
> not NULL, so your suggested change would change the behavior of
> xf86_crtc_transform_cursor_position in that case. Now with that said,
> xf86_use_hw_cursor() does contain:
> 
>         if (crtc->transformPresent)
>             return FALSE;
> 
> So if a transform is in use we should never end up in
> xf86_crtc_transform_cursor_position, so your suggestion should probably
> work.

Exactly.


> But I'm afraid that despite that it might still cause regressions
> for non slave outputs, so I'm going to play it safe and keep the
> separate function for now. We could try switching entirely to the new
> version at the start of the next xserver devel cycle.

What's the problem with testing non-slave outputs? If you can't or don't
want to do it, I can.


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


More information about the xorg-devel mailing list