On 01/17/2012 11:00 PM, Kenneth Graunke wrote:
On 01/17/2012 05:27 PM, Ian Romanick wrote:
On 01/17/2012 05:12 PM, Chad Versace wrote:
This reverts commit 3f1fab06844f696de44d9a56e83ff62e8ea576bd.
This loosens the format validation in glBlitFramebuffer. When blitting
depth bits, don't require an exact match between the depth formats;
only
require that the two formats have the same number of depth bits. Ditto
for
stencil.
Fixes Piglit fbo/fbo-blit-d24s8 on Intel drivers with separate stencil
enabled.
The problem was that, on Intel drivers with separate stencil, the
default
framebuffer has separate depth and stencil buffers with formats X8_Z24
and
S8. The test attempts to blit the depth bits from a S8_Z24 buffer into
the
default framebuffer.
Between S8_Z24 buffers, the EXT_framebuffer_blit spec allows
glBlitFramebuffer to blit the depth and stencil bits separately. So
I see
no reason to prevent blitting the depth bits between X8_Z24 and
S8_Z24 or
the stencil bits between S8 and S8_Z24.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=44665
Note: This is a candidate for the 8.0 branch.
CC: Brian Paul<bri...@wmware.com>
Reported-by: Xunx Fang<xunx.f...@intel.com>
Signed-off-by: Chad Versace<chad.vers...@linux.intel.com>
---
src/mesa/main/fbobject.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 0524959..9a5e780 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -2709,9 +2709,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint
srcY0, GLint srcX1, GLint srcY1,
if ((readRb == NULL) || (drawRb == NULL)) {
mask&= ~GL_STENCIL_BUFFER_BIT;
}
- else if (readRb->Format != drawRb->Format) {
+ else if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
+ _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
_mesa_error(ctx, GL_INVALID_OPERATION,
- "glBlitFramebufferEXT(stencil buffer format mismatch)");
+ "glBlitFramebufferEXT(stencil buffer size mismatch)");
return;
}
}
@@ -2731,9 +2732,10 @@ _mesa_BlitFramebufferEXT(GLint srcX0, GLint
srcY0, GLint srcX1, GLint srcY1,
if ((readRb == NULL) || (drawRb == NULL)) {
mask&= ~GL_DEPTH_BUFFER_BIT;
}
- else if (readRb->Format != drawRb->Format) {
+ else if (_mesa_get_format_bits(readRb->Format, GL_DEPTH_BITS) !=
+ _mesa_get_format_bits(drawRb->Format, GL_DEPTH_BITS)) {
I think the check needs to be slightly stronger than this. This would
allow Z32 and Z32_FLOAT which is outside the intention of the spec.
Do we want to allow "X-types", like S8Z24->X8Z24 and
Yes, I think we want to support that type of case.
ARGB8888->XRGB8888,
Yes, the spec explicitly allows color format conversion:
"If the color formats of the read and draw framebuffers do not
match, and <mask> includes COLOR_BUFFER_BIT, the pixel groups are
converted to match the destination format as in CopyPixels, except
that no pixel transfer operations apply and clamping behaves as if
CLAMP_FRAGMENT_COLOR_ARB is set to FIXED_ONLY_ARB."
It's the Z / stencil case that's a bit vague and the reason why I
changed the test in the first place:
"Calling BlitFramebufferEXT will result in an INVALID_OPERATION
error if <mask> includes DEPTH_BUFFER_BIT or STENCIL_BUFFER_BIT
and the source and destination depth and stencil buffer formats do
not match."
but not general format mismatches (like
Z32<->Z32_FLOAT)?
That should probably not be allowed (but it would be interesting to
test with an AMD or NVIDIA driver to see what they do).
I think you just need to test _mesa_get_format_datatype(src) ==
_mesa_get_format_datatype(dst) in addition to the depth comparison.
Thinking about FBOs...I could see raising an error if the user
explicitly made ARGB8888 and XRGB8888 FBOs, but if they chose "RGB"
and the driver internally decided to use one or the other, it seems
wrong to throw an error.
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
