[PATCH 2/2] xfree86/modes: Make cursor position transform a helper function

[Jamey Sharp]

I think this patch is totally sensible.

Reviewed-by: Jamey Sharp <jamey at minilop.net>

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 <aplattner at nvidia.com>
> ---
>  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
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.x.org/archives/xorg-devel/attachments/20110929/9a87bdf1/attachment.pgp>