On 01/08/2013 02:03 AM, Anuj Phogat wrote:
On Tue, Jan 8, 2013 at 5:25 AM, Kenneth Graunke <kenn...@whitecape.org> wrote:
From: Anuj Phogat <anuj.pho...@gmail.com>
V2:
If mask has GL_STENCIL_BUFFER_BIT set, the depth formats for readRenderBuffer
and drawRenderBuffer must match unless one of the two buffers doesn't have
depth, in which case it's not blitted, so the format check should be ignored.
Same comment goes for stencil formats in depth renderbuffers if mask has
GL_DEPTH_BUFFER_BIT set.
v3 (Kayden): Refactor code to be a bit more readable.
Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
Signed-off-by: Kenneth Graunke <kenn...@whitecape.org>
---
src/mesa/main/fbobject.c | 59 ++++++++++++++++++++++++++++++++++++------------
1 file changed, 44 insertions(+), 15 deletions(-)
Anuj,
This looks correct, but the run-on conditionals are pretty hard to read...
just too much going on in a single if-statement.
How about splitting them up a bit? Here's my proposed version. It also
gives slightly more descriptive messages.
I'm happy with the refactored code except a small mistake I explained below.
Untested.
diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
index 281cdd0..bf613ff 100644
--- a/src/mesa/main/fbobject.c
+++ b/src/mesa/main/fbobject.c
@@ -2833,14 +2833,28 @@ _mesa_BlitFramebuffer(GLint srcX0, GLint srcY0, GLint
srcX1, GLint srcY1,
if ((readRb == NULL) || (drawRb == NULL)) {
mask &= ~GL_STENCIL_BUFFER_BIT;
}
- else if (_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
- _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) {
- /* There is no need to check the stencil datatype here, because
- * there is only one: GL_UNSIGNED_INT.
- */
- _mesa_error(ctx, GL_INVALID_OPERATION,
- "glBlitFramebufferEXT(stencil buffer size mismatch)");
- return;
+ else {
+ if ((_mesa_get_format_bits(readRb->Format, GL_STENCIL_BITS) !=
+ _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS)) ||
+ (_mesa_get_format_datatype(readRb->Format) !=
+ _mesa_get_format_datatype(drawRb->Format))) {
Matching the data type is not required here as data type for stencil buffer is
always GL_UNSIGNED_INT. But, data type of GL_DEPTH24_STENCIL8 is
GL_UNSIGNED_NORMALIZED. So, matching the data types here will disallow
blitting between GL_STENCIL_INDEX8 and GL_DEPTH24_STENCIL8 which is a
valid blitting case. This caused 7 gles3 conformance blitframebuffer test cases
fail.
Oops. I thought that would be harmless, since it'd always be the same
in the stencil-only case, and needed to match in the depth-only case.
But you're right. Sorry for the trouble.
I'm fine with you pushing your series now (other than my comments on 7.1/7).
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
