[Mesa-dev] [PATCH] meta: do fallback when texture is enabled for DrawPixels

[Liu Aleaxander]

On Sat, May 5, 2012 at 10:19 AM, Roland Scheidegger <sroland at vmware.com> wrote:
> Am 05.05.2012 04:07, schrieb Liu Aleaxander:
>> On Sat, May 5, 2012 at 9:44 AM, Roland Scheidegger <sroland at vmware.com> wrote:
>> [snip]...
>>
>>>> +    * or the original texture stuff would not work
>>>> +    *
>>>> +    * FIXME: we may use multi-texture to handle this case. But fallback is
>>>> +    * definitely a much simple and straight way.
>>>> +    */
>>>> +   if (ctx->Texture._EnabledUnits)
>>>> +      fallback = GL_TRUE;
>>>> +
>>>>     if (_mesa_is_color_format(format)) {
>>>>        /* use more compact format when possible */
>>>>        /* XXX disable special case for GL_LUMINANCE for now to work around
>>>
>>> Wouldn't you also have to test for active fragment shader too (i.e.
>>> essentially same test that _mesa_meta_Bitmap is doing I think they might
>>> have the same prerequisites for fallback for the hilarious combination
>>> of these ops with ordinary fragment shading)? Though I guess maybe you
>>> could avoid fallback in some cases (like drawing to stencil buffer then
>>> texture bound shouldn't matter I think though a fragment shader writing
>>> to depth or killing fragments still would).
>>
>> Yes and thanks. And here is the new patch based on your comments:
>>
>> From 1e1bacb8cba6ee55b4659613f8a9037b4f54d9c5 Mon Sep 17 00:00:00 2001
>> From: Yuanhan Liu <yuanhan.liu at linux.intel.com>
>> Date: Fri, 4 May 2012 22:23:37 +0800
>> Subject: [PATCH] meta: do fallback when texture is enabled for DrawPixels
>>
>> If there are already some texture unit enabled, a fallback is needed,
>> or the original texture stuff would not work.
>>
>> A much better way is to use multi-texture to handle this case: like
>> treat the pixels as texture 1 and the original texture as texture 2.
>> I haven't do much inverstigation on that way, but fallback is definitely
>> a much simpler and straight way.
>>
>> This would fix oglc mipsel test case.
>>
>> v2: Roland: do fallback also for active fragment shader
>>             no need to fallback for stencil buffer
>>
>> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
>> ---
>>  src/mesa/drivers/common/meta.c |   13 +++++++++++++
>>  1 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
>> index 95336fc..eedc889 100644
>> --- a/src/mesa/drivers/common/meta.c
>> +++ b/src/mesa/drivers/common/meta.c
>> @@ -2206,10 +2206,23 @@ _mesa_meta_DrawPixels(struct gl_context *ctx,
>>      */
>>     fallback = GL_FALSE;
>>     if (ctx->_ImageTransferState ||
>> +       ctx->FragmentProgram._Enabled ||
>>         ctx->Fog.Enabled) {
>>        fallback = GL_TRUE;
>>     }
>>
>> +   /*
>> +    * If there are already some texture unit enabled, a fallback is needed,
>> +    * or the original texture stuff would not work
>> +    *
>> +    * No need to fallback if drawing to stencil buffer.
>> +    *
>> +    * FIXME: we may use multi-texture to handle this case. But fallback is
>> +    * definitely a much simple and straight way.
>> +    */
>> +   if (ctx->Texture._EnabledUnits && !_mesa_is_stencil_format(format))
>> +      fallback = GL_TRUE;
>> +
>>     if (_mesa_is_color_format(format)) {
>>        /* use more compact format when possible */
>>        /* XXX disable special case for GL_LUMINANCE for now to work around
>
> Well I think it should also apply to depth buffer though I wouldn't be
> concerned over that.

I guess I just need move those code into the if (_mesa_is_color_format()) block.

> If really the same rules apply to the bitmap case
> though I think it would be great if that could be seen more easily (even
> if it might not be worth to share the code).

Yeah, I see no much worth.

> Though whatever works there is really fine by me, I doubt any apps
> really do that, and even if they do a swrast fallback isn't really
> better than misrendering anyway :-).

Well, this patch severed as fixing an OpenGL conformance test case ;)

Then, here is another much 'complicate' one that moved the texture
check code into the color format block: