subject:"\[PATCH\] Add a return value to set_cursor_position\(\) to allow it to report failure"

Re: [PATCH] Add a return value to set_cursor_position() to allow it to report failure

2014-05-05 Thread Michael Thayer


On 30/04/14 19:23, Keith Packard wrote:

Michael Thayer michael.tha...@oracle.com writes:


On 22/04/14 07:00, Keith Packard wrote:

Michael Thayer michael.tha...@oracle.com writes:


set_cursor_position() may need to be able to fail and have the server fall
back to a software cursor in at least the situation in which we are running
on virtual hardware and using the host cursor as a hardware cursor for the
guest but cannot change its position.


Frankly, the usual solution for nested or virtual X servers is to just
ignore the cursor position assignment.


I would still very much like to have the change if it considered
acceptable, and can submit an fixed patch like the one for
set_cursor_argb() and friends to force a driver build break.  Of course
if you tell me it is something that you would rather not have in I can
leave it at that.


I don't see any value in allowing pointer warping to fail and setting a
software cursor in response. If the next input event from the user isn't
going to happen at the warped position, then you should be setting the
cursor position before sending the next input event anyways.

The alternative is for you to simply always use a software cursor, which
would be easy to manage.


The cases I had in mind were pointer confining and input events from a 
different input device, both of which cause the position of the cursor 
sprite to differ from that of that reported by the input device for more 
than a negligible time.  No matter though, I can solve this as a 
fall-back with a client application which monitors the positions and 
tells the driver to disable the hardware cursor when needed.


Thanks and regards,

Michael
--
ORACLE Deutschland B.V.  Co. KG   Michael Thayer
Werkstrasse 24 VirtualBox engineering
71384 Weinstadt, Germany   mailto:michael.tha...@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
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

Re: [PATCH] Add a return value to set_cursor_position() to allow it to report failure

2014-04-30 Thread Keith Packard

Michael Thayer michael.tha...@oracle.com writes:

 On 22/04/14 07:00, Keith Packard wrote:
 Michael Thayer michael.tha...@oracle.com writes:

 set_cursor_position() may need to be able to fail and have the server fall
 back to a software cursor in at least the situation in which we are running
 on virtual hardware and using the host cursor as a hardware cursor for the
 guest but cannot change its position.

 Frankly, the usual solution for nested or virtual X servers is to just
 ignore the cursor position assignment.

 I would still very much like to have the change if it considered 
 acceptable, and can submit an fixed patch like the one for 
 set_cursor_argb() and friends to force a driver build break.  Of course 
 if you tell me it is something that you would rather not have in I can 
 leave it at that.

I don't see any value in allowing pointer warping to fail and setting a
software cursor in response. If the next input event from the user isn't
going to happen at the warped position, then you should be setting the
cursor position before sending the next input event anyways.

The alternative is for you to simply always use a software cursor, which
would be easy to manage.

-- 
keith.pack...@intel.com


pgpT_S3NDuFXy.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] Add a return value to set_cursor_position() to allow it to report failure

2014-04-24 Thread Michael Thayer

set_cursor_position() may need to be able to fail and have the server fall
back to a software cursor in at least the situation in which we are running
on virtual hardware and using the host cursor as a hardware cursor for the
guest but cannot change its position.  Rename relevant APIs to force driver
build breaks as the return type change will only trigger warnings otherwise.

Signed-off-by: Michael Thayer michael.tha...@oracle.com
---
 hw/xfree86/common/xf86Module.h |  2 +-
 hw/xfree86/modes/xf86Crtc.h|  4 ++--
 hw/xfree86/modes/xf86Cursors.c | 18 --
 hw/xfree86/ramdac/IBM.c| 10 ++
 hw/xfree86/ramdac/TI.c |  5 +++--
 hw/xfree86/ramdac/xf86Cursor.c | 28 
 hw/xfree86/ramdac/xf86Cursor.h |  2 +-
 hw/xfree86/ramdac/xf86CursorPriv.h |  4 +++-
 hw/xfree86/ramdac/xf86HWCurs.c | 10 +-
 9 files changed, 57 insertions(+), 26 deletions(-)

diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index 62ac95d..b848f53 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -80,7 +80,7 @@ typedef enum {
  * mask is 0x.
  */
 #define ABI_ANSIC_VERSION  SET_ABI_VERSION(0, 4)
-#define ABI_VIDEODRV_VERSION   SET_ABI_VERSION(17, 0)
+#define ABI_VIDEODRV_VERSION   SET_ABI_VERSION(18, 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 5407deb..a5c05f6 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -168,8 +168,8 @@ typedef struct _xf86CrtcFuncs {
 /**
  * Set cursor position
  */
-void
- (*set_cursor_position) (xf86CrtcPtr crtc, int x, int y);
+Bool
+ (*set_cursor_position2) (xf86CrtcPtr crtc, int x, int y);
 
 /**
  * Show cursor
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 10ef6f6..828b3ba 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -355,7 +355,7 @@ xf86CrtcTransformCursorPos(xf86CrtcPtr crtc, int *x, int *y)
 *y -= dy;
 }
 
-static void
+static Bool
 xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
 {
 ScrnInfoPtr scrn = crtc-scrn;
@@ -363,6 +363,7 @@ 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;
+Bool ret = FALSE;
 
 /*
  * Transform position of cursor on screen
@@ -388,18 +389,21 @@ xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, 
int y)
 crtc-cursor_in_range = in_range;
 
 if (in_range) {
-crtc-funcs-set_cursor_position(crtc, x, y);
+if (crtc-funcs-set_cursor_position2(crtc, x, y))
+ret = TRUE;
 xf86_crtc_show_cursor(crtc);
 }
 else
 xf86_crtc_hide_cursor(crtc);
+return ret;
 }
 
-static void
+static Bool
 xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
 {
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
 int c;
+Bool ret = FALSE;
 
 /* undo what xf86HWCurs did to the coordinates */
 x += scrn-frameX0;
@@ -408,8 +412,10 @@ xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
 xf86CrtcPtr crtc = xf86_config-crtc[c];
 
 if (crtc-enabled)
-xf86_crtc_set_cursor_position(crtc, x, y);
+if (xf86_crtc_set_cursor_position(crtc, x, y))
+ret = TRUE;
 }
+return ret;
 }
 
 /*
@@ -593,7 +599,7 @@ xf86_cursors_init(ScreenPtr screen, int max_width, int 
max_height, int flags)
 cursor_info-Flags = flags;
 
 cursor_info-SetCursorColors = xf86_set_cursor_colors;
-cursor_info-SetCursorPosition = xf86_set_cursor_position;
+cursor_info-SetCursorPosition2 = xf86_set_cursor_position;
 cursor_info-LoadCursorImage = xf86_load_cursor_image;
 cursor_info-HideCursor = xf86_hide_cursors;
 cursor_info-ShowCursor = xf86_show_cursors;
@@ -666,7 +672,7 @@ xf86_reload_cursors(ScreenPtr screen)
 
 x += scrn-frameX0 + cursor_screen_priv-HotX;
 y += scrn-frameY0 + cursor_screen_priv-HotY;
-(*cursor_info-SetCursorPosition) (scrn, x, y);
+(*cursor_info-SetCursorPosition2) (scrn, x, y);
 }
 }
 
diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
index 872d3d4..21dc172 100644
--- a/hw/xfree86/ramdac/IBM.c
+++ b/hw/xfree86/ramdac/IBM.c
@@ -505,7 +505,7 @@ IBMramdac640HideCursor(ScrnInfoPtr pScrn)
 (*ramdacPtr-WriteDAC) (pScrn, RGB640_CURSOR_CONTROL, 0x00, 0x08);
 }
 
-static void
+static Bool
 IBMramdac526SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
 {
 RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -519,9 +519,10 @@ IBMramdac526SetCursorPosition(ScrnInfoPtr pScrn, int x, 
int y)
 (*ramdacPtr-WriteDAC) (pScrn,

Re: [PATCH] Add a return value to set_cursor_position() to allow it to report failure

2014-04-21 Thread Keith Packard

Michael Thayer michael.tha...@oracle.com writes:

 set_cursor_position() may need to be able to fail and have the server fall
 back to a software cursor in at least the situation in which we are running
 on virtual hardware and using the host cursor as a hardware cursor for the
 guest but cannot change its position.

Frankly, the usual solution for nested or virtual X servers is to just
ignore the cursor position assignment.

-- 
keith.pack...@intel.com


pgp1m3Gydh17r.pgp
Description: PGP signature
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel

[PATCH] Add a return value to set_cursor_position() to allow it to report failure

2014-04-10 Thread Michael Thayer

set_cursor_position() may need to be able to fail and have the server fall
back to a software cursor in at least the situation in which we are running
on virtual hardware and using the host cursor as a hardware cursor for the
guest but cannot change its position.

Signed-off-by: Michael Thayer michael.tha...@oracle.com
---

 hw/xfree86/common/xf86Module.h |  2 +-
 hw/xfree86/modes/xf86Crtc.h|  2 +-
 hw/xfree86/modes/xf86Cursors.c | 14 ++
 hw/xfree86/ramdac/IBM.c|  6 --
 hw/xfree86/ramdac/TI.c |  3 ++-
 hw/xfree86/ramdac/xf86Cursor.c | 28 
 hw/xfree86/ramdac/xf86Cursor.h |  2 +-
 hw/xfree86/ramdac/xf86CursorPriv.h |  4 +++-
 hw/xfree86/ramdac/xf86HWCurs.c |  4 ++--
 9 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/hw/xfree86/common/xf86Module.h b/hw/xfree86/common/xf86Module.h
index 62ac95d..b848f53 100644
--- a/hw/xfree86/common/xf86Module.h
+++ b/hw/xfree86/common/xf86Module.h
@@ -80,7 +80,7 @@ typedef enum {
  * mask is 0x.
  */
 #define ABI_ANSIC_VERSION  SET_ABI_VERSION(0, 4)
-#define ABI_VIDEODRV_VERSION   SET_ABI_VERSION(17, 0)
+#define ABI_VIDEODRV_VERSION   SET_ABI_VERSION(18, 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 5407deb..6fe0b1b 100644
--- a/hw/xfree86/modes/xf86Crtc.h
+++ b/hw/xfree86/modes/xf86Crtc.h
@@ -168,7 +168,7 @@ typedef struct _xf86CrtcFuncs {
 /**
  * Set cursor position
  */
-void
+Bool
  (*set_cursor_position) (xf86CrtcPtr crtc, int x, int y);
 
 /**
diff --git a/hw/xfree86/modes/xf86Cursors.c b/hw/xfree86/modes/xf86Cursors.c
index 10ef6f6..f8fb941 100644
--- a/hw/xfree86/modes/xf86Cursors.c
+++ b/hw/xfree86/modes/xf86Cursors.c
@@ -355,7 +355,7 @@ xf86CrtcTransformCursorPos(xf86CrtcPtr crtc, int *x, int *y)
 *y -= dy;
 }
 
-static void
+static Bool
 xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, int y)
 {
 ScrnInfoPtr scrn = crtc-scrn;
@@ -363,6 +363,7 @@ 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;
+Bool ret = FALSE;
 
 /*
  * Transform position of cursor on screen
@@ -388,18 +389,21 @@ xf86_crtc_set_cursor_position(xf86CrtcPtr crtc, int x, 
int y)
 crtc-cursor_in_range = in_range;
 
 if (in_range) {
-crtc-funcs-set_cursor_position(crtc, x, y);
+if (crtc-funcs-set_cursor_position(crtc, x, y))
+ret = TRUE;
 xf86_crtc_show_cursor(crtc);
 }
 else
 xf86_crtc_hide_cursor(crtc);
+return ret;
 }
 
-static void
+static Bool
 xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
 {
 xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn);
 int c;
+Bool ret = FALSE;
 
 /* undo what xf86HWCurs did to the coordinates */
 x += scrn-frameX0;
@@ -408,8 +412,10 @@ xf86_set_cursor_position(ScrnInfoPtr scrn, int x, int y)
 xf86CrtcPtr crtc = xf86_config-crtc[c];
 
 if (crtc-enabled)
-xf86_crtc_set_cursor_position(crtc, x, y);
+if (xf86_crtc_set_cursor_position(crtc, x, y))
+ret = TRUE;
 }
+return ret;
 }
 
 /*
diff --git a/hw/xfree86/ramdac/IBM.c b/hw/xfree86/ramdac/IBM.c
index 872d3d4..73b3d20 100644
--- a/hw/xfree86/ramdac/IBM.c
+++ b/hw/xfree86/ramdac/IBM.c
@@ -505,7 +505,7 @@ IBMramdac640HideCursor(ScrnInfoPtr pScrn)
 (*ramdacPtr-WriteDAC) (pScrn, RGB640_CURSOR_CONTROL, 0x00, 0x08);
 }
 
-static void
+static Bool
 IBMramdac526SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
 {
 RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -519,9 +519,10 @@ IBMramdac526SetCursorPosition(ScrnInfoPtr pScrn, int x, 
int y)
 (*ramdacPtr-WriteDAC) (pScrn, IBMRGB_curs_xh, 0x00, (x  8)  0xf);
 (*ramdacPtr-WriteDAC) (pScrn, IBMRGB_curs_yl, 0x00, y  0xff);
 (*ramdacPtr-WriteDAC) (pScrn, IBMRGB_curs_yh, 0x00, (y  8)  0xf);
+return TRUE;
 }
 
-static void
+static Bool
 IBMramdac640SetCursorPosition(ScrnInfoPtr pScrn, int x, int y)
 {
 RamDacRecPtr ramdacPtr = RAMDACSCRPTR(pScrn);
@@ -535,6 +536,7 @@ IBMramdac640SetCursorPosition(ScrnInfoPtr pScrn, int x, int 
y)
 (*ramdacPtr-WriteDAC) (pScrn, RGB640_CURS_X_HIGH, 0x00, (x  8)  0xf);
 (*ramdacPtr-WriteDAC) (pScrn, RGB640_CURS_Y_LOW, 0x00, y  0xff);
 (*ramdacPtr-WriteDAC) (pScrn, RGB640_CURS_Y_HIGH, 0x00, (y  8)  0xf);
+return TRUE;
 }
 
 static void
diff --git a/hw/xfree86/ramdac/TI.c b/hw/xfree86/ramdac/TI.c
index 7d4e0d7..05914b7 100644
--- a/hw/xfree86/ramdac/TI.c
+++ b/hw/xfree86/ramdac/TI.c
@@ -606,7 +606,7 @@ TIramdacHideCursor(ScrnInfoPtr pScrn)
 (*ramdacPtr-WriteDAC) (pScrn, TIDAC_ind_curs_ctrl, 0, 0x00);
 }
 
-static void
+static Bool

Re: [PATCH] Add a return value to set_cursor_position() to allow it to report failure

Re: [PATCH] Add a return value to set_cursor_position() to allow it to report failure

[PATCH] Add a return value to set_cursor_position() to allow it to report failure

Re: [PATCH] Add a return value to set_cursor_position() to allow it to report failure

[PATCH] Add a return value to set_cursor_position() to allow it to report failure

5 matches

Site Navigation

Mail list logo

Footer information