On Sat, May 5, 2012 at 10:19 AM, Roland Scheidegger <srol...@vmware.com> wrote:
> Am 05.05.2012 04:07, schrieb Liu Aleaxander:
>> On Sat, May 5, 2012 at 9:44 AM, Roland Scheidegger <srol...@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....@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....@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:
>From cdbbff1d8223f248734c6e1b63a1f79011fb59a4 Mon Sep 17 00:00:00 2001
From: Yuanhan Liu <yuanhan....@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
v3: Roland: check texture fallback just for color format
Signed-off-by: Yuanhan Liu <yuanhan....@linux.intel.com>
---
src/mesa/drivers/common/meta.c | 45 +++++++++++++++++++++++++--------------
1 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/src/mesa/drivers/common/meta.c b/src/mesa/drivers/common/meta.c
index 95336fc..3d22462 100644
--- a/src/mesa/drivers/common/meta.c
+++ b/src/mesa/drivers/common/meta.c
@@ -2206,28 +2206,41 @@ _mesa_meta_DrawPixels(struct gl_context *ctx,
*/
fallback = GL_FALSE;
if (ctx->_ImageTransferState ||
+ ctx->FragmentProgram._Enabled ||
ctx->Fog.Enabled) {
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
- * apparent i965 driver bug (see bug #23670).
+ /*
+ * If there are already some texture unit enabled, a fallback is needed,
+ * 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 (/*format == GL_LUMINANCE ||*/ format == GL_LUMINANCE_ALPHA)
- texIntFormat = format;
- else
- texIntFormat = GL_RGBA;
-
- /* If we're not supposed to clamp the resulting color, then just
- * promote our texture to fully float. We could do better by
- * just going for the matching set of channels, in floating
- * point.
- */
- if (ctx->Color.ClampFragmentColor != GL_TRUE &&
- ctx->Extensions.ARB_texture_float)
- texIntFormat = GL_RGBA32F;
+ if (ctx->Texture._EnabledUnits) {
+ fallback = GL_TRUE;
+ }
+ else {
+ /* use more compact format when possible */
+ /* XXX disable special case for GL_LUMINANCE for now to work around
+ * apparent i965 driver bug (see bug #23670).
+ */
+ if (/*format == GL_LUMINANCE ||*/ format == GL_LUMINANCE_ALPHA)
+ texIntFormat = format;
+ else
+ texIntFormat = GL_RGBA;
+
+ /* If we're not supposed to clamp the resulting color, then just
+ * promote our texture to fully float. We could do better by
+ * just going for the matching set of channels, in floating
+ * point.
+ */
+ if (ctx->Color.ClampFragmentColor != GL_TRUE &&
+ ctx->Extensions.ARB_texture_float)
+ texIntFormat = GL_RGBA32F;
+ }
}
else if (_mesa_is_stencil_format(format)) {
if (ctx->Extensions.ARB_fragment_program &&
--
1.7.3.1
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
