On 08/09/16 12:43 AM, Hans de Goede wrote: > > /* > * Given a screen coordinate, rotate back to a cursor source coordinate > */ > xf86_crtc_rotate_coord(...) > > /* > * Given a cursor source coordinate, rotate to a screen coordinate > */ > xf86_crtc_rotate_coord_back(...) > > Looking at the comments, these 2 are meant to be each-others inverse > operation, so if I call them both succession I should get back the same > end result as before, but as explained in detail in my previous mail > for 90 / 270 degree rotation they are not each-others inverse, showing > that something is wrong.
I've come to agree with that, but the problem you describe is actually in xf86_crtc_rotate_coord, not in xf86_crtc_rotate_coord_back. ASCII art time! First of all, I'm afraid xf86_crtc_rotate_coord(_back) talking about "screen coordinate" is a little confusing. They really convert between cursor space and CRTC space, not screen space (in the X screen sense). Cursor space and screen space always have the same orientation. +--------------------------- CRTC space x axis ------------------------+ | | | +- cursor space y axis -+ | | | | | | c | | u | C | r | R | s | T | o | C | r | | | s | s | p | p | a | a | c | c | e | e | | | y | x | | | a | a | x | x | i | i | s | s | | | | | | +-----------------------+ | | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | | +----------------------------------------------------------------------+ Keep in mind that the width and height passed to both functions are in cursor space. When transforming from cursor space to CRTC space by 90/270 degrees, it becomes clear that the CRTC space x depends on the cursor space height, and the CRTC space y depends on the cursor space width, just as it's done in xf86_crtc_rotate_coord_back. However, for the reverse transform, the cursor space x depends on the cursor space width, and the cursor space y depends on the cursor space height, so xf86_crtc_rotate_coord has it backwards. >>>>> This was likely not noticed before since xf86_crtc_rotate_coord_back >>>>> until now was only used with cursor_info->MaxWidth and >>>>> cursor_info->MaxHeight, which are usally the same. >>>> >>>> I'd say it's kind of the other way around: >>>> xf86_crtc_transform_cursor_position just happens to still work with >>>> your >>>> change because usually cursor_info->MaxWidth == >>>> cursor_info->MaxHeight . >>> >>> I could be wrong, but I don't think so, see above. >> >> My analysis stands. > > And I still believe your analysis is wrong. > >> You're trying to use xf86_crtc_rotate_coord_back for >> something else than what it's being used for now, > > 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. Which means that neither xf86_crtc_rotate_coord_back nor xf86_crtc_rotate_coord are a perfect fit for xf86_crtc_transform_gpu_cursor_position. The fixed xf86_crtc_rotate_coord might kind of work, but might require inverting the rotation sense. [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? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer
signature.asc
Description: OpenPGP digital signature
_______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
