Menu
▾
▴
mesa3d-dev
|
From: Luca B. <lu...@lu...> - 2010-03-23 16:14:34
|
Commit bd1ce874728c06d08a1f9881f51edbdd2f1c9db0 "st/dri: Switch from st_public.h to st_api.h" broke etracer on Gallium nvfx. The root cause appears to be that etracer calls glClear with the stencil bit set, but does not ask for a stencil buffer. Enabling "stencil_buffer" in the etracer config works around the issue. This results in strb being null, and the check for strb->surface in st_clear segfaulting. I'm not sure why that commit would cause this: I guess it unintentionally indirectly changes the way st/dri handles stencil buffers. Changing if(strb->surface) to if(strb && strb->surface) everywhere inside st_clear fixes the issue, but I'm not at all sure it is the right fix. Should st_clear check strb for null or is it supposed to be always non-null? (and the bug is thus somewhere else?) Did that commit just expose a bug, or is it buggy itself? |
|
From: Luca B. <lu...@lu...> - 2010-03-23 16:18:17
|
Apparently _mesa_Clear should not set BUFFER_BIT_STENCIL if the visual lacks a stencil buffer. So it seems the visual is somehow incorrect, or a visual with a stencil buffer is being selected but the stencil renderbuffer is not actually set. |
|
From: Brian P. <br...@vm...> - 2010-03-23 16:24:55
|
Luca Barbieri wrote: > Apparently _mesa_Clear should not set BUFFER_BIT_STENCIL if the visual > lacks a stencil buffer. > > So it seems the visual is somehow incorrect, or a visual with a > stencil buffer is being selected but the stencil renderbuffer is not > actually set. At line 167 of src/mesa/main/clear.c we check if the user specifies GL_STENCIL_BUFFER_BIT but the drawing surface has no stencil bits. So, st_clear() should not be getting BUFFER_BIT_STENCIL if there's no stencil buffer. Perhaps you could debug that a bit further. -Brian |
|
From: Luca B. <lu...@lu...> - 2010-03-23 16:33:42
|
We have a visual haveStencilBuffer == 1 but stencilBits == 0 (and no
stencil renderbuffer), which I suppose shouldn't be happening.
visualid and fbconfigid are also 0.
Here is the full structure:
$1 = {next = 0x0, rgbMode = 1 '\001', floatMode = 0 '\000',
colorIndexMode = 0 '\000', doubleBufferMode = 1, stereoMode = 0,
haveAccumBuffer = 0 '\000', haveDepthBuffer = 1 '\001',
haveStencilBuffer = 1 '\001', redBits = 8, greenBits = 8, blueBits =
8, alphaBits = 8, redMask = 0, greenMask = 0, blueMask = 0, alphaMask
= 0, rgbBits = 32, indexBits = 0, accumRedBits = 0,
accumGreenBits = 0, accumBlueBits = 0, accumAlphaBits = 0, depthBits
= 24, stencilBits = 0, numAuxBuffers = 0, level = 0, pixmapMode = 0,
visualID = 0, visualType = 0, visualRating = 0, transparentPixel = 0,
transparentRed = 0, transparentGreen = 0, transparentBlue = 0,
transparentAlpha = 0, transparentIndex = 0, sampleBuffers = 0, samples
= 0, drawableType = 0, renderType = 0, xRenderable = 0, fbconfigID =
0,
maxPbufferWidth = 0, maxPbufferHeight = 0, maxPbufferPixels = 0,
optimalPbufferWidth = 0, optimalPbufferHeight = 0, visualSelectGroup =
0, swapMethod = 0, screen = 0, bindToTextureRgb = 0,
bindToTextureRgba = 0, bindToMipmapTexture = 0, bindToTextureTargets
= 0, yInverted = 0}
BTW, what's the purpose of having haveStencilBuffer at all? Isn't
checking stencilBits != 0 enough?
|
|
From: Luca B. <lu...@lu...> - 2010-03-23 16:39:45
|
The problem seems to be in st_manager.c:
if (visual->depth_stencil_format != PIPE_FORMAT_NONE) {
mode->haveDepthBuffer = GL_TRUE;
mode->haveStencilBuffer = GL_TRUE;
mode->depthBits =
util_format_get_component_bits(visual->depth_stencil_format,
UTIL_FORMAT_COLORSPACE_ZS, 0);
mode->stencilBits =
util_format_get_component_bits(visual->depth_stencil_format,
UTIL_FORMAT_COLORSPACE_ZS, 1);
}
This sets haveStencilBuffer even for depth-only buffers.
How about fixing this to set haveDepthBuffer and haveStencilBuffer
only if bits > 0, and later removing haveStencilBuffer,
haveDepthBuffer and haveAccumBuffer in favor of just testing the *bits
counterparts?
BTW, what if we have a floating-point depth buffer, or, say, a shared
exponent floating-point color buffer? How do we represent that with
the visual structure?
Shouldn't we be using the actual formats instead of this *bits stuff,
maybe by having Mesa look at its internal structures instead of a
GLXVisual-like struct?
|
|
From: Luca B. <lu...@lu...> - 2010-03-23 16:47:11
|
Fixes a segfault when clearing a non-existent stencil buffer.
---
src/mesa/state_tracker/st_manager.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
index ca3a29c..cac62e4 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -333,15 +333,15 @@ st_visual_to_context_mode(const struct st_visual *visual,
}
if (visual->depth_stencil_format != PIPE_FORMAT_NONE) {
- mode->haveDepthBuffer = GL_TRUE;
- mode->haveStencilBuffer = GL_TRUE;
-
mode->depthBits =
util_format_get_component_bits(visual->depth_stencil_format,
UTIL_FORMAT_COLORSPACE_ZS, 0);
mode->stencilBits =
util_format_get_component_bits(visual->depth_stencil_format,
UTIL_FORMAT_COLORSPACE_ZS, 1);
+
+ mode->haveDepthBuffer = mode->depthBits > 0;
+ mode->haveStencilBuffer = mode->stencilBits > 0;
}
if (visual->accum_format != PIPE_FORMAT_NONE) {
--
1.7.0.1.147.g6d84b
|
|
From: Brian P. <br...@vm...> - 2010-03-23 16:57:29
|
Luca Barbieri wrote:
> Fixes a segfault when clearing a non-existent stencil buffer.
> ---
> src/mesa/state_tracker/st_manager.c | 6 +++---
> 1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
> index ca3a29c..cac62e4 100644
> --- a/src/mesa/state_tracker/st_manager.c
> +++ b/src/mesa/state_tracker/st_manager.c
> @@ -333,15 +333,15 @@ st_visual_to_context_mode(const struct st_visual *visual,
> }
>
> if (visual->depth_stencil_format != PIPE_FORMAT_NONE) {
> - mode->haveDepthBuffer = GL_TRUE;
> - mode->haveStencilBuffer = GL_TRUE;
> -
> mode->depthBits =
> util_format_get_component_bits(visual->depth_stencil_format,
> UTIL_FORMAT_COLORSPACE_ZS, 0);
> mode->stencilBits =
> util_format_get_component_bits(visual->depth_stencil_format,
> UTIL_FORMAT_COLORSPACE_ZS, 1);
> +
> + mode->haveDepthBuffer = mode->depthBits > 0;
> + mode->haveStencilBuffer = mode->stencilBits > 0;
> }
>
> if (visual->accum_format != PIPE_FORMAT_NONE) {
Looks good.
-Brian
|
|
From: Luca B. <lu...@lu...> - 2010-03-23 17:08:13
|
OK, pushed. |
|
From: Brian P. <br...@vm...> - 2010-03-23 16:56:50
|
Luca Barbieri wrote:
> The problem seems to be in st_manager.c:
> if (visual->depth_stencil_format != PIPE_FORMAT_NONE) {
> mode->haveDepthBuffer = GL_TRUE;
> mode->haveStencilBuffer = GL_TRUE;
>
> mode->depthBits =
> util_format_get_component_bits(visual->depth_stencil_format,
> UTIL_FORMAT_COLORSPACE_ZS, 0);
> mode->stencilBits =
> util_format_get_component_bits(visual->depth_stencil_format,
> UTIL_FORMAT_COLORSPACE_ZS, 1);
> }
>
> This sets haveStencilBuffer even for depth-only buffers.
>
> How about fixing this to set haveDepthBuffer and haveStencilBuffer
> only if bits > 0, and later removing haveStencilBuffer,
> haveDepthBuffer and haveAccumBuffer in favor of just testing the *bits
> counterparts?
The haveDepth/Stencil fields come from the original SGI GLX. We
should probably just test the number of bits in Mesa, as you say.
Want to make a patch for that?
> BTW, what if we have a floating-point depth buffer, or, say, a shared
> exponent floating-point color buffer? How do we represent that with
> the visual structure?
Those things came along after the SGI GLX release. They'd have to be
added to __GLcontextModes.
> Shouldn't we be using the actual formats instead of this *bits stuff,
> maybe by having Mesa look at its internal structures instead of a
> GLXVisual-like struct?
yeah, probably. I'd have to study it for a while.
-Brian
|
|
From: Chia-I Wu <ol...@gm...> - 2010-03-24 00:30:50
|
On Wed, Mar 24, 2010 at 12:56 AM, Brian Paul <br...@vm...> wrote:
>> The problem seems to be in st_manager.c:
>> if (visual->depth_stencil_format != PIPE_FORMAT_NONE) {
>> mode->haveDepthBuffer = GL_TRUE;
>> mode->haveStencilBuffer = GL_TRUE;
>>
>> mode->depthBits =
>> util_format_get_component_bits(visual->depth_stencil_format,
>> UTIL_FORMAT_COLORSPACE_ZS, 0);
>> mode->stencilBits =
>> util_format_get_component_bits(visual->depth_stencil_format,
>> UTIL_FORMAT_COLORSPACE_ZS, 1);
>> }
>>
>> This sets haveStencilBuffer even for depth-only buffers.
Yes, I think this is the root cause. Thanks for looking into and fixing it.
>> How about fixing this to set haveDepthBuffer and haveStencilBuffer
>> only if bits > 0, and later removing haveStencilBuffer,
>> haveDepthBuffer and haveAccumBuffer in favor of just testing the *bits
>> counterparts?
> The haveDepth/Stencil fields come from the original SGI GLX. We should
> probably just test the number of bits in Mesa, as you say. Want to make a
> patch for that?
>> BTW, what if we have a floating-point depth buffer, or, say, a shared
>> exponent floating-point color buffer? How do we represent that with
>> the visual structure?
> Those things came along after the SGI GLX release. They'd have to be added
> to __GLcontextModes.
>> Shouldn't we be using the actual formats instead of this *bits stuff,
>> maybe by having Mesa look at its internal structures instead of a
>> GLXVisual-like struct?
> yeah, probably. I'd have to study it for a while.
That sounds good. Many fields of __GLcontextModes are of no interest in Mesa
core. It might make some sense not to store __GLcontextModes in the contexts
and the framebuffers after creation.
--
ol...@Lu...
|
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X