[Mesa-dev] [PATCH] Revert "mesa: check depth, stencil formats (not depths) in glBlitFramebuffer"

[Kenneth Graunke]

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<brianp at wmware.com>
>> Reported-by: Xunx Fang<xunx.fang at intel.com>
>> Signed-off-by: Chad Versace<chad.versace at 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 ARGB8888->XRGB8888, 
but not general format mismatches (like Z32<->Z32_FLOAT)?

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.