I think this patch is totally sensible. Reviewed-by: Jamey Sharp <[email protected]>
For the other patch in the series, "xfree86/modes: Let the driver handle the transform", I don't think I can review it. I don't know the relevant code and the patch touches quite a bit. I tried to see if I could write a different patch that might get you the same effect with fewer core server changes. I couldn't, but maybe the approach I had in mind will help somehow. I imagined adding a "crtc->funcs->handle_transform" callback that, if it's present and returns True for a given transform, indicates that the driver is handling that transform and the server should render without any transformation. In that case, I'd let crtc->transform_in_use be False, and I hoped most of the code paths your patch had to change would already have the right behavior then. I got confused by how to handle LeaveVT/EnterVT or modesetting failure--and whether transform was even the right thing to look at, or if crtc->rotation is what you're after. Jamey On Thu, Aug 25, 2011 at 04:35:10PM -0700, Aaron Plattner wrote: > When the driver can handle the crtc transform in hardware, it sets > crtc->driverIsPerformingTransform, which turns off both the shadow > layer and the cursor's position-transforming code. However, some > drivers actually do require the cursor position to still be > transformed in these cases. Move the cursor position transform into a > helper function that can be called by such drivers. > > Signed-off-by: Aaron Plattner <[email protected]> > --- > hw/xfree86/modes/xf86Crtc.h | 8 +++++ > hw/xfree86/modes/xf86Cursors.c | 57 +++++++++++++++++++++------------------ > 2 files changed, 39 insertions(+), 26 deletions(-) > > diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h > index 0d7a6a6..ffb2eff 100644 > --- a/hw/xfree86/modes/xf86Crtc.h > +++ b/hw/xfree86/modes/xf86Crtc.h > @@ -948,6 +948,14 @@ xf86_hide_cursors (ScrnInfoPtr scrn); > extern _X_EXPORT void > xf86_cursors_fini (ScreenPtr screen); > > +/** > + * Transform the cursor's coordinates based on the crtc transform. Normally > + * this is done by the server, but if crtc->driverIsPerformingTransform is > TRUE, > + * then the server does not transform the cursor position automatically. > + */ > +extern _X_EXPORT void > +xf86CrtcTransformCursorPos (xf86CrtcPtr crtc, int *x, int *y); > + > /* > * For overlay video, compute the relevant CRTC and > * clip video to that. > diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c > index 4281ab3..276bd27 100644 > --- a/hw/xfree86/modes/xf86Cursors.c > +++ b/hw/xfree86/modes/xf86Cursors.c > @@ -337,7 +337,36 @@ xf86_show_cursors (ScrnInfoPtr scrn) > xf86_crtc_show_cursor (crtc); > } > } > - > + > +void xf86CrtcTransformCursorPos (xf86CrtcPtr crtc, int *x, int *y) > +{ > + ScrnInfoPtr scrn = crtc->scrn; > + ScreenPtr screen = scrn->pScreen; > + xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); > + xf86CursorInfoPtr cursor_info = xf86_config->cursor_info; > + xf86CursorScreenPtr ScreenPriv = > + (xf86CursorScreenPtr)dixLookupPrivate(&screen->devPrivates, > + xf86CursorScreenKey); > + struct pict_f_vector v; > + int dx, dy; > + > + v.v[0] = (*x + ScreenPriv->HotX) + 0.5; > + v.v[1] = (*y + ScreenPriv->HotY) + 0.5; > + v.v[2] = 1; > + pixman_f_transform_point (&crtc->f_framebuffer_to_crtc, &v); > + /* cursor will have 0.5 added to it already so floor is sufficent */ > + *x = floor (v.v[0]); > + *y = floor (v.v[1]); > + /* > + * Transform position of cursor upper left corner > + */ > + xf86_crtc_rotate_coord_back (crtc->rotation, cursor_info->MaxWidth, > + cursor_info->MaxHeight, ScreenPriv->HotX, > + ScreenPriv->HotY, &dx, &dy); > + *x -= dx; > + *y -= dy; > +} > + > static void > xf86_crtc_set_cursor_position (xf86CrtcPtr crtc, int x, int y) > { > @@ -346,36 +375,12 @@ xf86_crtc_set_cursor_position (xf86CrtcPtr crtc, int x, > int y) > xf86CursorInfoPtr cursor_info = xf86_config->cursor_info; > DisplayModePtr mode = &crtc->mode; > Bool in_range; > - int dx, dy; > > /* > * Transform position of cursor on screen > */ > if (crtc->transform_in_use && !crtc->driverIsPerformingTransform) > - { > - ScreenPtr screen = scrn->pScreen; > - xf86CursorScreenPtr ScreenPriv = > - (xf86CursorScreenPtr)dixLookupPrivate(&screen->devPrivates, > - xf86CursorScreenKey); > - struct pict_f_vector v; > - > - v.v[0] = (x + ScreenPriv->HotX) + 0.5; > - v.v[1] = (y + ScreenPriv->HotY) + 0.5; > - v.v[2] = 1; > - pixman_f_transform_point (&crtc->f_framebuffer_to_crtc, &v); > - /* cursor will have 0.5 added to it already so floor is sufficent */ > - x = floor (v.v[0]); > - y = floor (v.v[1]); > - /* > - * Transform position of cursor upper left corner > - */ > - xf86_crtc_rotate_coord_back (crtc->rotation, > - cursor_info->MaxWidth, > - cursor_info->MaxHeight, > - ScreenPriv->HotX, ScreenPriv->HotY, &dx, > &dy); > - x -= dx; > - y -= dy; > - } > + xf86CrtcTransformCursorPos(crtc, &x, &y); > else > { > x -= crtc->x; > -- > 1.7.4.1 > > _______________________________________________ > [email protected]: X.Org development > Archives: http://lists.x.org/archives/xorg-devel > Info: http://lists.x.org/mailman/listinfo/xorg-devel
signature.asc
Description: Digital signature
_______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
