Switching between a software and a hardware mouse cursor
Michael Thayer
michael.thayer at oracle.com
Tue Mar 25 13:10:50 PDT 2014
On 18/03/14 11:17, Michael Thayer wrote:
> On 05/03/14 19:45, Adam Jackson wrote:
>> On Thu, 2014-02-27 at 15:46 +0100, Michael Thayer wrote:
>>
>>> Another would be
>>> to add a return value to the DDX CRTC functions "load_cursor_argb", so
>>> that if the KMS driver failed to set the cursor, "modesetting" could
>>> pass this on to the X server.
>>
>> Actually we really should make load_cursor_argb return something other
>> than void. Some particularly incapable server GPUs have format
>> restrictions on their hardware cursors that mean _some_ ARGB cursors
>> could be made to work but not all; and right now, the fallback code in
>> -modesetting doesn't get this case right, and you end up with no cursor
>> at all.
I have done an initial patch to do this, see below. If I get positive
feedback I will resend it formatted to be applied to the tree, but I
expect a couple of iterations.
> I have been looking at what this would take, and what rather puts me off
> is that it would mean patching all supported video drivers without even
> the means to test them (the patches will be mostly be adding "return
> TRUE" in the right place of course, but you know how it is; how is this
> sort of thing normally handled?)
This question is still relevant.
[...]
> Even nicer would be a way in addition of telling the server that a
> particular hardware cursor is linked to a particular input device, and
> that it should use a software cursor if it wants the cursor to be
> somewhere other than the position reported by the device; I can't
> immediately think though where that would fit in cleanly, and neither
> can I immediately think of any other potential users of such an interface.
This could be achieved by making SetCursorPosition() return a success
value too. Would this be an acceptable idea?
Regards,
Michael
---
diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index e8c24f2..62ac95d 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -80,7 +80,7 @@ typedef enum {
* mask is 0xFFFF0000.
*/
#define ABI_ANSIC_VERSION SET_ABI_VERSION(0, 4)
-#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(16, 0)
+#define ABI_VIDEODRV_VERSION SET_ABI_VERSION(17, 0)
#define ABI_XINPUT_VERSION SET_ABI_VERSION(21, 0)
#define ABI_EXTENSION_VERSION SET_ABI_VERSION(8, 0)
#define ABI_FONT_VERSION SET_ABI_VERSION(0, 6)
diff --git a/hw/xfree86/modes/xf86Crtc.h b/hw/xfree86/modes/xf86Crtc.h
index c127d78..5407deb 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -186,13 +186,13 @@ typedef struct _xf86CrtcFuncs {
/**
* Load monochrome image
*/
- void
+ Bool
(*load_cursor_image) (xf86CrtcPtr crtc, CARD8 *image);
/**
* Load ARGB image
*/
- void
+ Bool
(*load_cursor_argb) (xf86CrtcPtr crtc, CARD32 *image);
/**
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 2b0db34..10ef6f6 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -211,7 +211,7 @@ set_bit(CARD8 *image, xf86CursorInfoPtr cursor_info,
int x, int y, Bool mask)
/*
* Load a two color cursor into a driver that supports only ARGB cursors
*/
-static void
+static Bool
xf86_crtc_convert_cursor_to_argb(xf86CrtcPtr crtc, unsigned char *src)
{
ScrnInfoPtr scrn = crtc->scrn;
@@ -244,7 +244,7 @@ xf86_crtc_convert_cursor_to_argb(xf86CrtcPtr crtc,
unsigned char *src)
bits = 0;
cursor_image[y * cursor_info->MaxWidth + x] = bits;
}
- crtc->funcs->load_cursor_argb(crtc, cursor_image);
+ return crtc->funcs->load_cursor_argb(crtc, cursor_image);
}
/*
@@ -415,7 +415,7 @@ xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
/*
* Load a two-color cursor into a crtc, performing rotation as needed
*/
-static void
+static Bool
xf86_crtc_load_cursor_image(xf86CrtcPtr crtc, CARD8 *src)
{
ScrnInfoPtr scrn = crtc->scrn;
@@ -450,13 +450,13 @@ xf86_crtc_load_cursor_image(xf86CrtcPtr crtc,
CARD8 *src)
set_bit(cursor_image, cursor_info, x, y, TRUE);
}
}
- crtc->funcs->load_cursor_image(crtc, cursor_image);
+ return crtc->funcs->load_cursor_image(crtc, cursor_image);
}
/*
* Load a cursor image into all active CRTCs
*/
-static void
+static Bool
xf86_load_cursor_image(ScrnInfoPtr scrn, unsigned char *src)
{
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -466,12 +466,17 @@ xf86_load_cursor_image(ScrnInfoPtr scrn, unsigned
char *src)
xf86CrtcPtr crtc = xf86_config->crtc[c];
if (crtc->enabled) {
- if (crtc->funcs->load_cursor_image)
- xf86_crtc_load_cursor_image(crtc, src);
- else if (crtc->funcs->load_cursor_argb)
- xf86_crtc_convert_cursor_to_argb(crtc, src);
+ if (crtc->funcs->load_cursor_image) {
+ if (!xf86_crtc_load_cursor_image(crtc, src))
+ return FALSE;
+ } else if (crtc->funcs->load_cursor_argb) {
+ if (!xf86_crtc_convert_cursor_to_argb(crtc, src))
+ return FALSE;
+ } else
+ return FALSE;
}
}
+ return TRUE;
}
static Bool
@@ -516,7 +521,7 @@ xf86_use_hw_cursor_argb(ScreenPtr screen, CursorPtr
cursor)
return TRUE;
}
-static void
+static Bool
xf86_crtc_load_cursor_argb(xf86CrtcPtr crtc, CursorPtr cursor)
{
ScrnInfoPtr scrn = crtc->scrn;
@@ -544,10 +549,10 @@ xf86_crtc_load_cursor_argb(xf86CrtcPtr crtc,
CursorPtr cursor)
cursor_image[y * image_width + x] = bits;
}
- crtc->funcs->load_cursor_argb(crtc, cursor_image);
+ return crtc->funcs->load_cursor_argb(crtc, cursor_image);
}
-static void
+static Bool
xf86_load_cursor_argb(ScrnInfoPtr scrn, CursorPtr cursor)
{
xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
@@ -557,8 +562,10 @@ xf86_load_cursor_argb(ScrnInfoPtr scrn, CursorPtr
cursor)
xf86CrtcPtr crtc = xf86_config->crtc[c];
if (crtc->enabled)
- xf86_crtc_load_cursor_argb(crtc, cursor);
+ if (!xf86_crtc_load_cursor_argb(crtc, cursor))
+ return FALSE;
}
+ return TRUE;
}
Bool
@@ -608,6 +615,8 @@ xf86_cursors_init(ScreenPtr screen, int max_width,
int max_height, int flags)
* Called when anything on the screen is reconfigured.
*
* Reloads cursor images as needed, then adjusts cursor positions
+ * @note We assume that all hardware cursors to be loaded have already been
+ * found to be usable by the hardware.
*/
void
diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
index bc71623..872d3d4 100644
--- a/hw/xfree86/ramdac/IBM.c
+++ b/hw/xfree86/ramdac/IBM.c
@@ -570,7 +570,7 @@ IBMramdac640SetCursorColors(ScrnInfoPtr pScrn, int
bg, int fg)
(*ramdacPtr->WriteData) (pScrn, bg);
}
-static void
+static Bool
IBMramdac526LoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -582,9 +582,10 @@ IBMramdac526LoadCursorImage(ScrnInfoPtr pScrn,
unsigned char *src)
*/
for (i = 0; i < 1024; i++)
(*ramdacPtr->WriteDAC) (pScrn, IBMRGB_curs_array + i, 0x00,
(*src++));
+ return TRUE;
}
-static void
+static Bool
IBMramdac640LoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -596,6 +597,7 @@ IBMramdac640LoadCursorImage(ScrnInfoPtr pScrn,
unsigned char *src)
*/
for (i = 0; i < 1024; i++)
(*ramdacPtr->WriteDAC) (pScrn, RGB640_CURS_WRITE + i, 0x00,
(*src++));
+ return TRUE;
}
static Bool
diff --git a/hw/xfree86/ramdac/TI.c b/hw/xfree86/ramdac/TI.c
index 393b774..7d4e0d7 100644
--- a/hw/xfree86/ramdac/TI.c
+++ b/hw/xfree86/ramdac/TI.c
@@ -642,7 +642,7 @@ TIramdacSetCursorColors(ScrnInfoPtr pScrn, int bg,
int fg)
(*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_COLOR, 0, (fg &
0x000000ff));
}
-static void
+static Bool
TIramdacLoadCursorImage(ScrnInfoPtr pScrn, unsigned char *src)
{
RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -657,6 +657,7 @@ TIramdacLoadCursorImage(ScrnInfoPtr pScrn, unsigned
char *src)
/* NOT_DONE: might need a delay here */
(*ramdacPtr->WriteDAC) (pScrn, TIDAC_CURS_RAM_DATA, 0, *(src++));
}
+ return TRUE;
}
static Bool
diff --git a/hw/xfree86/ramdac/xf86Cursor.c b/hw/xfree86/ramdac/xf86Cursor.c
index 860704e..fac6822 100644
--- a/hw/xfree86/ramdac/xf86Cursor.c
+++ b/hw/xfree86/ramdac/xf86Cursor.c
@@ -352,12 +352,13 @@ xf86CursorSetCursor(DeviceIntPtr pDev, ScreenPtr
pScreen, CursorPtr pCurs,
(*ScreenPriv->spriteFuncs->SetCursor) (pDev, pScreen,
NullCursor, x, y);
- xf86SetCursor(pScreen, cursor, x, y);
- ScreenPriv->SWCursor = FALSE;
- ScreenPriv->isUp = TRUE;
+ if (xf86SetCursor(pScreen, cursor, x, y)) {
+ ScreenPriv->SWCursor = FALSE;
+ ScreenPriv->isUp = TRUE;
- miPointerSetWaitForUpdate(pScreen,
!infoPtr->pScrn->silkenMouse);
- return;
+ miPointerSetWaitForUpdate(pScreen,
!infoPtr->pScrn->silkenMouse);
+ return;
+ }
}
miPointerSetWaitForUpdate(pScreen, TRUE);
diff --git a/hw/xfree86/ramdac/xf86Cursor.h b/hw/xfree86/ramdac/xf86Cursor.h
index 5658e7b..1ecbdcd 100644
--- a/hw/xfree86/ramdac/xf86Cursor.h
+++ b/hw/xfree86/ramdac/xf86Cursor.h
@@ -12,7 +12,7 @@ typedef struct _xf86CursorInfoRec {
int MaxHeight;
void (*SetCursorColors) (ScrnInfoPtr pScrn, int bg, int fg);
void (*SetCursorPosition) (ScrnInfoPtr pScrn, int x, int y);
- void (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
+ Bool (*LoadCursorImage) (ScrnInfoPtr pScrn, unsigned char *bits);
void (*HideCursor) (ScrnInfoPtr pScrn);
void (*ShowCursor) (ScrnInfoPtr pScrn);
unsigned char *(*RealizeCursor) (struct _xf86CursorInfoRec *,
CursorPtr);
@@ -20,7 +20,7 @@ typedef struct _xf86CursorInfoRec {
#ifdef ARGB_CURSOR
Bool (*UseHWCursorARGB) (ScreenPtr, CursorPtr);
- void (*LoadCursorARGB) (ScrnInfoPtr, CursorPtr);
+ Bool (*LoadCursorARGB) (ScrnInfoPtr, CursorPtr);
#endif
} xf86CursorInfoRec, *xf86CursorInfoPtr;
diff --git a/hw/xfree86/ramdac/xf86CursorPriv.h
b/hw/xfree86/ramdac/xf86CursorPriv.h
index a5d2aab..f34c1c7 100644
--- a/hw/xfree86/ramdac/xf86CursorPriv.h
+++ b/hw/xfree86/ramdac/xf86CursorPriv.h
@@ -37,7 +37,7 @@ typedef struct {
void *transparentData;
} xf86CursorScreenRec, *xf86CursorScreenPtr;
-void xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y);
+Bool xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y);
void xf86SetTransparentCursor(ScreenPtr pScreen);
void xf86MoveCursor(ScreenPtr pScreen, int x, int y);
void xf86RecolorCursor(ScreenPtr pScreen, CursorPtr pCurs, Bool
displayed);
diff --git a/hw/xfree86/ramdac/xf86HWCurs.c b/hw/xfree86/ramdac/xf86HWCurs.c
index 3b69698..0b5caa2 100644
--- a/hw/xfree86/ramdac/xf86HWCurs.c
+++ b/hw/xfree86/ramdac/xf86HWCurs.c
@@ -119,7 +119,7 @@ xf86InitHardwareCursor(ScreenPtr pScreen,
xf86CursorInfoPtr infoPtr)
return TRUE;
}
-void
+Bool
xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs, int x, int y)
{
xf86CursorScreenPtr ScreenPriv =
@@ -130,7 +130,7 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs,
int x, int y)
if (pCurs == NullCursor) {
(*infoPtr->HideCursor) (infoPtr->pScrn);
- return;
+ return TRUE;
}
bits =
@@ -152,18 +152,21 @@ xf86SetCursor(ScreenPtr pScreen, CursorPtr pCurs,
int x, int y)
(*infoPtr->HideCursor) (infoPtr->pScrn);
#ifdef ARGB_CURSOR
- if (pCurs->bits->argb && infoPtr->LoadCursorARGB)
- (*infoPtr->LoadCursorARGB) (infoPtr->pScrn, pCurs);
- else
+ if (pCurs->bits->argb && infoPtr->LoadCursorARGB) {
+ if (!(*infoPtr->LoadCursorARGB) (infoPtr->pScrn, pCurs))
+ return FALSE;
+ } else
#endif
if (bits)
- (*infoPtr->LoadCursorImage) (infoPtr->pScrn, bits);
+ if (!(*infoPtr->LoadCursorImage) (infoPtr->pScrn, bits))
+ return FALSE;
xf86RecolorCursor(pScreen, pCurs, 1);
(*infoPtr->SetCursorPosition) (infoPtr->pScrn, x, y);
(*infoPtr->ShowCursor) (infoPtr->pScrn);
+ return TRUE;
}
void
--
ORACLE Deutschland B.V. & Co. KG Michael Thayer
Werkstrasse 24 VirtualBox engineering
71384 Weinstadt, Germany mailto:michael.thayer at oracle.com
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRA 95603
Geschäftsführer: Jürgen Kunz
Komplementärin: ORACLE Deutschland Verwaltung B.V.
Hertogswetering 163/167, 3543 AS Utrecht, Niederlande
Handelsregister der Handelskammer Midden-Niederlande, Nr. 30143697
Geschäftsführer: Alexander van der Ven, Astrid Kepper, Val Maher
More information about the xorg-devel
mailing list