Bug 35852 - [bisected pineview] oglc case pxconv-read failed
[bisected pineview] oglc case pxconv-read failed
Status: VERIFIED FIXED
Product: Mesa
Classification: Unclassified
Component: Mesa core
git
All Linux (All)
: high normal
Assigned To: Marek Olšák
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-31 20:07 UTC by fangxun
Modified: 2011-05-05 03:37 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
workaround (994 bytes, patch)
2011-04-18 13:56 UTC, Marek Olšák
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description fangxun 2011-03-31 20:07:16 UTC
System Environment:
--------------------------
Arch:           i386
Platform:       Pineview
Libdrm: (master)2.4.24-9-g5cb554a0d6e986f2d7300a91d95983fa09b17f65
Mesa:   (master)5eb9f687087a4bc71775a32efcd848fc6cd67694
Xserver:(master)xorg-server-1.10.0-143-g327e1d88012102af6aca6c6840aa0ed3c7041a77
Xf86_video_intel:(master)2.14.902-2-g630d77bf10ba6234bb9c04538636f7d8aa319aea
Kernel: (drm-intel-next)f0c860246472248a534656d6cdbed5a36d1feb2e


Bug detailed description:
-------------------------
This is regression. There are other three cases failed due to the same cause: pxstore-read, pxtrans-read, pbo subcase texImage.1PBOChangeInternalFormat.
Bisect shows e5c6a92a12b5cd7db205d72039f58d302b0be9d5 is the first bad commit.
commit e5c6a92a12b5cd7db205d72039f58d302b0be9d5
Author:     Marek Olšák <maraeo@gmail.com>
AuthorDate: Tue Feb 15 23:30:23 2011 +0100
Commit:     Marek Olšák <maraeo@gmail.com>
CommitDate: Tue Mar 29 12:04:55 2011 +0200

    mesa: implement clamping controls (ARB_color_buffer_float)

    Squashed commit of the following:

    Author: Marek Olšák <maraeo@gmail.com>

        mesa: fix getteximage so that it doesn't clamp values
        mesa: update the compute_version function
        mesa: add display list support for ARB_color_buffer_float
        mesa: fix glGet query with GL_ALPHA_TEST_REF and ARB_color_buffer_float
Comment 1 Ian Romanick 2011-04-02 17:27:01 UTC
Each of these tests has many, many subtests.  Do we have any idea what specifically is failing?
Comment 2 Ian Romanick 2011-04-02 17:33:59 UTC
I've also noticed that glean's readPixSanity fails on master but passes on 7.10.  Do you know if that bisects to the same commit?
Comment 3 fangxun 2011-04-05 18:24:42 UTC
glean's readPixSanity failure is not the bisect commit caused.

Following is oglc pxconv-read subtest error info:
PixelConvertCompareRead ERROR: Test failed with format PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_HALF_FLOAT_ARB, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {2.266}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {-1.000}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267}. Expected {1.000}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_HALF_FLOAT_ARB, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {2.266, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255, 0.945}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.255, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {-1.000, 0.949}. Expected {1.000, 0.949}. 
PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and type GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
Read back {0.267, 0.949}. Expected {1.000, 0.949}.
Comment 4 Ian Romanick 2011-04-05 19:10:20 UTC
(In reply to comment #3)
> glean's readPixSanity failure is not the bisect commit caused.
> 
> Following is oglc pxconv-read subtest error info:
> PixelConvertCompareRead ERROR: Test failed with format PixelConvertCompareRead
> ERROR: Test failed with format GL_LUMINANCE and type GL_HALF_FLOAT_ARB,
> generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {2.266}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255}. Expected {1.000}. 

For glReadPixels, the RGB-to-luminance conversion is (see page 225 of the OpenGL 2.1 spec):

    L=R+G+B

If the source color is (1.000, 0.341, 0.925, 0.949), that's (0x7f, 0x2b, 0x75, 0x78) as signed bytes.  Without clamping, 0x7f + 0x2b + 0x75 = 0x11f.  Masking that to a signed byte gives 0x1f, and converting that to float gives 0.244.

It looks like the post-conversion clamp isn't happening.

> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {-1.000}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE and type
> GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267}. Expected {1.000}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_HALF_FLOAT_ARB, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {2.266, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255, 0.945}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_UNSIGNED_BYTE, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.255, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_SHORT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {-1.000, 0.949}. Expected {1.000, 0.949}. 
> PixelConvertCompareRead ERROR: Test failed with format GL_LUMINANCE_ALPHA and
> type GL_UNSIGNED_INT, generated color (1.000, 0.341, 0.925, 0.949). 
> Read back {0.267, 0.949}. Expected {1.000, 0.949}.
Comment 5 Ian Romanick 2011-04-05 19:11:35 UTC
I'm changing the component to "Mesa core" because the conversions happen there.  I suspect this same bug occurs with swrast.
Comment 6 Gordon Jin 2011-04-17 22:17:31 UTC
Ian, can you follow up with this bug?
Comment 7 Marek Olšák 2011-04-18 13:56:42 UTC
Created attachment 45783 [details] [review]
workaround

Does the attached patch fix the bug?
Comment 8 Ian Romanick 2011-04-19 18:28:04 UTC
(In reply to comment #7)
> Created an attachment (id=45783) [details]
> workaround
> 
> Does the attached patch fix the bug?

That fixes the test case here.

It seems like the tests that cause IMAGE_CLAMP_BIT to be set for the read case should be refactored to a utility function that, as your patch suggests, can be called from the callers of _mesa_pack_rgba_span_float.
Comment 9 fangxun 2011-04-20 00:22:51 UTC
Confirm the patch fix the bug.
Comment 10 Marek Olšák 2011-04-21 01:21:34 UTC
Please try this patch which should be the final fix:

http://lists.freedesktop.org/archives/mesa-dev/2011-April/006910.html
Comment 11 fangxun 2011-04-21 02:27:53 UTC
The final patch fixes the bug on pineview. 
Tested by: Fang Xun <xunx.fang@intel.com>
Comment 12 Marek Olšák 2011-04-21 21:42:55 UTC
Fixed with 1faf079a692bbf4b24c8e83fa2b331c1e3b58e13. Closing.
Comment 13 fangxun 2011-05-05 03:37:26 UTC
Verified with mesa master fc30910c65e7ab078b900c29d2066e45d3edd8c2.