On 02/01/2016 03:51 PM, Miklós Máté wrote:
On 12/19/2015 12:24 AM, Marek Olšák wrote:
On Fri, Dec 18, 2015 at 11:45 PM, Miklós Máté<mtm...@gmail.com>
wrote:
On 12/17/2015 01:06 PM, Marek Olšák wrote:
On Wed, Dec 16, 2015 at 11:30 PM, Miklós Máté<mtm...@gmail.com>
wrote:
On 12/16/2015 05:27 PM, Marek Olšák wrote:
What is this good for?
Marek
KotOR uses a series of scratch framebuffers for drawing the
framebuffer
effects. These have no depth and no stencil, so check_compatible()
rejects
them, subsequent GL calls are no-op, and the screen becomes
garbage. I
also
experimented successfully with disabling the visuals that have no
depth
or
no stencil in gallium/state_trackers/dri/dri_screen.c, but I
concluded
that
this one was a smaller hack.
What kind of scratch buffer are we talking about? How is it created?
Marek
They are pbuffers, created like this:
glXChooseFBConfig(dpy = 0x7cbe2280, screen = 0, attribList =
[GLX_RED_SIZE,
8, GLX_GREEN_SIZE, 8, GLX_BLUE_SIZE, 8, GLX_ALPHA_SIZE, 8,
GLX_DOUBLEBUFFER,
False, GLX_DRAWABLE_TYPE, GLX_PBUFFER_BIT, GLX_RENDER_TYPE,
GLX_RGBA_BIT |
GLX_COLOR_INDEX_BIT | 0xfffffffffffffffc, 0], nitems = [80])
glXGetFBConfigAttrib(dpy = 0x7cbe2280, config = 0x7ccf08a0, attribute =
GLX_FBCONFIG_ID, value = [104])
glXCreatePbuffer(dpy = 0x7cbe2280, config = 0x7ccf08a0, attribList =
[GLX_PBUFFER_WIDTH, 64, GLX_PBUFFER_HEIGHT, 64, 0])
Since depth is unspecified in the attrib list (I checked that if the
game
had supplied WGL_DEPTH_BITS_ARB to wglChoosePixelFormatARB, Wine
would have
added GLX_DEPTH_SIZE to the attrib list), the first fbconfig is
chosen that
has 8 bits of r,g,b, which happens to be the very first config in
the list,
and it has no depth or stencil.
I noticed that the list of visuals that glxinfo prints has two elements
prepended that look like a bugfix for a similar problem. Maybe that
would be
the optimal solution in this case as well?
Yes. If reordering the FBConfigs fixes the issue, it would be an
acceptable workaround I think.
Marek
I'm ready to post v2 of my patch series, except this one (well, I
haven't been able to tackle any of the 3 problems mentioned in 00/11
either, but those are a different story).
Quick recap: the problem is that KotOR uses a series of pbuffers to
draw its post-process effects, but their visuals (depth=0, stencil=0
because they are not specified in the attrib list) are incompatible
with the gl context, so the gl calls are no-op, and it blits back
uninitialized data onto the final image.
I found 3 ways to work around this issue (tested with radeonsi), but I
can't decide which one is the smallest/acceptable hack:
1. remove the compatibility check in main/context.c (this is what
patch 05/11 does) -- honestly I don't know why ctx needs a visual, it
should be sufficient to use the visual of the currently bound buffer;
moreover, I couldn't find these compatibility criteria in the glX
specification
2. in src/glx/glxcmds.c fbconfig_compare(): use PREFER_LARGER() for
depth and stencil -- this violates the glX specification, but it makes
glXChooseFBConfig() return a list that starts with a config that is
compatible with the ctx (KotOR uses the first config from the list)
3. in gallium/state_trackers/dri/dri_screen.c dri_fill_in_modes():
disable all the modes that are not z24s8 -- this is probably the worst
option, because z32 must also be disabled or it gets chosen over z24s8
(BTW is it really necessary to advertise 600 fbconfigs? fglrx only has
about 50)
I just filed bugreport #93955 with a test code about the first issue
mentioned in 00/11; that test code also demonstrates this issue (see
the comments in the code).
MM
I started working on option 1, as I think that one is a progressive
approach, and it doesn't violate specs. I found that the only legit user
of gl_context::Visual was the compatibility check, and it was marked for
removal by Brian Paul in 2007 (the last time it was touched). How does
the attached patch look like? I'd like to replace the current 05/11 with
this one in v2 of my series.
MM
diff --git a/src/mesa/drivers/dri/radeon/radeon_common_context.c b/src/mesa/drivers/dri/radeon/radeon_common_context.c
index 4660d98..2989f63 100644
--- a/src/mesa/drivers/dri/radeon/radeon_common_context.c
+++ b/src/mesa/drivers/dri/radeon/radeon_common_context.c
@@ -604,7 +604,7 @@ GLboolean radeonMakeCurrent(__DRIcontext * driContextPriv,
}
if(driDrawPriv == NULL && driReadPriv == NULL) {
- drfb = _mesa_create_framebuffer(&radeon->glCtx.Visual);
+ drfb = _mesa_get_incomplete_framebuffer();
readfb = drfb;
}
else {
diff --git a/src/mesa/main/blend.c b/src/mesa/main/blend.c
index 2ae22e9..28e2dbf 100644
--- a/src/mesa/main/blend.c
+++ b/src/mesa/main/blend.c
@@ -921,7 +921,7 @@ _mesa_get_render_format(const struct gl_context *ctx, mesa_format format)
* Initializes the related fields in the context color attribute group,
* __struct gl_contextRec::Color.
*/
-void _mesa_init_color( struct gl_context * ctx )
+void _mesa_init_color( struct gl_context * ctx, GLuint doubleBufferMode )
{
GLuint i;
@@ -951,7 +951,7 @@ void _mesa_init_color( struct gl_context * ctx )
/* GL_FRONT is not possible on GLES. Instead GL_BACK will render to either
* the front or the back buffer depending on the config */
- if (ctx->Visual.doubleBufferMode || _mesa_is_gles(ctx)) {
+ if (doubleBufferMode || _mesa_is_gles(ctx)) {
ctx->Color.DrawBuffer[0] = GL_BACK;
}
else {
diff --git a/src/mesa/main/blend.h b/src/mesa/main/blend.h
index 8ab9e02..f4854a6 100644
--- a/src/mesa/main/blend.h
+++ b/src/mesa/main/blend.h
@@ -124,7 +124,7 @@ _mesa_update_clamp_vertex_color(struct gl_context *ctx,
extern mesa_format
_mesa_get_render_format(const struct gl_context *ctx, mesa_format format);
-extern void
-_mesa_init_color( struct gl_context * ctx );
+extern void
+_mesa_init_color( struct gl_context * ctx, GLuint doubleBufferMode );
#endif
diff --git a/src/mesa/main/context.c b/src/mesa/main/context.c
index f3fd01f..ea51424 100644
--- a/src/mesa/main/context.c
+++ b/src/mesa/main/context.c
@@ -796,7 +796,7 @@ check_context_limits(struct gl_context *ctx)
* functions for the more complex data structures.
*/
static GLboolean
-init_attrib_groups(struct gl_context *ctx)
+init_attrib_groups(struct gl_context *ctx, GLuint doubleBufferMode)
{
assert(ctx);
@@ -810,7 +810,7 @@ init_attrib_groups(struct gl_context *ctx)
_mesa_init_accum( ctx );
_mesa_init_attrib( ctx );
_mesa_init_buffer_objects( ctx );
- _mesa_init_color( ctx );
+ _mesa_init_color( ctx, doubleBufferMode );
_mesa_init_current( ctx );
_mesa_init_depth( ctx );
_mesa_init_debug( ctx );
@@ -828,7 +828,7 @@ init_attrib_groups(struct gl_context *ctx)
_mesa_init_multisample( ctx );
_mesa_init_performance_monitors( ctx );
_mesa_init_pipeline( ctx );
- _mesa_init_pixel( ctx );
+ _mesa_init_pixel( ctx, doubleBufferMode );
_mesa_init_pixelstore( ctx );
_mesa_init_point( ctx );
_mesa_init_polygon( ctx );
@@ -1159,15 +1159,6 @@ _mesa_initialize_context(struct gl_context *ctx,
ctx->WinSysDrawBuffer = NULL;
ctx->WinSysReadBuffer = NULL;
- if (visual) {
- ctx->Visual = *visual;
- ctx->HasConfig = GL_TRUE;
- }
- else {
- memset(&ctx->Visual, 0, sizeof ctx->Visual);
- ctx->HasConfig = GL_FALSE;
- }
-
_mesa_override_gl_version(ctx);
/* misc one-time initializations */
@@ -1193,7 +1184,7 @@ _mesa_initialize_context(struct gl_context *ctx,
_mesa_reference_shared_state(ctx, &ctx->Shared, shared);
- if (!init_attrib_groups( ctx ))
+ if (!init_attrib_groups( ctx, visual->doubleBufferMode ))
goto fail;
/* setup the API dispatch tables with all nop functions */
@@ -1521,57 +1512,6 @@ _mesa_copy_context( const struct gl_context *src, struct gl_context *dst,
/**
- * Check if the given context can render into the given framebuffer
- * by checking visual attributes.
- *
- * Most of these tests could go away because Mesa is now pretty flexible
- * in terms of mixing rendering contexts with framebuffers. As long
- * as RGB vs. CI mode agree, we're probably good.
- *
- * \return GL_TRUE if compatible, GL_FALSE otherwise.
- */
-static GLboolean
-check_compatible(const struct gl_context *ctx,
- const struct gl_framebuffer *buffer)
-{
- const struct gl_config *ctxvis = &ctx->Visual;
- const struct gl_config *bufvis = &buffer->Visual;
-
- if (buffer == _mesa_get_incomplete_framebuffer())
- return GL_TRUE;
-
-#if 0
- /* disabling this fixes the fgl_glxgears pbuffer demo */
- if (ctxvis->doubleBufferMode && !bufvis->doubleBufferMode)
- return GL_FALSE;
-#endif
- if (ctxvis->stereoMode && !bufvis->stereoMode)
- return GL_FALSE;
- if (ctxvis->haveAccumBuffer && !bufvis->haveAccumBuffer)
- return GL_FALSE;
- if (ctxvis->haveDepthBuffer && !bufvis->haveDepthBuffer)
- return GL_FALSE;
- if (ctxvis->haveStencilBuffer && !bufvis->haveStencilBuffer)
- return GL_FALSE;
- if (ctxvis->redMask && ctxvis->redMask != bufvis->redMask)
- return GL_FALSE;
- if (ctxvis->greenMask && ctxvis->greenMask != bufvis->greenMask)
- return GL_FALSE;
- if (ctxvis->blueMask && ctxvis->blueMask != bufvis->blueMask)
- return GL_FALSE;
-#if 0
- /* disabled (see bug 11161) */
- if (ctxvis->depthBits && ctxvis->depthBits != bufvis->depthBits)
- return GL_FALSE;
-#endif
- if (ctxvis->stencilBits && ctxvis->stencilBits != bufvis->stencilBits)
- return GL_FALSE;
-
- return GL_TRUE;
-}
-
-
-/**
* Check if the viewport/scissor size has not yet been initialized.
* Initialize the size if the given width and height are non-zero.
*/
@@ -1614,7 +1554,7 @@ handle_first_current(struct gl_context *ctx)
/* According to GL_MESA_configless_context the default value of
* glDrawBuffers depends on the config of the first surface it is bound to.
* For GLES it is always GL_BACK which has a magic interpretation */
- if (!ctx->HasConfig && _mesa_is_desktop_gl(ctx)) {
+ if (_mesa_is_desktop_gl(ctx)) {
if (ctx->DrawBuffer != _mesa_get_incomplete_framebuffer()) {
if (ctx->DrawBuffer->Visual.doubleBufferMode)
buffer = GL_BACK;
@@ -1673,24 +1613,7 @@ _mesa_make_current( struct gl_context *newCtx,
if (MESA_VERBOSE & VERBOSE_API)
_mesa_debug(newCtx, "_mesa_make_current()\n");
- /* Check that the context's and framebuffer's visuals are compatible.
- */
- if (newCtx && drawBuffer && newCtx->WinSysDrawBuffer != drawBuffer) {
- if (!check_compatible(newCtx, drawBuffer)) {
- _mesa_warning(newCtx,
- "MakeCurrent: incompatible visuals for context and drawbuffer");
- return GL_FALSE;
- }
- }
- if (newCtx && readBuffer && newCtx->WinSysReadBuffer != readBuffer) {
- if (!check_compatible(newCtx, readBuffer)) {
- _mesa_warning(newCtx,
- "MakeCurrent: incompatible visuals for context and readbuffer");
- return GL_FALSE;
- }
- }
-
- if (curCtx &&
+ if (curCtx &&
(curCtx->WinSysDrawBuffer || curCtx->WinSysReadBuffer) &&
/* make sure this context is valid for flushing */
curCtx != newCtx &&
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 15dd1ca..17f379d 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4290,7 +4290,6 @@ struct gl_context
struct _glapi_table *CurrentDispatch;
/*@}*/
- struct gl_config Visual;
struct gl_framebuffer *DrawBuffer; /**< buffer for writing */
struct gl_framebuffer *ReadBuffer; /**< buffer for reading */
struct gl_framebuffer *WinSysDrawBuffer; /**< set with MakeCurrent */
@@ -4527,12 +4526,6 @@ struct gl_context
GLboolean FirstTimeCurrent;
/*@}*/
- /**
- * False if this context was created without a config. This is needed
- * because the initial state of glDrawBuffers depends on this
- */
- GLboolean HasConfig;
-
/** software compression/decompression supported or not */
GLboolean Mesa_DXTn;
diff --git a/src/mesa/main/pixel.c b/src/mesa/main/pixel.c
index 608a545..ffdb31b 100644
--- a/src/mesa/main/pixel.c
+++ b/src/mesa/main/pixel.c
@@ -649,7 +649,7 @@ init_pixelmap(struct gl_pixelmap *map)
* Initialize the context's PIXEL attribute group.
*/
void
-_mesa_init_pixel( struct gl_context *ctx )
+_mesa_init_pixel( struct gl_context *ctx, GLuint doubleBufferMode )
{
/* Pixel group */
ctx->Pixel.RedBias = 0.0;
@@ -679,7 +679,7 @@ _mesa_init_pixel( struct gl_context *ctx )
init_pixelmap(&ctx->PixelMaps.BtoB);
init_pixelmap(&ctx->PixelMaps.AtoA);
- if (ctx->Visual.doubleBufferMode) {
+ if (doubleBufferMode) {
ctx->Pixel.ReadBuffer = GL_BACK;
}
else {
diff --git a/src/mesa/main/pixel.h b/src/mesa/main/pixel.h
index fd1782e..51e4b51 100644
--- a/src/mesa/main/pixel.h
+++ b/src/mesa/main/pixel.h
@@ -66,8 +66,8 @@ _mesa_PixelTransferi( GLenum pname, GLint param );
extern void
_mesa_update_pixel( struct gl_context *ctx, GLuint newstate );
-extern void
-_mesa_init_pixel( struct gl_context * ctx );
+extern void
+_mesa_init_pixel( struct gl_context * ctx, GLuint doubleBufferMode );
/*@}*/
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
