Bug 87407 - dFdx/dFdy when having a parameter modified with abs() is giving odd results (0.0) in the limit values (first column in x and first row in y)
Summary: dFdx/dFdy when having a parameter modified with abs() is giving odd results (...
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Mesa core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Andrés Gómez García
QA Contact:
URL:
Whiteboard:
Keywords:
: 89329 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-12-17 11:22 UTC by Andrés Gómez García
Modified: 2017-09-18 13:48 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:

Attachments
Patch to correct the GCC warnings and introduce the 2 latest use cases (1.33 KB, text/plain)
2014-12-17 11:22 UTC, Andrés Gómez García 		Details
Image showing some different pixels (2.42 KB, image/png)
2014-12-17 18:58 UTC, Ian Romanick 		Details
View All

Description Andrés Gómez García 2014-12-17 11:22:32 UTC 
Created attachment 110945 [details]
Patch to correct the GCC warnings and introduce the 2 latest use cases

At bug 82991 we extended the piglit test glsl-deriv-varyings with 3 new cases:
http://lists.freedesktop.org/archives/piglit/2014-December/013648.html

However, a couple of GCC warnings were introduced in piglit and the last 2 extended cases using dFdx(abs()) and dFdy(abs()) were not being tested.

After fixing the warnings (see attached patch) and getting the 2 last cases to work, a new bug has been identified in mesa.

For example, in this piglit test, using the following fragment shader:

uniform sampler2D tex2d;
varying vec2 texCoords;
void main()
{
    gl_FragColor = vec4(dFdx(abs(texCoords.x)) * 40.0,
                        dFdy(abs(texCoords.y)) * 40.0,
                        0.0, 1.0);
}

The piglit result is:
Probe color at
  Expected: 0.800000 0.400000 0.000000
  Observed: 0.800000 0.000000 0.000000
PIGLIT: {"result": "fail" }

However, running manually the test with:

$ PIGLIT_SOURCE_DIR="<path_to>/piglit.git" PIGLIT_PLATFORM="mixed_glx_egl" <path_to>/piglit.git/bin/glsl-deriv-varyings

We can see that the pixels painted in the display are, actually, the expected ones.

Hence, the problem happens when checking the colors of the pixels through glReadPixels.

I've also checked that the problem is reproducible, at least, with swrast, i965 (gen6) and nouveau.

While I'm investigating this problem, I've sent a patch proposal to remove these test cases and fix also the introduced warnings at:
http://lists.freedesktop.org/archives/piglit/2014-December/013764.html
Comment 1 Ian Romanick 2014-12-17 18:58:35 UTC 
Created attachment 110961 [details]
Image showing some different pixels

Looking at that test... I'm surprised Matt gave a Reviewed-by on your original patch. :) This is a really old test, and, IMO, each of the subtests should be converted to individual shader_runner tests.

Also... none of the added shaders need 'uniform sampler2D tex2d;'  That should be removed.

I'm not convinced that the results are correct.  Look at the attached image.  The bottom right of the 3rd box and the bottom left of the 4th box are... different.  Is that intended?
Comment 2 Andrés Gómez García 2014-12-17 22:43:54 UTC 
(In reply to Ian Romanick from comment #1)
> Created attachment 110961 [details]
> Image showing some different pixels
> 
> Looking at that test... I'm surprised Matt gave a Reviewed-by on your
> original patch. :) This is a really old test, and, IMO, each of the subtests
> should be converted to individual shader_runner tests.
> 
> Also... none of the added shaders need 'uniform sampler2D tex2d;'  That
> should be removed.

I realized about that but I didn't want to modify the original test that already had the "uniform sample2D tex2d;" coming from the vertex shader used by every fragment shader.

Basically, I was just following the example of the existent test.

In any case, I can work to convert every subtest to an individual one. What do you think?
 
> I'm not convinced that the results are correct.  Look at the attached image.
> The bottom right of the 3rd box and the bottom left of the 4th box are...
> different.  Is that intended?

Yes, I realized about that today while testing and, not, it is not intended.

Hence, the description of this bug is not correct. The problem is not glReadPixels but, actually, the results when using dFdx(abs()) or dFdy(abs())

Another problem I've observed is that the bottom in the 3rd and 4th box change when resizing the window.
Comment 3 Andrés Gómez García 2014-12-17 22:57:37 UTC 
(In reply to Andrés Gómez García from comment #2)
> > Also... none of the added shaders need 'uniform sampler2D tex2d;'  That
> > should be removed.
> 
> I realized about that but I didn't want to modify the original test that
> already had the "uniform sample2D tex2d;" coming from the vertex shader used
> by every fragment shader.

Oooops!!!

Forget about this. Yes, I should have removed it completely :/
Comment 4 Andrés Gómez García 2014-12-17 23:23:10 UTC 
(In reply to Andrés Gómez García from comment #2)
> Another problem I've observed is that the bottom in the 3rd and 4th box
> change when resizing the window.

The bottom pixels line in the 3rd and 4th box is read when the window size is even and not when it is odd. You can see this just by changing the window size from 300 to 301.

However, piglit_probe_pixel_rgb is saying that the bottom line in the 3rd and 4th boxes are still red.

After some checking, I'm seeing that this depends on the piglit run -fbo option. With -fbo, the window size doesn't matter but, without -fbo, it does.
Comment 5 Andrés Gómez García 2015-02-23 14:01:23 UTC 
Checked with swrast, nouveau, i965 and the NVIDIA proprietary driver and this test is failing consistently in all of them.

I'm wondering whether my expectations are actually wrong.
Comment 6 Ian Romanick 2015-02-23 18:21:59 UTC 
(In reply to Andrés Gómez García from comment #5)
> Checked with swrast, nouveau, i965 and the NVIDIA proprietary driver and
> this test is failing consistently in all of them.
> 
> I'm wondering whether my expectations are actually wrong.

I think I know what the problem is.

The lower-left fragment gets (0,0).  In order to get the correct derivatives for the dFdx(texCoords.x) case, the helper fragments outside the polygon get negative values.  dFdx(abs(texCoords.x)) around that fragment will be zero.

Biasing the texture coordinates passed from the vertex shader by some small amount should fix the problem.  The box is 50 pixels wide by 100 pixels tall.  Biasing by (1/50, 1/100) should be sufficient.  That means you'll probably want a different vertex shader for the abs() cases.
Comment 7 Ian Romanick 2015-02-26 02:11:00 UTC 
*** Bug 89329 has been marked as a duplicate of this bug. ***
Comment 8 Andrés Gómez García 2015-04-28 14:31:07 UTC 
Proposed piglit patch at:
http://lists.freedesktop.org/archives/piglit/2015-April/015879.html
Comment 9 Andrés Gómez García 2015-05-15 10:51:20 UTC 
This has been resolved, so closing.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.