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

[Kenneth Graunke]

On 12/18/2012 12:05 PM, Anuj Phogat wrote:
> 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.

Okay.  That seems reasonable too, since:

"If a buffer is specified in mask and does not exist in both the read 
and draw framebuffers, the corresponding bit is silently ignored."

Reading between the lines a bit, perhaps when GL_STENCIL_BIT is set, the 
depth formats 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.  That would allow the GL_STENCIL_INDEX8/GL_DEPTH24_STENCIL8 case.

Likewise, if GL_DEPTH_BIT is set, the stencil formats must 
match...unless one of the two buffers doesn't have stencil.