[Mesa-dev] [PATCH] mesa: Fix GL_LUMINANCE handling for compressed textures in glGetTexImage
Anuj Phogat
anuj.phogat at gmail.com
Fri Nov 16 16:06:58 PST 2012
On Fri, Nov 16, 2012 at 3:28 PM, Ian Romanick <idr at freedesktop.org> wrote:
> On 11/16/2012 01:21 PM, Anuj Phogat wrote:
>>
>> We need to rebase colors (ex: set G=B=0) when getting GL_LUMINANCE
>> textures in following cases:
>> 1. If the luminance texture is actually stored as rgba
>> 2. If getting a luminance texture, but returning rgba
>> 3. If getting an rgba texture, but returning luminance
>>
>> A similar fix was pushed by Brian Paul for uncompressed textures
>> in commit: f5d0ced.
>>
>> Fixes intel oglconform pxconv-gettex test:
>> https://bugs.freedesktop.org/show_bug.cgi?id=47220
>>
>> Observed no failures in piglit and ogles2conform due to this fix.
>> This patch will cause failures in intel oglconform pxstore-gettex
>> and pxtrans-gettex test cases. The cause of a failures is a bug
>> in test cases. Expected luminance value is calculted incorrectly:
>
> calculated
>
>> L = R+G+B.
>
>
> This seems weird. We fail pxconv-gettex because we convert the luminance
> texture supplied by the application to RGB by replicating the L value to
> each of R, G, and B. In this case, and this case only, we need to read back
> luminance by just returning R. If the application actually specified an RGB
> texture, we need to read back luminance by R+G+B. Right?
>
If a texture is stored in RGB/RGBA internal format and we later read it back
using glGetTexImage. There is no way to find out the format of initial texture
data supplied by application later in the driver. For glGetTexImage we
return L = Tex(R). This is what we do in mesa for uncompressed textures.
it's different from glReadPixels which returns L=R+G+B.
> Does this match what pxconv-gettex and pxconv-trans expect?
>
> This L vs. RGB crap confuses me every time...
>
Yeah this is confusing. pxstore-gettex and pxtrans-gettex expects
L=R+G+B when getting an rgba texture, but returning luminance.
Sometime back I submitted a fix to change the expected values
in pxconv-gettex to L = tex(R). Similar fix is required for failing two
test cases.
>
>> Note: This is a candifate for stable branches.
>
> candidate
>
>
>>
>> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
>> ---
>> src/mesa/drivers/common/meta.c | 16 +++++++++++++---
>> src/mesa/main/texgetimage.c | 34 ++++++++++++++++++++++++----------
>> 2 files changed, 37 insertions(+), 13 deletions(-)
>>
>> diff --git a/src/mesa/drivers/common/meta.c
>> b/src/mesa/drivers/common/meta.c
>> index 417dbd0..e019140 100644
>> --- a/src/mesa/drivers/common/meta.c
>> +++ b/src/mesa/drivers/common/meta.c
>> @@ -3685,6 +3685,7 @@ decompress_texture_image(struct gl_context *ctx,
>> /* read pixels from renderbuffer */
>> {
>> GLenum baseTexFormat = texImage->_BaseFormat;
>> + GLenum destBaseFormat = _mesa_base_tex_format(ctx, destFormat);
>>
>> /* The pixel transfer state will be set to default values at this
>> point
>> * (see MESA_META_PIXEL_TRANSFER) so pixel transfer ops are
>> effectively
>> @@ -3693,9 +3694,18 @@ decompress_texture_image(struct gl_context *ctx,
>> * returned as red and two-channel texture values are returned as
>> * red/alpha.
>> */
>> - if (baseTexFormat == GL_LUMINANCE ||
>> - baseTexFormat == GL_LUMINANCE_ALPHA ||
>> - baseTexFormat == GL_INTENSITY) {
>> + if ((baseTexFormat == GL_LUMINANCE ||
>> + baseTexFormat == GL_LUMINANCE_ALPHA ||
>> + baseTexFormat == GL_INTENSITY) ||
>> + /* If we're reading back an RGB(A) texture (using
>> glGetTexImage) as
>> + * luminance then we need to return L=tex(R).
>> + */
>> + ((baseTexFormat == GL_RGBA ||
>> + baseTexFormat == GL_RGB) &&
>> + (destBaseFormat == GL_LUMINANCE ||
>> + destBaseFormat == GL_LUMINANCE_ALPHA ||
>> + destBaseFormat == GL_LUMINANCE_INTEGER_EXT ||
>> + destBaseFormat == GL_LUMINANCE_ALPHA_INTEGER_EXT))) {
>> /* Green and blue must be zero */
>> _mesa_PixelTransferf(GL_GREEN_SCALE, 0.0f);
>> _mesa_PixelTransferf(GL_BLUE_SCALE, 0.0f);
>> diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c
>> index 2ccdceb..bd6d481 100644
>> --- a/src/mesa/main/texgetimage.c
>> +++ b/src/mesa/main/texgetimage.c
>> @@ -229,6 +229,8 @@ get_tex_rgba_compressed(struct gl_context *ctx, GLuint
>> dimensions,
>> const gl_format texFormat =
>> _mesa_get_srgb_format_linear(texImage->TexFormat);
>> const GLenum baseFormat = _mesa_get_format_base_format(texFormat);
>> + const GLenum destBaseFormat = _mesa_base_tex_format(ctx, format);
>> + GLenum rebaseFormat = GL_NONE;
>> const GLuint width = texImage->Width;
>> const GLuint height = texImage->Height;
>> const GLuint depth = texImage->Depth;
>> @@ -266,9 +268,30 @@ get_tex_rgba_compressed(struct gl_context *ctx,
>> GLuint dimensions,
>> }
>>
>> if (baseFormat == GL_LUMINANCE ||
>> + baseFormat == GL_INTENSITY ||
>> baseFormat == GL_LUMINANCE_ALPHA) {
>> + /* If a luminance (or intensity) texture is read back as RGB(A),
>> the
>> + * returned value should be (L,0,0,1), not (L,L,L,1). Set
>> rebaseFormat
>> + * here to get G=B=0.
>> + */
>> + rebaseFormat = texImage->_BaseFormat;
>> + }
>> + else if ((baseFormat == GL_RGBA ||
>> + baseFormat == GL_RGB) &&
>> + (destBaseFormat == GL_LUMINANCE ||
>> + destBaseFormat == GL_LUMINANCE_ALPHA ||
>> + destBaseFormat == GL_LUMINANCE_INTEGER_EXT ||
>> + destBaseFormat == GL_LUMINANCE_ALPHA_INTEGER_EXT)) {
>> + /* If we're reading back an RGB(A) texture as luminance then we
>> need
>> + * to return L=tex(R). Note, that's different from glReadPixels
>> which
>> + * returns L=R+G+B.
>> + */
>> + rebaseFormat = GL_LUMINANCE_ALPHA; /* this covers GL_LUMINANCE too
>> */
>> + }
>> +
>> + if (rebaseFormat) {
>> _mesa_rebase_rgba_float(width * height, (GLfloat (*)[4])
>> tempImage,
>> - baseFormat);
>> + rebaseFormat);
>> }
>>
>> srcRow = tempImage;
>> @@ -419,15 +442,6 @@ get_tex_rgba(struct gl_context *ctx, GLuint
>> dimensions,
>> transferOps |= IMAGE_CLAMP_BIT;
>> }
>> }
>> - /* This applies to RGB, RGBA textures. if the format is either
>> LUMINANCE
>> - * or LUMINANCE ALPHA, luminance (L) is computed as L=R+G+B .we need
>> to
>> - * clamp the sum to [0,1].
>> - */
>> - else if ((format == GL_LUMINANCE ||
>> - format == GL_LUMINANCE_ALPHA) &&
>> - dataType == GL_UNSIGNED_NORMALIZED) {
>> - transferOps |= IMAGE_CLAMP_BIT;
>> - }
>>
>> if (_mesa_is_format_compressed(texImage->TexFormat)) {
>> get_tex_rgba_compressed(ctx, dimensions, format, type,
>>
>
More information about the mesa-dev
mailing list