Re: [PATCH xserver 4/7] glx: Use vnd layer for dispatch (v2)

2018-01-10 Thread Kyle Brenneman 


On 01/10/2018 01:57 PM, Kyle Brenneman wrote:

On 01/10/2018 11:05 AM, Adam Jackson wrote:

The big change here is MakeCurrent and context tag tracking. We now
delegate context tags entirely to the vnd layer, and simply store a
pointer to the context state as the tag data. If a context is deleted
while it's current, we allocate a fake ID for the context and move the
context state there, so the tag data still points to a real context. As
a result we can stop trying so hard to detach the client from contexts
at disconnect time and just let resource destruction handle it.

Since vnd handles all the MakeCurrent protocol now, our request handlers
for it can just be return BadImplementation.

We also remove a bunch of LEGAL_NEW_RESOURCE, because now by the time
we're called vnd has already allocated its tracking resource on that
XID. Note that we only do this for core GLX requests, for vendor private
requests we still need to call LEGAL_NEW_RESOURCE and in addition need
to call up to addXIDMap and friends.

v2: Update to match v2 of the vnd import, and remove more redundant work
like request length checks.

Signed-off-by: Adam Jackson 
---
  configure.ac   |   2 +-
  glx/createcontext.c|   2 -
  glx/glxcmds.c  | 275 
--

  glx/glxcmdsswap.c  |  98 +---
  glx/glxext.c   | 329 
-

  glx/glxext.h   |   4 +
  glx/glxscreens.h   |   1 +
  glx/glxserver.h|   5 -
  glx/xfont.c|   2 -
  hw/kdrive/ephyr/ephyr.c|   2 +-
  hw/kdrive/ephyr/meson.build|   1 +
  hw/kdrive/src/kdrive.c |   3 +
  hw/vfb/InitOutput.c|   2 +
  hw/vfb/meson.build |   3 +-
  hw/xfree86/Makefile.am |   5 +
  hw/xfree86/common/xf86Init.c   |   2 +-
  hw/xfree86/dixmods/glxmodule.c |   1 +
  hw/xfree86/meson.build |   1 +
  hw/xquartz/darwin.c|   4 +-
  hw/xwayland/Makefile.am|   1 +
  hw/xwayland/meson.build|   1 +
  hw/xwayland/xwayland.c |   2 +
  include/glx_extinit.h  |   5 +-
  23 files changed, 359 insertions(+), 392 deletions(-)




xorgGlxThunkRequest should be calling addXIDMap and removeXIDMap, not 
the internal handlers. The default branch there may need some 
additional attention if it would handle any requests that create or 
destroy resources.


And, in answer to the question next to xorgGlxThunkRequest, those 
could indeed be generated. The generate_dispatch_stubs.py script in 
the libglvnd repo only generates the core GLX requests, but it should 
be able to handle of those GLXVendorPrivate requests as well.


-Kyle



You've still got the xorgVendorInitClosure struct left over in glxext.c, 
too. I don't think anything uses it now.

___
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

Re: [PATCH xserver 4/7] glx: Use vnd layer for dispatch (v2)

2018-01-10 Thread Kyle Brenneman 

On 01/10/2018 11:05 AM, Adam Jackson wrote:

The big change here is MakeCurrent and context tag tracking. We now
delegate context tags entirely to the vnd layer, and simply store a
pointer to the context state as the tag data. If a context is deleted
while it's current, we allocate a fake ID for the context and move the
context state there, so the tag data still points to a real context. As
a result we can stop trying so hard to detach the client from contexts
at disconnect time and just let resource destruction handle it.

Since vnd handles all the MakeCurrent protocol now, our request handlers
for it can just be return BadImplementation.

We also remove a bunch of LEGAL_NEW_RESOURCE, because now by the time
we're called vnd has already allocated its tracking resource on that
XID. Note that we only do this for core GLX requests, for vendor private
requests we still need to call LEGAL_NEW_RESOURCE and in addition need
to call up to addXIDMap and friends.

v2: Update to match v2 of the vnd import, and remove more redundant work
like request length checks.

Signed-off-by: Adam Jackson 
---
  configure.ac   |   2 +-
  glx/createcontext.c|   2 -
  glx/glxcmds.c  | 275 --
  glx/glxcmdsswap.c  |  98 +---
  glx/glxext.c   | 329 -
  glx/glxext.h   |   4 +
  glx/glxscreens.h   |   1 +
  glx/glxserver.h|   5 -
  glx/xfont.c|   2 -
  hw/kdrive/ephyr/ephyr.c|   2 +-
  hw/kdrive/ephyr/meson.build|   1 +
  hw/kdrive/src/kdrive.c |   3 +
  hw/vfb/InitOutput.c|   2 +
  hw/vfb/meson.build |   3 +-
  hw/xfree86/Makefile.am |   5 +
  hw/xfree86/common/xf86Init.c   |   2 +-
  hw/xfree86/dixmods/glxmodule.c |   1 +
  hw/xfree86/meson.build |   1 +
  hw/xquartz/darwin.c|   4 +-
  hw/xwayland/Makefile.am|   1 +
  hw/xwayland/meson.build|   1 +
  hw/xwayland/xwayland.c |   2 +
  include/glx_extinit.h  |   5 +-
  23 files changed, 359 insertions(+), 392 deletions(-)




xorgGlxThunkRequest should be calling addXIDMap and removeXIDMap, not 
the internal handlers. The default branch there may need some additional 
attention if it would handle any requests that create or destroy resources.


And, in answer to the question next to xorgGlxThunkRequest, those could 
indeed be generated. The generate_dispatch_stubs.py script in the 
libglvnd repo only generates the core GLX requests, but it should be 
able to handle of those GLXVendorPrivate requests as well.


-Kyle

___
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

[PATCH xserver 5/7] miinitext: Load GLX on the mi path

2018-01-10 Thread Adam Jackson 
Add a stub for Xnest so it continues to link, but otherwise we support
GLX on every server so there's no need to make every DDX add it.

Signed-off-by: Adam Jackson 
---
 hw/dmx/dmxinit.c   |  3 ---
 hw/kdrive/ephyr/ephyrinit.c| 16 
 hw/vfb/InitOutput.c| 15 ---
 hw/xfree86/dixmods/glxmodule.c |  6 --
 hw/xnest/Init.c|  7 +++
 hw/xquartz/quartz.c| 21 -
 hw/xwayland/xwayland.c |  3 ---
 hw/xwin/InitOutput.c   |  1 -
 include/extinit.h  |  1 +
 include/glx_extinit.h  |  3 +--
 mi/miinitext.c |  3 +++
 test/Makefile.am   |  4 
 12 files changed, 16 insertions(+), 67 deletions(-)

diff --git a/hw/dmx/dmxinit.c b/hw/dmx/dmxinit.c
index d1ffcc538..02031ed7c 100644
--- a/hw/dmx/dmxinit.c
+++ b/hw/dmx/dmxinit.c
@@ -536,9 +536,6 @@ static void dmxAddExtensions(void)
 {
 const ExtensionModule dmxExtensions[] = {
 { DMXExtensionInit, DMX_EXTENSION_NAME, NULL },
-#ifdef GLXEXT
-{ GlxExtensionInit, "GLX",  },
-#endif
 };
 
 LoadExtensionList(dmxExtensions, ARRAY_SIZE(dmxExtensions), TRUE);
diff --git a/hw/kdrive/ephyr/ephyrinit.c b/hw/kdrive/ephyr/ephyrinit.c
index 947a6e8ef..abc35dfca 100644
--- a/hw/kdrive/ephyr/ephyrinit.c
+++ b/hw/kdrive/ephyr/ephyrinit.c
@@ -58,25 +58,9 @@ InitCard(char *name)
 KdCardInfoAdd(, 0);
 }
 
-static const ExtensionModule ephyrExtensions[] = {
-#ifdef GLXEXT
- { GlxExtensionInit, "GLX",  },
-#endif
-};
-
-static
-void ephyrExtensionInit(void)
-{
-LoadExtensionList(ephyrExtensions, ARRAY_SIZE(ephyrExtensions), TRUE);
-}
-
-
 void
 InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv)
 {
-if (serverGeneration == 1)
-ephyrExtensionInit();
-
 KdInitOutput(pScreenInfo, argc, argv);
 }
 
diff --git a/hw/vfb/InitOutput.c b/hw/vfb/InitOutput.c
index 2b7bca5fa..407f2afcd 100644
--- a/hw/vfb/InitOutput.c
+++ b/hw/vfb/InitOutput.c
@@ -957,27 +957,12 @@ vfbScreenInit(ScreenPtr pScreen, int argc, char **argv)
 
 }   /* end vfbScreenInit */
 
-static const ExtensionModule vfbExtensions[] = {
-#ifdef GLXEXT
-{ GlxExtensionInit, "GLX",  },
-#endif
-};
-
-static
-void vfbExtensionInit(void)
-{
-LoadExtensionList(vfbExtensions, ARRAY_SIZE(vfbExtensions), TRUE);
-}
-
 void
 InitOutput(ScreenInfo * screen_info, int argc, char **argv)
 {
 int i;
 int NumFormats = 0;
 
-if (serverGeneration == 1)
-vfbExtensionInit();
-
 /* initialize pixmap formats */
 
 /* must have a pixmap depth to match every screen depth */
diff --git a/hw/xfree86/dixmods/glxmodule.c b/hw/xfree86/dixmods/glxmodule.c
index c3d92fa92..2215c8867 100644
--- a/hw/xfree86/dixmods/glxmodule.c
+++ b/hw/xfree86/dixmods/glxmodule.c
@@ -47,10 +47,6 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 static MODULESETUPPROTO(glxSetup);
 
-static const ExtensionModule GLXExt[] = {
-{ GlxExtensionInit, "GLX",  },
-};
-
 static XF86ModuleVersionInfo VersRec = {
 "glx",
 MODULEVENDORSTRING,
@@ -85,7 +81,5 @@ glxSetup(void *module, void *opts, int *errmaj, int *errmin)
 GlxPushProvider(provider);
 xorgGlxCreateVendor();
 
-LoadExtensionList(GLXExt, ARRAY_SIZE(GLXExt), FALSE);
-
 return module;
 }
diff --git a/hw/xnest/Init.c b/hw/xnest/Init.c
index e8a700e88..b45685216 100644
--- a/hw/xnest/Init.c
+++ b/hw/xnest/Init.c
@@ -47,6 +47,13 @@ is" without express or implied warranty.
 
 Bool xnestDoFullGeneration = True;
 
+#ifdef GLXEXT
+void
+GlxExtensionInit(void)
+{
+}
+#endif
+
 void
 InitOutput(ScreenInfo * screen_info, int argc, char *argv[])
 {
diff --git a/hw/xquartz/quartz.c b/hw/xquartz/quartz.c
index c8ea3bf8b..980aa4a36 100644
--- a/hw/xquartz/quartz.c
+++ b/hw/xquartz/quartz.c
@@ -149,25 +149,6 @@ QuartzSetupScreen(int index,
 return TRUE;
 }
 
-static const ExtensionModule quartzExtensions[] = {
-/* PseudoramiX needs to be done before RandR, so
- * it is in miinitext.c until it can be reordered.
- * { PseudoramiXExtensionInit, "PseudoramiX",  },
- */
-#ifdef GLXEXT
-{GlxExtensionInit, "GLX", },
-#endif
-};
-
-/*
- * QuartzExtensionInit
- * Initialises XQuartz-specific extensions.
- */
-static void QuartzExtensionInit(void)
-{
-LoadExtensionList(quartzExtensions, ARRAY_SIZE(quartzExtensions), TRUE);
-}
-
 /*
  * QuartzInitOutput
  *  Quartz display initialization.
@@ -208,8 +189,6 @@ QuartzInitOutput(int argc,
 
 // Do display mode specific initialization
 quartzProcs->DisplayInit();
-
-QuartzExtensionInit();
 }
 
 /*
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 487d3dc5f..15ac4546e 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -1039,9 +1039,6 @@ xwl_log_handler(const char *format, va_list args)
 }
 
 static const ExtensionModule xwayland_extensions[] = {
-#ifdef GLXEXT
-{ GlxExtensionInit, "GLX",  },

[PATCH xserver 6/7] miinitext: Remove separate extension toggle list

2018-01-10 Thread Adam Jackson 
This was only separate because GLX was loadable. The frontend is now
linked statically, so we can use the static extension list directly.

Signed-off-by: Adam Jackson 
---
 mi/miinitext.c | 173 +
 1 file changed, 51 insertions(+), 122 deletions(-)

diff --git a/mi/miinitext.c b/mi/miinitext.c
index c8162337c..e55073bf3 100644
--- a/mi/miinitext.c
+++ b/mi/miinitext.c
@@ -106,128 +106,6 @@ SOFTWARE.
 #include "micmap.h"
 #include "globals.h"
 
-/* The following is only a small first step towards run-time
- * configurable extensions.
- */
-typedef struct {
-const char *name;
-Bool *disablePtr;
-} ExtensionToggle;
-
-static ExtensionToggle ExtensionToggleList[] = {
-/* sort order is extension name string as shown in xdpyinfo */
-{"Generic Events", },
-#ifdef COMPOSITE
-{"Composite", },
-#endif
-#ifdef DAMAGE
-{"DAMAGE", },
-#endif
-#ifdef DBE
-{"DOUBLE-BUFFER", },
-#endif
-#ifdef DPMSExtension
-{"DPMS", },
-#endif
-#ifdef GLXEXT
-{"GLX", },
-#endif
-#ifdef SCREENSAVER
-{"MIT-SCREEN-SAVER", },
-#endif
-#ifdef MITSHM
-{"MIT-SHM", },
-#endif
-#ifdef RANDR
-{"RANDR", },
-#endif
-{"RENDER", },
-#ifdef XCSECURITY
-{"SECURITY", },
-#endif
-#ifdef RES
-{"X-Resource", },
-#endif
-#ifdef XF86BIGFONT
-{"XFree86-Bigfont", },
-#endif
-#ifdef XORGSERVER
-#ifdef XFreeXDGA
-{"XFree86-DGA", },
-#endif
-#ifdef XF86DRI
-{"XFree86-DRI", },
-#endif
-#ifdef XF86VIDMODE
-{"XFree86-VidModeExtension", },
-#endif
-#endif
-{"XFIXES", },
-#ifdef PANORAMIX
-{"XINERAMA", },
-#endif
-{"XInputExtension", NULL},
-{"XKEYBOARD", NULL},
-#ifdef XSELINUX
-{"SELinux", },
-#endif
-{"XTEST", },
-#ifdef XV
-{"XVideo", },
-#endif
-};
-
-Bool
-EnableDisableExtension(const char *name, Bool enable)
-{
-ExtensionToggle *ext;
-int i;
-
-for (i = 0; i < ARRAY_SIZE(ExtensionToggleList); i++) {
-ext = [i];
-if (strcmp(name, ext->name) == 0) {
-if (ext->disablePtr != NULL) {
-*ext->disablePtr = !enable;
-return TRUE;
-}
-else {
-/* Extension is always on, impossible to disable */
-return enable;  /* okay if they wanted to enable,
-   fail if they tried to disable */
-}
-}
-}
-
-return FALSE;
-}
-
-void
-EnableDisableExtensionError(const char *name, Bool enable)
-{
-ExtensionToggle *ext;
-int i;
-Bool found = FALSE;
-
-for (i = 0; i < ARRAY_SIZE(ExtensionToggleList); i++) {
-ext = [i];
-if ((strcmp(name, ext->name) == 0) && (ext->disablePtr == NULL)) {
-ErrorF("[mi] Extension \"%s\" can not be disabled\n", name);
-found = TRUE;
-break;
-}
-}
-if (found == FALSE)
-ErrorF("[mi] Extension \"%s\" is not recognized\n", name);
-ErrorF("[mi] Only the following extensions can be run-time %s:\n",
-   enable ? "enabled" : "disabled");
-for (i = 0; i < ARRAY_SIZE(ExtensionToggleList); i++) {
-ext = [i];
-if (ext->disablePtr != NULL) {
-ErrorF("[mi]%s\n", ext->name);
-}
-}
-}
-
 /* List of built-in (statically linked) extensions */
 static const ExtensionModule staticExtensions[] = {
 {GEExtensionInit, "Generic Event Extension", },
@@ -303,6 +181,57 @@ static const ExtensionModule staticExtensions[] = {
 #endif
 };
 
+Bool
+EnableDisableExtension(const char *name, Bool enable)
+{
+const ExtensionModule *ext;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(staticExtensions); i++) {
+ext = [i];
+if (strcmp(name, ext->name) == 0) {
+if (ext->disablePtr != NULL) {
+*ext->disablePtr = !enable;
+return TRUE;
+}
+else {
+/* Extension is always on, impossible to disable */
+return enable;  /* okay if they wanted to enable,
+   fail if they tried to disable */
+}
+}
+}
+
+return FALSE;
+}
+
+void
+EnableDisableExtensionError(const char *name, Bool enable)
+{
+const ExtensionModule *ext;
+int i;
+Bool found = FALSE;
+
+for (i = 0; i < ARRAY_SIZE(staticExtensions); i++) {
+ext = [i];
+if ((strcmp(name, ext->name) == 0) && (ext->disablePtr == NULL)) {
+ErrorF("[mi] Extension \"%s\" can not be disabled\n", name);
+found = TRUE;
+break;
+}
+}
+if (found == FALSE)
+ErrorF("[mi] Extension \"%s\" is not recognized\n", name);
+ErrorF("[mi] Only the following extensions can be run-time %s:\n",
+   enable ? "enabled" : "disabled");
+for (i = 0; i < ARRAY_SIZE(staticExtensions); i++) {
+ext = [i];
+if (ext->disablePtr != NULL) {
+ErrorF("[mi]%s\n", ext->name);
+

[PATCH xserver 7/7] glx: Large commands are context state, not client state

2018-01-10 Thread Adam Jackson 
There's no reason a multithreaded client shouldn't be allowed to
interleave other requests (for other contexts) with a RenderLarge. Move
the check into __glXForceCurrent, and store the state in the context not
the client.

Signed-off-by: Adam Jackson 
---
 glx/glxcmds.c| 70 
 glx/glxcontext.h | 10 
 glx/glxext.c | 31 +
 glx/glxext.h |  1 -
 glx/glxserver.h  | 10 
 5 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 66018f6de..c1b09d5a1 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -1984,6 +1984,18 @@ __glXDisp_GetDrawableAttributesSGIX(__GLXclientState * 
cl, GLbyte * pc)
 ** client library to send batches of GL rendering commands.
 */
 
+/*
+** Reset state used to keep track of large (multi-request) commands.
+*/
+static void
+ResetLargeCommandStatus(__GLXcontext *cx)
+{
+cx->largeCmdBytesSoFar = 0;
+cx->largeCmdBytesTotal = 0;
+cx->largeCmdRequestsSoFar = 0;
+cx->largeCmdRequestsTotal = 0;
+}
+
 /*
 ** Execute all the drawing commands in a request.
 */
@@ -2115,8 +2127,6 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * pc)
 
 glxc = __glXForceCurrent(cl, req->contextTag, );
 if (!glxc) {
-/* Reset in case this isn't 1st request. */
-__glXResetLargeCommandStatus(cl);
 return error;
 }
 if (safe_pad(req->dataBytes) < 0)
@@ -2129,12 +2139,12 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * 
pc)
 if ((req->length << 2) != safe_pad(dataBytes) + sz_xGLXRenderLargeReq) {
 client->errorValue = req->length;
 /* Reset in case this isn't 1st request. */
-__glXResetLargeCommandStatus(cl);
+ResetLargeCommandStatus(glxc);
 return BadLength;
 }
 pc += sz_xGLXRenderLargeReq;
 
-if (cl->largeCmdRequestsSoFar == 0) {
+if (glxc->largeCmdRequestsSoFar == 0) {
 __GLXrenderSizeData entry;
 int extra = 0;
 int left = (req->length << 2) - sz_xGLXRenderLargeReq;
@@ -2193,21 +2203,21 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * 
pc)
 /*
  ** Make enough space in the buffer, then copy the entire request.
  */
-if (cl->largeCmdBufSize < cmdlen) {
-   GLbyte *newbuf = cl->largeCmdBuf;
+if (glxc->largeCmdBufSize < cmdlen) {
+   GLbyte *newbuf = glxc->largeCmdBuf;
 
if (!(newbuf = realloc(newbuf, cmdlen)))
return BadAlloc;
 
-   cl->largeCmdBuf = newbuf;
-cl->largeCmdBufSize = cmdlen;
+   glxc->largeCmdBuf = newbuf;
+glxc->largeCmdBufSize = cmdlen;
 }
-memcpy(cl->largeCmdBuf, pc, dataBytes);
+memcpy(glxc->largeCmdBuf, pc, dataBytes);
 
-cl->largeCmdBytesSoFar = dataBytes;
-cl->largeCmdBytesTotal = cmdlen;
-cl->largeCmdRequestsSoFar = 1;
-cl->largeCmdRequestsTotal = req->requestTotal;
+glxc->largeCmdBytesSoFar = dataBytes;
+glxc->largeCmdBytesTotal = cmdlen;
+glxc->largeCmdRequestsSoFar = 1;
+glxc->largeCmdRequestsTotal = req->requestTotal;
 return Success;
 
 }
@@ -2221,37 +2231,37 @@ __glXDisp_RenderLarge(__GLXclientState * cl, GLbyte * 
pc)
 /*
  ** Check the request number and the total request count.
  */
-if (req->requestNumber != cl->largeCmdRequestsSoFar + 1) {
+if (req->requestNumber != glxc->largeCmdRequestsSoFar + 1) {
 client->errorValue = req->requestNumber;
-__glXResetLargeCommandStatus(cl);
+ResetLargeCommandStatus(glxc);
 return __glXError(GLXBadLargeRequest);
 }
-if (req->requestTotal != cl->largeCmdRequestsTotal) {
+if (req->requestTotal != glxc->largeCmdRequestsTotal) {
 client->errorValue = req->requestTotal;
-__glXResetLargeCommandStatus(cl);
+ResetLargeCommandStatus(glxc);
 return __glXError(GLXBadLargeRequest);
 }
 
 /*
  ** Check that we didn't get too much data.
  */
-if ((bytesSoFar = safe_add(cl->largeCmdBytesSoFar, dataBytes)) < 0) {
+if ((bytesSoFar = safe_add(glxc->largeCmdBytesSoFar, dataBytes)) < 0) {
 client->errorValue = dataBytes;
-__glXResetLargeCommandStatus(cl);
+ResetLargeCommandStatus(glxc);
 return __glXError(GLXBadLargeRequest);
 }
 
-if (bytesSoFar > cl->largeCmdBytesTotal) {
+if (bytesSoFar > glxc->largeCmdBytesTotal) {
 client->errorValue = dataBytes;
-__glXResetLargeCommandStatus(cl);
+ResetLargeCommandStatus(glxc);
 return __glXError(GLXBadLargeRequest);
 }
 
-memcpy(cl->largeCmdBuf + cl->largeCmdBytesSoFar, pc, dataBytes);
-cl->largeCmdBytesSoFar += dataBytes;
-

[PATCH xserver 4/7] glx: Use vnd layer for dispatch (v2)

2018-01-10 Thread Adam Jackson 
The big change here is MakeCurrent and context tag tracking. We now
delegate context tags entirely to the vnd layer, and simply store a
pointer to the context state as the tag data. If a context is deleted
while it's current, we allocate a fake ID for the context and move the
context state there, so the tag data still points to a real context. As
a result we can stop trying so hard to detach the client from contexts
at disconnect time and just let resource destruction handle it.

Since vnd handles all the MakeCurrent protocol now, our request handlers
for it can just be return BadImplementation.

We also remove a bunch of LEGAL_NEW_RESOURCE, because now by the time
we're called vnd has already allocated its tracking resource on that
XID. Note that we only do this for core GLX requests, for vendor private
requests we still need to call LEGAL_NEW_RESOURCE and in addition need
to call up to addXIDMap and friends.

v2: Update to match v2 of the vnd import, and remove more redundant work
like request length checks.

Signed-off-by: Adam Jackson 
---
 configure.ac   |   2 +-
 glx/createcontext.c|   2 -
 glx/glxcmds.c  | 275 --
 glx/glxcmdsswap.c  |  98 +---
 glx/glxext.c   | 329 -
 glx/glxext.h   |   4 +
 glx/glxscreens.h   |   1 +
 glx/glxserver.h|   5 -
 glx/xfont.c|   2 -
 hw/kdrive/ephyr/ephyr.c|   2 +-
 hw/kdrive/ephyr/meson.build|   1 +
 hw/kdrive/src/kdrive.c |   3 +
 hw/vfb/InitOutput.c|   2 +
 hw/vfb/meson.build |   3 +-
 hw/xfree86/Makefile.am |   5 +
 hw/xfree86/common/xf86Init.c   |   2 +-
 hw/xfree86/dixmods/glxmodule.c |   1 +
 hw/xfree86/meson.build |   1 +
 hw/xquartz/darwin.c|   4 +-
 hw/xwayland/Makefile.am|   1 +
 hw/xwayland/meson.build|   1 +
 hw/xwayland/xwayland.c |   2 +
 include/glx_extinit.h  |   5 +-
 23 files changed, 359 insertions(+), 392 deletions(-)

diff --git a/configure.ac b/configure.ac
index 30b4b383d..29ccd986d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1297,7 +1297,7 @@ if test "x$GLX" = xyes; then
PKG_CHECK_MODULES([GL], $GLPROTO $LIBGL)
AC_SUBST(XLIB_CFLAGS)
AC_DEFINE(GLXEXT, 1, [Build GLX extension])
-   GLX_LIBS='$(top_builddir)/glx/libglx.la'
+   GLX_LIBS='$(top_builddir)/glx/libglx.la 
$(top_builddir)/glx/libglxvnd.la'
GLX_SYS_LIBS="$GLX_SYS_LIBS $GL_LIBS"
 else
 GLX=no
diff --git a/glx/createcontext.c b/glx/createcontext.c
index 76316db67..00c23fcdd 100644
--- a/glx/createcontext.c
+++ b/glx/createcontext.c
@@ -123,8 +123,6 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl, 
GLbyte * pc)
 if (req->length != expected_size)
 return BadLength;
 
-LEGAL_NEW_RESOURCE(req->context, client);
-
 /* The GLX_ARB_create_context spec says:
  *
  * "* If  is not a valid GLXFBConfig, GLXBadFBConfig is
diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index f4820d8e4..66018f6de 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -47,6 +47,7 @@
 #include "indirect_table.h"
 #include "indirect_util.h"
 #include "protocol-versions.h"
+#include "glxvndabi.h"
 
 static char GLXServerVendorName[] = "SGI";
 
@@ -135,6 +136,10 @@ _X_HIDDEN int
 validGlxContext(ClientPtr client, XID id, int access_mode,
 __GLXcontext ** context, int *err)
 {
+/* no ghost contexts */
+if (id & SERVER_BIT)
+return FALSE;
+
 *err = dixLookupResourceByType((void **) context, id,
__glXContextRes, client, access_mode);
 if (*err != Success || (*context)->idExists == GL_FALSE) {
@@ -240,8 +245,6 @@ DoCreateContext(__GLXclientState * cl, GLXContextID gcId,
 __GLXcontext *glxc, *shareglxc;
 int err;
 
-LEGAL_NEW_RESOURCE(gcId, client);
-
 /*
  ** Find the display list space that we want to share.
  **
@@ -356,14 +359,11 @@ DoCreateContext(__GLXclientState * cl, GLXContextID gcId,
 int
 __glXDisp_CreateContext(__GLXclientState * cl, GLbyte * pc)
 {
-ClientPtr client = cl->client;
 xGLXCreateContextReq *req = (xGLXCreateContextReq *) pc;
 __GLXconfig *config;
 __GLXscreen *pGlxScreen;
 int err;
 
-REQUEST_SIZE_MATCH(xGLXCreateContextReq);
-
 if (!validGlxScreen(cl->client, req->screen, , ))
 return err;
 if (!validGlxVisual(cl->client, pGlxScreen, req->visual, , ))
@@ -376,14 +376,11 @@ __glXDisp_CreateContext(__GLXclientState * cl, GLbyte * 
pc)
 int
 __glXDisp_CreateNewContext(__GLXclientState * cl, GLbyte * pc)
 {
-ClientPtr client = cl->client;
 xGLXCreateNewContextReq *req = (xGLXCreateNewContextReq *) pc;
 __GLXconfig *config;
 __GLXscreen *pGlxScreen;
 int err;
 
-REQUEST_SIZE_MATCH(xGLXCreateNewContextReq);
-
 if

[PATCH xserver 1/7] miinitext: General cleanup (v2)

2018-01-10 Thread Adam Jackson 
This really just wants to be the list of disable booleans and
initialization functions, and nothing else. Stop including the protocol
headers from extinit.h, remove a stray mention of xgl, and move an
XInput declaration to a better place.

v2: A bunch of drivers assume they'll get the DPMS tokens implicitly,
so add it to globals.h.

Signed-off-by: Adam Jackson 
---
 glx/glxcmds.c   |  1 +
 hw/xfree86/drivers/modesetting/driver.h |  1 +
 include/extinit.h   | 21 
 include/globals.h   |  4 
 include/inputstr.h  |  4 
 mi/miinitext.c  | 34 +++--
 6 files changed, 25 insertions(+), 40 deletions(-)

diff --git a/glx/glxcmds.c b/glx/glxcmds.c
index 2ebf4a350..f4820d8e4 100644
--- a/glx/glxcmds.c
+++ b/glx/glxcmds.c
@@ -37,6 +37,7 @@
 
 #include "glxserver.h"
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/hw/xfree86/drivers/modesetting/driver.h 
b/hw/xfree86/drivers/modesetting/driver.h
index 08dc3b58a..fe835918b 100644
--- a/hw/xfree86/drivers/modesetting/driver.h
+++ b/hw/xfree86/drivers/modesetting/driver.h
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifdef GLAMOR_HAS_GBM
 #define GLAMOR_FOR_XORG 1
diff --git a/include/extinit.h b/include/extinit.h
index 4ad4fcac0..67e300d18 100644
--- a/include/extinit.h
+++ b/include/extinit.h
@@ -69,7 +69,6 @@ extern void DbeExtensionInit(void);
 #endif
 
 #if defined(DPMSExtension)
-#include 
 extern _X_EXPORT Bool noDPMSExtension;
 extern void DPMSExtensionInit(void);
 #endif
@@ -82,7 +81,6 @@ extern _X_EXPORT Bool noGlxExtension;
 #endif
 
 #ifdef PANORAMIX
-#include 
 extern _X_EXPORT Bool noPanoramiXExtension;
 extern void PanoramiXExtensionInit(void);
 #endif
@@ -100,23 +98,18 @@ extern _X_EXPORT Bool noRenderExtension;
 extern void RenderExtensionInit(void);
 
 #if defined(RES)
-#include 
 extern _X_EXPORT Bool noResExtension;
 extern void ResExtensionInit(void);
 #endif
 
 #if defined(SCREENSAVER)
-#include 
 extern _X_EXPORT Bool noScreenSaverExtension;
 extern void ScreenSaverExtensionInit(void);
 #endif
 
-#include 
 extern void ShapeExtensionInit(void);
 
 #ifdef MITSHM
-#include 
-#include 
 extern _X_EXPORT Bool noMITShmExtension;
 extern void ShmExtensionInit(void);
 #endif
@@ -126,14 +119,11 @@ extern void SyncExtensionInit(void);
 extern void XCMiscExtensionInit(void);
 
 #ifdef XCSECURITY
-#include 
-#include "securitysrv.h"
 extern _X_EXPORT Bool noSecurityExtension;
 extern void SecurityExtensionInit(void);
 #endif
 
 #ifdef XF86BIGFONT
-#include 
 extern _X_EXPORT Bool noXFree86BigfontExtension;
 extern void XFree86BigfontExtensionInit(void);
 #endif
@@ -144,40 +134,29 @@ extern _X_EXPORT Bool noXFixesExtension;
 extern void XFixesExtensionInit(void);
 
 extern void XInputExtensionInit(void);
-extern _X_EXPORT void AssignTypeAndName(DeviceIntPtr dev,
-Atom type,
-const char *name);
 
-#include 
 extern void XkbExtensionInit(void);
 
 #if defined(XSELINUX)
-#include "xselinux.h"
 extern _X_EXPORT Bool noSELinuxExtension;
 extern void SELinuxExtensionInit(void);
 #endif
 
 #ifdef XTEST
-#include 
-#include 
 extern void XTestExtensionInit(void);
 #endif
 
 #if defined(XV)
-#include 
-#include 
 extern _X_EXPORT Bool noXvExtension;
 extern void XvExtensionInit(void);
 extern void XvMCExtensionInit(void);
 #endif
 
 #if defined(DRI3)
-#include 
 extern void dri3_extension_init(void);
 #endif
 
 #if defined(PRESENT)
-#include 
 #include "presentext.h"
 #endif
 
diff --git a/include/globals.h b/include/globals.h
index 44314f03f..341ce832c 100644
--- a/include/globals.h
+++ b/include/globals.h
@@ -4,6 +4,10 @@
 
 #include "window.h" /* for WindowPtr */
 #include "extinit.h"
+#ifdef DPMSExtension
+/* sigh, too many drivers assume this */
+#include 
+#endif
 
 /* Global X server variables that are visible to mi, dix, os, and ddx */
 
diff --git a/include/inputstr.h b/include/inputstr.h
index 5f0026b9b..bf35dbf4b 100644
--- a/include/inputstr.h
+++ b/include/inputstr.h
@@ -57,6 +57,10 @@ SOFTWARE.
 #include "geext.h"
 #include "privates.h"
 
+extern _X_EXPORT void AssignTypeAndName(DeviceIntPtr dev,
+Atom type,
+const char *name);
+
 #define BitIsOn(ptr, bit) (!!(((const BYTE *) (ptr))[(bit)>>3] & (1 << ((bit) 
& 7
 #define SetBit(ptr, bit)  (((BYTE *) (ptr))[(bit)>>3] |= (1 << ((bit) & 7)))
 #define ClearBit(ptr, bit) (((BYTE *)(ptr))[(bit)>>3] &= ~(1 << ((bit) & 7)))
diff --git a/mi/miinitext.c b/mi/miinitext.c
index 9e6578d4a..ce9325e29 100644
--- a/mi/miinitext.c
+++ b/mi/miinitext.c
@@ -97,10 +97,6 @@ SOFTWARE.
 #undef DPMSExtension
 #endif
 
-#ifdef HAVE_XGL_CONFIG_H
-#include 
-#endif
-
 #include "misc.h"
 #include "extension.h"
 #include "extinit.h"
@@ -140,7

[PATCH xserver 2/7] glx: Move provider setup interface to glx_extinit.h

2018-01-10 Thread Adam Jackson 
Keeps us from needing to add glx/ to the include path everywhere, since
we can't add it to the dix include path because the header file names
conflict with glxproxy.

Signed-off-by: Adam Jackson 
---
 glx/glxserver.h   | 11 +--
 include/glx_extinit.h | 12 
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/glx/glxserver.h b/glx/glxserver.h
index 31c635b97..8a3e6f98b 100644
--- a/glx/glxserver.h
+++ b/glx/glxserver.h
@@ -63,6 +63,7 @@ typedef struct __GLXcontext __GLXcontext;
 #include "glxscreens.h"
 #include "glxdrawable.h"
 #include "glxcontext.h"
+#include "glx_extinit.h"
 
 extern __GLXscreen *glxGetScreen(ScreenPtr pScreen);
 extern __GLXclientState *glxGetClient(ClientPtr pClient);
@@ -81,16 +82,6 @@ int __glXError(int error);
 
 //
 
-typedef struct __GLXprovider __GLXprovider;
-struct __GLXprovider {
-__GLXscreen *(*screenProbe) (ScreenPtr pScreen);
-const char *name;
-__GLXprovider *next;
-};
-extern __GLXprovider __glXDRISWRastProvider;
-
-void GlxPushProvider(__GLXprovider * provider);
-
 enum {
 GLX_MINIMAL_VISUALS,
 GLX_TYPICAL_VISUALS,
diff --git a/include/glx_extinit.h b/include/glx_extinit.h
index ad4741dd1..710ca6e3e 100644
--- a/include/glx_extinit.h
+++ b/include/glx_extinit.h
@@ -29,6 +29,18 @@
 /* this is separate due to sdksyms pulling in extinit.h */
 #ifdef GLXEXT
 extern void GlxExtensionInit(void);
+
+typedef struct __GLXprovider __GLXprovider;
+typedef struct __GLXscreen __GLXscreen;
+struct __GLXprovider {
+__GLXscreen *(*screenProbe) (ScreenPtr pScreen);
+const char *name;
+__GLXprovider *next;
+};
+extern __GLXprovider __glXDRISWRastProvider;
+
+void GlxPushProvider(__GLXprovider * provider);
+
 #endif
 
 #endif
-- 
2.14.3

___
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

[PATCH xserver 3/7] glx: Import glxvnd server module (v2)

2018-01-10 Thread Adam Jackson 
From: Kyle Brenneman 

This is based on an out-of-tree module written by Kyle:

https://github.com/kbrenneman/libglvnd/tree/server-libglx

I (ajax) did a bunch of cosmetic fixes, ported it off xfree86 API,
added request length checks, and fixed a minor bug or two.

v2: Use separate functions to set/get a context tag's private data, and
call the backend's MakeCurrent when a client disconnects to unbind the
context. (Kyle Brenneman)

Signed-off-by: Adam Jackson 
---
 glx/Makefile.am  |   8 +-
 glx/meson.build  |  21 ++
 glx/vnd_dispatch_stubs.c | 525 +++
 glx/vndcmds.c| 478 ++
 glx/vndext.c | 306 +++
 glx/vndserver.h  | 119 +++
 glx/vndservermapping.c   | 196 ++
 glx/vndservervendor.c|  91 
 glx/vndservervendor.h|  68 ++
 include/Makefile.am  |   1 +
 include/glxvndabi.h  | 307 +++
 include/meson.build  |   1 +
 12 files changed, 2120 insertions(+), 1 deletion(-)
 create mode 100644 glx/vnd_dispatch_stubs.c
 create mode 100644 glx/vndcmds.c
 create mode 100644 glx/vndext.c
 create mode 100644 glx/vndserver.h
 create mode 100644 glx/vndservermapping.c
 create mode 100644 glx/vndservervendor.c
 create mode 100644 glx/vndservervendor.h
 create mode 100644 include/glxvndabi.h

diff --git a/glx/Makefile.am b/glx/Makefile.am
index 699de63b8..cb71a3763 100644
--- a/glx/Makefile.am
+++ b/glx/Makefile.am
@@ -2,7 +2,7 @@ if DRI2
 GLXDRI_LIBRARY = libglxdri.la
 endif
 
-noinst_LTLIBRARIES = libglx.la $(GLXDRI_LIBRARY)
+noinst_LTLIBRARIES = libglx.la $(GLXDRI_LIBRARY) libglxvnd.la
 
 AM_CFLAGS = \
@DIX_CFLAGS@ \
@@ -83,3 +83,9 @@ libglx_la_SOURCES = \
 xfont.c
 
 libglx_la_LIBADD = $(DLOPEN_LIBS)
+
+libglxvnd_la_SOURCES = \
+   vndcmds.c \
+   vndext.c \
+   vndservermapping.c \
+   vndservervendor.c
diff --git a/glx/meson.build b/glx/meson.build
index f13f8f24b..5f93a75a5 100644
--- a/glx/meson.build
+++ b/glx/meson.build
@@ -53,3 +53,24 @@ srcs_glxdri2 = []
 if build_dri2 or build_dri3
 srcs_glxdri2 = files('glxdri2.c')
 endif
+
+srcs_vnd = [
+'vndcmds.c',
+'vndext.c',
+'vndservermapping.c',
+'vndservervendor.c',
+]
+
+libglxvnd = ''
+if build_glx
+libglxvnd = static_library('libglxvnd',
+   srcs_vnd,
+   include_directories: inc,
+dependencies: [
+common_dep,
+dl_dep,
+dependency('glproto', version: '>= 1.4.17'),
+dependency('gl', version: '>= 9.2.0'),
+],
+)
+endif
diff --git a/glx/vnd_dispatch_stubs.c b/glx/vnd_dispatch_stubs.c
new file mode 100644
index 0..629160fe7
--- /dev/null
+++ b/glx/vnd_dispatch_stubs.c
@@ -0,0 +1,525 @@
+
+#include 
+#include 
+#include "vndserver.h"
+
+// HACK: The opcode in old glxproto.h has a typo in it.
+#if !defined(X_GLXCreateContextAttribsARB)
+#define X_GLXCreateContextAttribsARB X_GLXCreateContextAtrribsARB
+#endif
+
+static int dispatch_Render(ClientPtr client)
+{
+REQUEST(xGLXRenderReq);
+CARD32 contextTag;
+GlxServerVendor *vendor = NULL;
+REQUEST_AT_LEAST_SIZE(*stuff);
+contextTag = GlxCheckSwap(client, stuff->contextTag);
+vendor = glxServer.getContextTag(client, contextTag);
+if (vendor != NULL) {
+int ret;
+ret = glxServer.forwardRequest(vendor, client);
+return ret;
+} else {
+client->errorValue = contextTag;
+return GlxErrorBase + GLXBadContextTag;
+}
+}
+static int dispatch_RenderLarge(ClientPtr client)
+{
+REQUEST(xGLXRenderLargeReq);
+CARD32 contextTag;
+GlxServerVendor *vendor = NULL;
+REQUEST_AT_LEAST_SIZE(*stuff);
+contextTag = GlxCheckSwap(client, stuff->contextTag);
+vendor = glxServer.getContextTag(client, contextTag);
+if (vendor != NULL) {
+int ret;
+ret = glxServer.forwardRequest(vendor, client);
+return ret;
+} else {
+client->errorValue = contextTag;
+return GlxErrorBase + GLXBadContextTag;
+}
+}
+static int dispatch_CreateContext(ClientPtr client)
+{
+REQUEST(xGLXCreateContextReq);
+CARD32 screen, context;
+GlxServerVendor *vendor = NULL;
+REQUEST_SIZE_MATCH(*stuff);
+screen = GlxCheckSwap(client, stuff->screen);
+context = GlxCheckSwap(client, stuff->context);
+LEGAL_NEW_RESOURCE(context, client);
+if (screen < screenInfo.numScreens) {
+vendor = glxServer.getVendorForScreen(client, 
screenInfo.screens[screen]);
+}
+if (vendor != NULL) {
+int ret;
+if (!glxServer.addXIDMap(context, vendor)) {
+return BadAlloc;
+}
+ret = glxServer.forwardRequest(vendor, client);
+if (ret != Success) {
+glxServer.removeXIDMap(context);
+}
+return ret;
+} else {
+

[PATCH xserver 0/7] Server-side glvnd v2

2018-01-10 Thread Adam Jackson 
This merges a change to context tag private data management that Kyle
had proposed. In general I'm quite happy with the cleanups this enables;
I have a few more in the pipeline that will require merging the glx
codegen scripts from Mesa to really accomplish, so I'd like to see this
initial roadblock out of the way.

This makes it possible to have different GLX providers on different
screens in Zaphod mode, and to switch between kernel/GLX driver stacks
without needing so much manual configuration.

- ajax

___
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

[PATCH xserver] x86emu: Teach the debug code about varargs

2018-01-10 Thread Adam Jackson 
With -Wformat-nonliteral and a debug build you'd get yelled at here:

../hw/xfree86/x86emu/x86emu/debug.h:188:9: warning: format not a string 
literal, argument types not checked [-Wformat-nonliteral]

To fix this, rewrite the printf code to actually use varargs and the
appropriate format attribute. All callers of DECODE_PRINTF() pass a
string with no % specifiers, so we pass that as the argument to
printf("%s"). For DECODE_PRINTF2() we just pass the args through.

Signed-off-by: Adam Jackson 
---
 hw/xfree86/x86emu/debug.c| 16 ++--
 hw/xfree86/x86emu/x86emu/debug.h |  7 +++
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/hw/xfree86/x86emu/debug.c b/hw/xfree86/x86emu/debug.c
index 72a06ffb8..576ace55e 100644
--- a/hw/xfree86/x86emu/debug.c
+++ b/hw/xfree86/x86emu/debug.c
@@ -40,8 +40,8 @@
 #include "x86emu/x86emui.h"
 #include 
 #include 
-#ifndef NO_SYS_HEADERS
 #include 
+#ifndef NO_SYS_HEADERS
 #include 
 #endif
 
@@ -174,18 +174,14 @@ x86emu_inc_decoded_inst_len(int x)
 }
 
 void
-x86emu_decode_printf(const char *x)
-{
-sprintf(M.x86.decoded_buf + M.x86.enc_str_pos, "%s", x);
-M.x86.enc_str_pos += strlen(x);
-}
-
-void
-x86emu_decode_printf2(const char *x, int y)
+x86emu_decode_printf(const char *x, ...)
 {
+va_list ap;
 char temp[100];
 
-snprintf(temp, sizeof(temp), x, y);
+va_start(ap, x);
+vsnprintf(temp, sizeof(temp), x, ap);
+va_end(ap);
 sprintf(M.x86.decoded_buf + M.x86.enc_str_pos, "%s", temp);
 M.x86.enc_str_pos += strlen(temp);
 }
diff --git a/hw/xfree86/x86emu/x86emu/debug.h b/hw/xfree86/x86emu/x86emu/debug.h
index 385b804dd..1f04b7b65 100644
--- a/hw/xfree86/x86emu/x86emu/debug.h
+++ b/hw/xfree86/x86emu/x86emu/debug.h
@@ -102,9 +102,9 @@
 #ifdef DEBUG
 
 #define DECODE_PRINTF(x)   if (DEBUG_DECODE()) \
-   
x86emu_decode_printf(x)
+   
x86emu_decode_printf("%s",x)
 #define DECODE_PRINTF2(x,y)if (DEBUG_DECODE()) \
-   
x86emu_decode_printf2(x,y)
+   
x86emu_decode_printf(x,y)
 
 /*
  * The following allow us to look at the bytes of an instruction.  The
@@ -189,8 +189,7 @@ extern "C" {/* Use "C" linkage when in 
C++ mode */
 #endif
 
 extern void x86emu_inc_decoded_inst_len(int x);
-extern void x86emu_decode_printf(const char *x);
-extern void x86emu_decode_printf2(const char *x, int y);
+extern void x86emu_decode_printf(const char *x, ...) 
_X_ATTRIBUTE_PRINTF(1,2);
 extern void x86emu_just_disassemble(void);
 extern void x86emu_single_step(void);
 extern void x86emu_end_instr(void);
-- 
2.14.3

___
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

Re: [PATCH xserver 2/2] randr: Use RRCrtcGetScanoutSize() in ProcRRSetScreenSize()

2018-01-10 Thread Keith Packard 
Michel Dänzer  writes:

> Sounds like we only want patch 1 then?

Well, there's a difference between what the software can do and what the
protocol should require. The original reason for the RandR restriction
requiring a frame buffer large enough for all CRTCs was simple hardware
requirements - you can't scanout pixels you haven't allocated.

With shadow buffers, that restriction is no longer necessary. And, in
fact, it's obviously not what you want either -- when rotated at a non
right-angle, you generally want the whole image displayed in through the
CRTC with blank pixels filling in the corners.

Hrm. On consideration of the potential for minor arithmetic rounding
errors between client and server, I actually do think it would be best
to not have this requirement for arbitrary transformations. Otherwise,
the client would have to carefully replicate the precise operations used
in the server to compute the bounds. Given that the matrix provided is
fixed point (which causes visible problems for projective transforms),
the opportunity for error is even higher.

-- 
-keith


signature.asc
Description: PGP signature
___
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

Re: [PATCH xserver 1/2] randr: Fix rotation check in ProcRRSetScreenSize()

2018-01-10 Thread Michel Dänzer 
On 2018-01-09 03:44 AM, Alex Goins wrote:
> ProcRRSetScreenSize() does bounds checking to ensure that none of the CRTCs 
> have
> a viewport that extends beyond the new screen size. In doing so, it accounts 
> for
> if the CRTC is rotated 90 or 270 degrees, swapping width and height.
> 
> However, it does so by testing if crtc->rotation is equal to RR_Rotate_90 or
> RR_Rotate_270. crtc->rotation is a bit mask, and it includes reflection as 
> well
> as rotation. If a CRTC is reflected as well as rotated, it will incorrectly 
> fail
> this test, resulting in incorrect dimensions being used to verify the validity
> of the new screen size. In some cases, this can cause valid uses of
> ProcRRSetScreenSize() to fail with BadMatch.
> 
> This patch fixes the issue by testing that the bits RR_Rotate_90 or
> RR_Rotate_270 are set, rather than testing for equality.
> 
> Signed-off-by: Alex Goins 
> ---
>  randr/rrscreen.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/randr/rrscreen.c b/randr/rrscreen.c
> index d6c4995..f484383 100644
> --- a/randr/rrscreen.c
> +++ b/randr/rrscreen.c
> @@ -272,7 +272,7 @@ ProcRRSetScreenSize(ClientPtr client)
>  int source_height = mode->mode.height;
>  Rotation rotation = crtc->rotation;
>  
> -if (rotation == RR_Rotate_90 || rotation == RR_Rotate_270) {
> +if (rotation & (RR_Rotate_90 | RR_Rotate_270)) {
>  source_width = mode->mode.height;
>  source_height = mode->mode.width;
>  }
> 

Reviewed-by: Michel Dänzer 


-- 
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

Re: [PATCH xserver 2/2] randr: Use RRCrtcGetScanoutSize() in ProcRRSetScreenSize()

2018-01-10 Thread Michel Dänzer 
On 2018-01-09 10:21 PM, Keith Packard wrote:
> Alex Goins  writes:
> 
>> RRCrtcGetScanoutSize() adds a significant amount of complexity compared to
>> simply checking the rotation and swapping width and height, since it 
>> operates by
>> applying the transformation matrix crtc->transform to the mode.
> 
> There's also an implicit assumption that when a transform is applied,
> there will be a shadow buffer and so the scanout buffer need not be
> large enough to hold the whole image. I don't know if that's useful in
> practice, but that is why there are two different paths here.

Sounds like we only want patch 1 then?


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer



signature.asc
Description: OpenPGP digital signature
___
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

XI_Enter/XI_Leave coming from a slave

2018-01-10 Thread David Bokan 
Hello,

Please bear with me as I'm utterly new to X - I'm trying to debug an issue
in Chromium when using xtest. I'm not sure if the bug is in Chromium, X, or
XTest or some combination of the three.

There's two things that are puzzling me:

1) When I click mouse button 1 on a physical device, it seems that there's
some kind of grab going on: I get an XI_Enter event with NotifyGrab and an
XI_Leave with NotifyUngrab. This kind of makes sense except that I've
disabled the explicit device grab (XIGrabDevice) and we don't have any
passive grabs that I can see. So where is this grab coming from? If I press
down and move the pointer out of the window I continue to see motion events.

I tried to make a simple app to repro but couldn't. Using XIGrabButton
doesn't cause XI_Enter/Leave events if the click is with in the window.
So...where could this behavior be coming from? Figuring out how to
reproduce this would help me debug the issue below.

2) This is the real issue I'm debugging. Given the above XI_Enter/Leave
behavior on mouse buttons, It seems that using xtest causes us to get an
extra pair of XI_Enter and XI_Leave events on a mouse down with deviceid ==
4 (xtest pointer) - in addition to the regular ones we get as described
above for the master pointer.

I can kind of repro this in a test app by using XIGrabButton on deviceid=4.
This causes me to get the crossing events with deviceid=4 but I don't get a
second pair for the master pointer.

Any idea what might be happening or advice on how to dig deeper? Anything I
could try (e.g. is there a way to clear all passive grabs?)

Thanks!
David
___
xorg@lists.x.org: X.Org support
Archives: http://lists.freedesktop.org/archives/xorg
Info: https://lists.x.org/mailman/listinfo/xorg
Your subscription address: %(user_address)s