[Mesa-dev] [PATCH 2/6] mesa: Fix GL error generation in glBlitFramebuffer()

[Anuj Phogat]

On Tue, Dec 18, 2012 at 11:09 AM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 12/12/2012 03:25 PM, Anuj Phogat wrote:
>>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>>   src/mesa/main/fbobject.c |   10 +++++++++-
>>   1 files changed, 9 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/mesa/main/fbobject.c b/src/mesa/main/fbobject.c
>> index 4b7d4ab..9e32836 100644
>> --- a/src/mesa/main/fbobject.c
>> +++ b/src/mesa/main/fbobject.c
>> @@ -2856,7 +2856,15 @@ _mesa_BlitFramebuffer(GLint srcX0, GLint srcY0,
>> GLint srcX1, GLint srcY1,
>>          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)) {
>> +               _mesa_get_format_bits(drawRb->Format, GL_STENCIL_BITS) ||
>> +               /* OpenGL/ES specification is not very clear about it. But
>> GLES3
>> +                * conformance test expects GL_INVALID_OPERATION in case
>> of below
>> +                * listed combination of formats for read/draw buffers.
>> +                */
>> +               (readRb->InternalFormat == GL_DEPTH32F_STENCIL8 &&
>> +                drawRb->InternalFormat == GL_DEPTH24_STENCIL8) ||
>> +               (readRb->InternalFormat == GL_DEPTH24_STENCIL8 &&
>> +                drawRb->InternalFormat == GL_DEPTH32F_STENCIL8)) {
>>          /* There is no need to check the stencil datatype here, because
>>           * there is only one: GL_UNSIGNED_INT.
>>           */
>
>
> I found this text:
>
> From the OpenGL 3.0 spec, section 4.3 (page 277):
> "Calling BlitFramebuffer 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."
>
> This text also appears in the ES 3.0 and GL 4.2 specs.
>
> I believe the existing code's interpretation of this paragraph was:
>
>   - if DEPTH_BUFFER_BIT is set, source & destination depth buffer formats
> must match.
>   - if STENCIL_BUFFER_BIT is set, source & destination stencil buffer
> formats must match.
>
> An alternate interpretation that might appease those tests is:
>
>   - if *either* DEPTH_BUFFER_BIT or STENCIL_BUFFER_BIT is set, both the
> depth *and* stencil formats must match.
>
> If that's the case, you can simply check readRb->Format == drawRb->Format.
> It seems reasonable to me, at least for packed formats.

gles3 conformance test cases expect no GL error when blitting between
formats GL_STENCIL_INDEX8 and GL_DEPTH24_STENCIL8.
But expect invalid operation when blitting between formats
GL_DEPTH24_STENCIL8 and GL_DEPTH32F_STENCIL8. This behavior
doesn't seem to match above interpretation of spec.
Making the changes suggested by you and Ian cause many other
gles3 conform BlitFrameBuffer test cases fail.