On 07/09/16 08:43 PM, Hans de Goede wrote: > On 07-09-16 03:21, Michel Dänzer wrote: >> On 06/09/16 08:31 PM, Hans De Goede wrote: >>> xf86_crtc_rotate_coord_back should be the exact inverse operation of >>> xf86_crtc_rotate_coord, but when calculating x / y for 90 / 270 degrees >>> rotation it was using height to calculate x / width to calculate y, >>> instead of the otherway around. >> >> I think it's correct as is. It's certainly symmetrical with >> xf86_crtc_rotate_coord. > > Actually no, it is not symmetrical, it uses width to calculate y and > height to calculate x, where as xf86_crtc_rotate_coord uses > width when calculating x and height when calculating y as one would > expect.
Are we looking at the same code? https://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/modes/xf86Cursors.c#n75 case RR_Rotate_90: t = x_dst; x_dst = height - y_dst - 1; y_dst = t; break; https://cgit.freedesktop.org/xorg/xserver/tree/hw/xfree86/modes/xf86Cursors.c#n84 case RR_Rotate_270: t = x_dst; x_dst = y_dst; y_dst = width - t - 1; break; If you think about it, it makes sense that the x & y axes have to be crossed for 90/270 degree rotation. > First lets call xf86_crtc_rotate_coord, with 270 degree rotation, Where would that be? I can't see that happening anywhere, neither before nor after this series. Before your series, the only caller of xf86_crtc_rotate_coord_back is xf86_crtc_transform_cursor_position, which uses it to transform the position of the cursor hotspot inside the cursor image. The hotspot position isn't transformed with xf86_crtc_rotate_coord before that. Similarly, the only callers of xf86_crtc_rotate_coord just use it for transforming positions within the cursor image, not the position of the cursor relative to anything else. >>> 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. You're trying to use xf86_crtc_rotate_coord_back for something else than what it's being used for now, and your changes break xf86_crtc_transform_cursor_position if cursor_info->MaxWidth != cursor_info->MaxHeight . That's a NAK for this patch (and patch 6 in the current form). -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ 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
