On 06/09/14 01:12 AM, DRC wrote:
Ugh.  I wonder aloud whether it's worth it to continue supporting color
index mode at all.  Are any apps still using that?

I rarely, if ever, have seen it, but then I mostly see apps where there is a rendering/performance issue which somewhat self-selects for newer apps (where all the kinks aren't worked out, and CI rendering is extremely unlikely).

I'd definitely prefer to not introduce a new environment variable.  I
think the best way to do it would be to introduce a new field in the
ContextAttribs structure in ContextHash.h, as well as a new argument to
ContextHash::add() (probably just a boolean specifying whether the
context is CI or not.)  Then you can add ContextHash::isIndex() and
ContextHash::indexCurrent() straightforwardly and use those within the
body of the CI emulation functions to pass through the calls if the
current context isn't CI.

What do you think of the attached patch?  Applies to trunk with

patch -p0 -i only-emulate-ci-for-ci-visuals.patch

It flips most of the CI emulation conditionals around which to me is clearer to read.

You'll notice that glReadPixels() comments out the emulateColorIndex() check. When glReadPixels() is called by VirtualDrawable::readPixels() the current context is the *temp* context, so glxCurrentContext()-based lookups fail. This means that up to now an attempt to glReadPixels() on a remote overlay (if that even works?) would have failed (since overlayCurrent() would never be true). If the format is GL_COLOR_INDEX then we'll get GL_INVALID_OPERATION unless the current context/drawable is CI. Bottom line: to make glReadPixels() work for overlays we'd need a way to query the apps current context (which the the temp current context has replaced).

This fixes the MSC Mentat case I have (working from a trace plaback). It also fixes the simplistic test created by modifing glxdemos/glxdemo.c and adding a glIndex(0) after the glColor() in the redraw() function. Prior to this patch the middle square is black. After patch square is yellow.

Also tests ok with 'glxspheres -c'.

-Nathan

Index: server/faker-glx.cpp
===================================================================
--- server/faker-glx.cpp	(revision 3559)
+++ server/faker-glx.cpp	(working copy)
@@ -527,7 +527,7 @@
 			if(!_XQueryExtension(dpy, "GLX", &dummy, &dummy, &dummy))
 				ctx=NULL;
 			else ctx=_glXCreateContext(dpy, vis, share_list, direct);
-			if(ctx) ctxhash.add(ctx, (GLXFBConfig)-1, -1);
+			if(ctx) ctxhash.add(ctx, (GLXFBConfig)-1, -1, 1);
 			goto done;
 		}
 	}
@@ -542,6 +542,8 @@
 	if(ctx)
 	{
 		int newctxIsDirect=_glXIsDirect(_dpy3D, ctx);
+		int colorindex=(glxvisual::visAttrib2D(dpy, DefaultScreen(dpy), vis->visualid,
+			GLX_RENDER_TYPE)==GLX_COLOR_INDEX_TYPE);
 		if(!newctxIsDirect && direct)
 		{
 			vglout.println("[VGL] WARNING: The OpenGL rendering context obtained on X display");
@@ -553,7 +555,7 @@
 		}
 		// Hash the FB config to the context so we can use it in subsequent calls
 		// to glXMake[Context]Current().
-		ctxhash.add(ctx, config, newctxIsDirect);
+		ctxhash.add(ctx, config, newctxIsDirect, colorindex);
 	}
 
 	CATCH();
@@ -590,7 +592,7 @@
 	{
 		ctx=_glXCreateContextAttribsARB(dpy, config, share_context, direct,
 			attribs);
-		if(ctx) ctxhash.add(ctx, (GLXFBConfig)-1, -1);
+		if(ctx) ctxhash.add(ctx, (GLXFBConfig)-1, -1, 1);
 		goto done;
 	}
 
@@ -610,6 +612,9 @@
 	if(ctx)
 	{
 		int newctxIsDirect=_glXIsDirect(_dpy3D, ctx);
+		VisualID vid=cfghash.getVisual(dpy, config);
+		int colorindex=(vid && glxvisual::visAttrib2D(dpy, DefaultScreen(dpy), vid,
+			GLX_RENDER_TYPE)==GLX_COLOR_INDEX_TYPE);
 		if(!newctxIsDirect && direct)
 		{
 			vglout.println("[VGL] WARNING: The OpenGL rendering context obtained on X display");
@@ -619,7 +624,7 @@
 				DisplayString(_dpy3D));
 			vglout.println("[VGL]    permissions may be set incorrectly.");
 		}
-		ctxhash.add(ctx, config, newctxIsDirect);
+		ctxhash.add(ctx, config, newctxIsDirect, colorindex);
 	}
 
 	CATCH();
@@ -653,7 +658,7 @@
 	if(rcfghash.isOverlay(dpy, config))
 	{
 		ctx=_glXCreateNewContext(dpy, config, render_type, share_list, direct);
-		if(ctx) ctxhash.add(ctx, (GLXFBConfig)-1, -1);
+		if(ctx) ctxhash.add(ctx, (GLXFBConfig)-1, -1, 1);
 		goto done;
 	}
 
@@ -661,6 +666,9 @@
 	if(ctx)
 	{
 		int newctxIsDirect=_glXIsDirect(_dpy3D, ctx);
+		VisualID vid=cfghash.getVisual(dpy, config);
+		int colorindex=(vid && glxvisual::visAttrib2D(dpy, DefaultScreen(dpy), vid,
+			GLX_RENDER_TYPE)==GLX_COLOR_INDEX_TYPE);
 		if(!newctxIsDirect && direct)
 		{
 			vglout.println("[VGL] WARNING: The OpenGL rendering context obtained on X display");
@@ -670,7 +678,7 @@
 				DisplayString(_dpy3D));
 			vglout.println("[VGL]    permissions may be set incorrectly.");
 		}
-		ctxhash.add(ctx, config, newctxIsDirect);
+		ctxhash.add(ctx, config, newctxIsDirect, colorindex);
 	}
 
 	CATCH();
Index: server/faker-gl.cpp
===================================================================
--- server/faker-gl.cpp	(revision 3559)
+++ server/faker-gl.cpp	(working copy)
@@ -230,10 +230,15 @@
 // The following functions are interposed in order to make color index
 // rendering work, since most platforms don't support color index Pbuffers.
 
+static int emulateColorIndex(void)
+{
+	return (ctxhash.colorindexCurrent() && !ctxhash.overlayCurrent());
+}
+
 void glClearIndex(GLfloat c)
 {
-	if(ctxhash.overlayCurrent()) _glClearIndex(c);
-	else glClearColor(c/255., 0., 0., 0.);
+	if(emulateColorIndex()) glClearColor(c/255., 0., 0., 0.);
+	else _glClearIndex(c);
 }
 
 
@@ -253,7 +258,7 @@
 {
 	TRY();
 
-	if(format==GL_COLOR_INDEX && !ctxhash.overlayCurrent() && type!=GL_BITMAP)
+	if(format==GL_COLOR_INDEX && emulateColorIndex() && type!=GL_BITMAP)
 	{
 		format=GL_RED;
 		if(type==GL_BYTE || type==GL_UNSIGNED_BYTE) type=GL_UNSIGNED_BYTE;
@@ -286,7 +291,7 @@
 
 void glGetDoublev(GLenum pname, GLdouble *params)
 {
-	if(!ctxhash.overlayCurrent())
+	if(emulateColorIndex())
 	{
 		if(pname==GL_CURRENT_INDEX)
 		{
@@ -321,7 +326,7 @@
 
 void glGetFloatv(GLenum pname, GLfloat *params)
 {
-	if(!ctxhash.overlayCurrent())
+	if(emulateColorIndex())
 	{
 		if(pname==GL_CURRENT_INDEX)
 		{
@@ -358,7 +363,7 @@
 
 void glGetIntegerv(GLenum pname, GLint *params)
 {
-	if(!ctxhash.overlayCurrent())
+	if(emulateColorIndex())
 	{
 		if(pname==GL_CURRENT_INDEX)
 		{
@@ -395,82 +400,82 @@
 
 void glIndexd(GLdouble index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexd(index);
-	else glColor3d(index/255., 0.0, 0.0);
+	if(emulateColorIndex()) glColor3d(index/255., 0.0, 0.0);
+	else _glIndexd(index);
 }
 
 void glIndexf(GLfloat index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexf(index);
-	else glColor3f(index/255., 0., 0.);
+	if(emulateColorIndex()) glColor3f(index/255., 0., 0.);
+	else _glIndexf(index);
 }
 
 void glIndexi(GLint index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexi(index);
-	else glColor3f((GLfloat)index/255., 0, 0);
+	if(emulateColorIndex()) glColor3f((GLfloat)index/255., 0, 0);
+	else _glIndexi(index);
 }
 
 void glIndexs(GLshort index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexs(index);
-	else glColor3f((GLfloat)index/255., 0, 0);
+	if(emulateColorIndex()) glColor3f((GLfloat)index/255., 0, 0);
+	else _glIndexs(index);
 }
 
 void glIndexub(GLubyte index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexub(index);
+	if(emulateColorIndex()) _glIndexub(index);
 	else glColor3f((GLfloat)index/255., 0, 0);
 }
 
 void glIndexdv(const GLdouble *index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexdv(index);
-	else
+	if(emulateColorIndex())
 	{
 		GLdouble color[3]={index? (*index)/255.:0., 0., 0.};
 		glColor3dv(index? color:NULL);
 	}
+	else _glIndexdv(index);
 }
 
 void glIndexfv(const GLfloat *index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexfv(index);
-	else
+	if(emulateColorIndex())
 	{
 		GLfloat color[3]={index? (*index)/255.0f:0.0f, 0.0f, 0.0f};
 		glColor3fv(index? color:NULL);  return;
 	}
+	else _glIndexfv(index);
 }
 
 void glIndexiv(const GLint *index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexiv(index);
-	else
+	if(emulateColorIndex())
 	{
 		GLfloat color[3]={index? (GLfloat)(*index)/255.0f:0.0f, 0.0f, 0.0f};
 		glColor3fv(index? color:NULL);  return;
 	}
+	else _glIndexiv(index);
 }
 
 void glIndexsv(const GLshort *index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexsv(index);
-	else
+	if(emulateColorIndex())
 	{
 		GLfloat color[3]={index? (GLfloat)(*index)/255.0f:0.0f, 0.0f, 0.0f};
 		glColor3fv(index? color:NULL);  return;
 	}
+	else _glIndexsv(index);
 }
 
 void glIndexubv(const GLubyte *index)
 {
-	if(ctxhash.overlayCurrent()) _glIndexubv(index);
-	else
+	if(emulateColorIndex())
 	{
 		GLfloat color[3]={index? (GLfloat)(*index)/255.0f:0.0f, 0.0f, 0.0f};
 		glColor3fv(index? color:NULL);  return;
 	}
+	else _glIndexubv(index);
 }
 
 
@@ -510,7 +515,7 @@
 
 void glPixelTransferf(GLenum pname, GLfloat param)
 {
-	if(!ctxhash.overlayCurrent())
+	if(emulateColorIndex())
 	{
 		if(pname==GL_INDEX_SHIFT)
 		{
@@ -529,7 +534,7 @@
 
 void glPixelTransferi(GLenum pname, GLint param)
 {
-	if(!ctxhash.overlayCurrent())
+	if(emulateColorIndex())
 	{
 		if(pname==GL_INDEX_SHIFT)
 		{
@@ -563,7 +568,7 @@
 {
 	TRY();
 
-	if(format==GL_COLOR_INDEX && !ctxhash.overlayCurrent() && type!=GL_BITMAP)
+	if(format==GL_COLOR_INDEX /*&& emulateColorIndex()*/ && type!=GL_BITMAP)
 	{
 		format=GL_RED;
 		if(type==GL_BYTE || type==GL_UNSIGNED_BYTE) type=GL_UNSIGNED_BYTE;
Index: server/ContextHash.h
===================================================================
--- server/ContextHash.h	(revision 3559)
+++ server/ContextHash.h	(working copy)
@@ -24,6 +24,7 @@
 {
 	GLXFBConfig config;
 	Bool direct;
+	bool colorindex; // from *app* perspective
 } ContextAttribs;
 
 
@@ -49,13 +50,14 @@
 
 			static bool isAlloc(void) { return (instance!=NULL); }
 
-			void add(GLXContext ctx, GLXFBConfig config, Bool direct)
+			void add(GLXContext ctx, GLXFBConfig config, Bool direct, bool colorindex)
 			{
 				if(!ctx || !config) _throw("Invalid argument");
 				ContextAttribs *attribs=NULL;
 				_newcheck(attribs=new ContextAttribs);
 				attribs->config=config;
 				attribs->direct=direct;
+				attribs->colorindex=colorindex;
 				HASH::add(ctx, NULL, attribs);
 			}
 
@@ -83,11 +85,26 @@
 				return -1;
 			}
 
+			bool isColorIndex(GLXContext ctx)
+			{
+				if(ctx)
+				{
+					ContextAttribs *attribs=HASH::find(ctx, NULL);
+					if(attribs) return attribs->colorindex;
+				}
+				return false;
+			}
+
 			bool overlayCurrent(void)
 			{
 				return isOverlay(glXGetCurrentContext());
 			}
 
+			bool colorindexCurrent(void)
+			{
+				return isColorIndex(glXGetCurrentContext());
+			}
+
 			void remove(GLXContext ctx)
 			{
 				if(ctx) HASH::remove(ctx, NULL);
Index: server/glxvisual.cpp
===================================================================
--- server/glxvisual.cpp	(revision 3559)
+++ server/glxvisual.cpp	(working copy)
@@ -316,6 +316,11 @@
 			{
 				return (va[i].isStereo && va[i].isGL && va[i].isDB);
 			}
+			if(attribute==GLX_RENDER_TYPE)
+			{
+				if(va[i].c_class==TrueColor) return GLX_RGBA_TYPE;
+				else return GLX_COLOR_INDEX_TYPE;
+			}
 		}
 	}
 	return 0;

------------------------------------------------------------------------------
Want excitement?
Manually upgrade your production database.
When you want reliability, choose Perforce.
Perforce version control. Predictably reliable.
http://pubads.g.doubleclick.net/gampad/clk?id=157508191&iu=/4140/ostg.clktrk
_______________________________________________
VirtualGL-Devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/virtualgl-devel

Reply via email to