search

gxquickly / angleproject

Automatically exported from code.google.com/p/angleproject
Other
0 stars 0 forks source link

Replacing references to constant variables with the raw constant value disregards precision qualifiers of constant variables #817

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1. Translate the following fragment shader to OpenGL ES shading language using 
ANGLE:

uniform mediump float u;
void main() {
   const highp float c = 2048.5;
   mediump float a = fract(c + u);
   gl_FragColor = vec4(a);
}

What is the expected output? What do you see instead?
Expected: fract(c + u); is going to be evaluated at highp in the translated 
shader.
Actual: all references to highp are lost when the expression is converted to 
fract(2048.5 + u); and it will be evaluated at mediump. Raw constant values are 
themselves evaluated at highest available precision, but they don't have 
precision qualifiers, so they don't affect the precision of the consuming 
operation.

What version of the product are you using? On what operating system?
TOT version of ANGLE on Android L preview.

This bug can manifest in more subtle forms as well, particularly when default 
precision qualifiers are used.

Original issue reported on code.google.com by oetu...@nvidia.com on 30 Oct 2014 at 12:42

GoogleCodeExporter commented 9 years ago 
Conformance test at: 
https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/constant-
precision-qualifier.html

The test fails on Nexus 5, for instance.

Original comment by oetu...@nvidia.com on 31 Oct 2014 at 8:49

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/b07aba0798d3bba3118dac78933e73b3f08a601b

commit b07aba0798d3bba3118dac78933e73b3f08a601b
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Fri May 29 09:19:19 2015

Make sure type gets set consistently in folded binary operations

Add a wrapper function that handles creating the folded node and setting the
right parameters on it, so that the folding function handles only
calculating the folded values.

This will fix the precision set to constant folded values in some cases.
Previously the precision was always set to be equal to one of the operands
to the binary operation, but now both operands are taken into account.

Folding binary operations is now in a separate function from folding unary
operations.

TEST=dEQP-GLES3.functional.shaders.constant_expressions.*
BUG=angleproject:817

Change-Id: Id97e765173c6110f49607e21c3fb90019b1ebac7
Reviewed-on: https://chromium-review.googlesource.com/274001
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>

[modify] 
http://crrev.com/b07aba0798d3bba3118dac78933e73b3f08a601b/src/compiler/translato
r/Intermediate.cpp
[modify] 
http://crrev.com/b07aba0798d3bba3118dac78933e73b3f08a601b/src/compiler/translato
r/IntermNode.cpp
[modify] 
http://crrev.com/b07aba0798d3bba3118dac78933e73b3f08a601b/src/compiler/translato
r/IntermNode.h

Original comment by bugdroid1@chromium.org on 4 Jun 2015 at 10:17

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/aebd002d00d39858819c58bad1970df121b78e1b

commit aebd002d00d39858819c58bad1970df121b78e1b
Author: Jamie Madill <jmadill@chromium.org>
Date: Thu Jun 04 19:43:44 2015

Revert "Make sure type gets set consistently in folded binary operations"

This is blocking the revert of the geometric constant folding 
patch, which is breaking gpu_unittests and blocking the roll.

BUG=angleproject:817

This reverts commit b07aba0798d3bba3118dac78933e73b3f08a601b.

Change-Id: Ia00fc45b1ddd9d3c079742dea0627aa12304f93b
Reviewed-on: https://chromium-review.googlesource.com/275321
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tested-by: Jamie Madill <jmadill@chromium.org>

[modify] 
http://crrev.com/aebd002d00d39858819c58bad1970df121b78e1b/src/compiler/translato
r/Intermediate.cpp
[modify] 
http://crrev.com/aebd002d00d39858819c58bad1970df121b78e1b/src/compiler/translato
r/IntermNode.cpp
[modify] 
http://crrev.com/aebd002d00d39858819c58bad1970df121b78e1b/src/compiler/translato
r/IntermNode.h

Original comment by bugdroid1@chromium.org on 4 Jun 2015 at 7:44

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/2c4b746cbd6a02502a953517e09e312969aed73b

commit 2c4b746cbd6a02502a953517e09e312969aed73b
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Mon Jun 08 08:30:31 2015

Revert "Revert "Make sure type gets set consistently in folded binary 
operations""

This patch was originally reverted only because a dependency patch failed a
buggy Chromium test.

This reverts commit aebd002d00d39858819c58bad1970df121b78e1b.

TEST=dEQP-GLES3.functional.shaders.constant_expressions.*
BUG=angleproject:817

Change-Id: Ia5acf15518ea89717c0cfe1398cb18ea27be5b19
Reviewed-on: https://chromium-review.googlesource.com/275811
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[modify] 
http://crrev.com/2c4b746cbd6a02502a953517e09e312969aed73b/src/compiler/translato
r/Intermediate.cpp
[modify] 
http://crrev.com/2c4b746cbd6a02502a953517e09e312969aed73b/src/compiler/translato
r/IntermNode.cpp
[modify] 
http://crrev.com/2c4b746cbd6a02502a953517e09e312969aed73b/src/compiler/translato
r/IntermNode.h

Original comment by bugdroid1@chromium.org on 8 Jun 2015 at 2:49

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/95310b000b8e684fa8a85e6cec91bb568a7cfe02

commit 95310b000b8e684fa8a85e6cec91bb568a7cfe02
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Tue Jun 02 14:43:38 2015

Unify unary operator folding with binary operator folding

Implement unary operator folding in a similar way to binary operator
folding, so that the code is easier to understand.

TEST=dEQP-GLES3.functional.shaders.constant_expressions.*
BUG=angleproject:817

Change-Id: I069bdb38f965e1badb3e8f3f954386b205b7bb00
Reviewed-on: https://chromium-review.googlesource.com/275185
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>

[modify] 
http://crrev.com/95310b000b8e684fa8a85e6cec91bb568a7cfe02/src/compiler/translato
r/Intermediate.cpp
[modify] 
http://crrev.com/95310b000b8e684fa8a85e6cec91bb568a7cfe02/src/compiler/translato
r/IntermNode.cpp
[modify] 
http://crrev.com/95310b000b8e684fa8a85e6cec91bb568a7cfe02/src/compiler/translato
r/IntermNode.h

Original comment by bugdroid1@chromium.org on 8 Jun 2015 at 2:58

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/b43846eef1db313414bc49d8a2f5754d72ad8b0b

commit b43846eef1db313414bc49d8a2f5754d72ad8b0b
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Tue Jun 02 15:18:57 2015

Unify aggregate operator folding with other constant folding

Setting the type for folded aggregate nodes should work in a similar way
as other constant folding. Common functionality between the different
folding functions is refactored into a single function.

TEST=dEQP-GLES3.functional.shaders.constant_expressions.*
BUG=angleproject:817

Change-Id: Ie0be561f4a30e52e52d570ff0b2bdb426f6e4f7a
Reviewed-on: https://chromium-review.googlesource.com/275186
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[modify] 
http://crrev.com/b43846eef1db313414bc49d8a2f5754d72ad8b0b/src/compiler/translato
r/Intermediate.cpp
[modify] 
http://crrev.com/b43846eef1db313414bc49d8a2f5754d72ad8b0b/src/compiler/translato
r/ParseContext.cpp
[modify] 
http://crrev.com/b43846eef1db313414bc49d8a2f5754d72ad8b0b/src/compiler/translato
r/Intermediate.h
[modify] 
http://crrev.com/b43846eef1db313414bc49d8a2f5754d72ad8b0b/src/compiler/translato
r/IntermNode.cpp
[modify] 
http://crrev.com/b43846eef1db313414bc49d8a2f5754d72ad8b0b/src/compiler/translato
r/IntermNode.h

Original comment by bugdroid1@chromium.org on 9 Jun 2015 at 7:29

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/7adfb18475dd591497da115e0cf84604edcb4ec2

commit 7adfb18475dd591497da115e0cf84604edcb4ec2
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Tue Jun 09 12:49:41 2015

Refactor common compiler test functionality into helper functions

Refactor translating a ESSL shader string into a target language so that
compiler initialization and cleanup code can be reused between test classes.

BUG=angleproject:817
TEST=angle_unittests

Change-Id: Idb229dceb9e17b13ed6ad2a68ab55ed5c968780e
Reviewed-on: https://chromium-review.googlesource.com/275814
Tested-by: Olli Etuaho <oetuaho@nvidia.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>

[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/compiler_tes
ts/PruneUnusedFunctions_test.cpp
[add] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/test_utils/c
ompiler_test.cpp
[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/compiler_tes
ts/DebugShaderPrecision_test.cpp
[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/compiler_tes
ts/compiler_tests.gypi
[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/angle_unitte
sts.gypi
[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/compiler_tes
ts/UnrollFlatten_test.cpp
[add] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/test_utils/c
ompiler_test.h
[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/compiler_tes
ts/NV_draw_buffers_test.cpp
[modify] 
http://crrev.com/7adfb18475dd591497da115e0cf84604edcb4ec2/src/tests/compiler_tes
ts/BuiltInFunctionEmulator_test.cpp

Original comment by bugdroid1@chromium.org on 10 Jun 2015 at 1:36

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/e754fb8a6d96eaaf6ac31d5cedf74d74446fb609

commit e754fb8a6d96eaaf6ac31d5cedf74d74446fb609
Author: Geoff Lang <geofflang@chromium.org>
Date: Wed Jun 10 14:52:25 2015

Fix missing newline at the end of compiler_test.h

BUG=angleproject:817

Change-Id: Iaa971c71c0f5bcec46fffb5d2e70f108e6ca6b02
Reviewed-on: https://chromium-review.googlesource.com/276607
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Tested-by: Geoff Lang <geofflang@chromium.org>

[modify] 
http://crrev.com/e754fb8a6d96eaaf6ac31d5cedf74d74446fb609/src/tests/test_utils/c
ompiler_test.h

Original comment by bugdroid1@chromium.org on 10 Jun 2015 at 2:53

GoogleCodeExporter commented 9 years ago 
The following revision refers to this bug:
  https://chromium.googlesource.com/angle/angle/+/a4aa4e300b91bc499abd76414094fce471ac2a94

commit a4aa4e300b91bc499abd76414094fce471ac2a94
Author: Olli Etuaho <oetuaho@nvidia.com>
Date: Thu Jun 04 12:54:30 2015

Record precision of constant variables when needed

Add a traverser that checks precision qualifiers of folded constants and
hoists them to separate precision qualified variables if needed.

Fixes sdk/tests/conformance/glsl/bugs/constant-precision-qualifier.html

TEST=WebGL conformance tests, angle_unittests
BUG=angleproject:817

Change-Id: I1639595e0e49470736be93274f0af07ee732e1fe
Reviewed-on: https://chromium-review.googlesource.com/275095
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Tested-by: Olli Etuaho <oetuaho@nvidia.com>

[add] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/tests/compiler_tes
ts/RecordConstantPrecision_test.cpp
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler/translato
r/IntermNode.cpp
[add] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler/translato
r/RecordConstantPrecision.h
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler/translato
r/TranslatorESSL.cpp
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/tests/compiler_tes
ts/compiler_tests.gypi
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/tests/angle_unitte
sts.gypi
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler/translato
r/IntermNode.h
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler/translato
r/IntermTraverse.cpp
[add] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler/translato
r/RecordConstantPrecision.cpp
[modify] 
http://crrev.com/a4aa4e300b91bc499abd76414094fce471ac2a94/src/compiler.gypi

Original comment by bugdroid1@chromium.org on 10 Jun 2015 at 5:44

GoogleCodeExporter commented 9 years ago 
This is fixed in ANGLE now (though testing could be more comprehensive). Driver 
compilers still have some issues with assigning correct precision to operations 
in some instances.

Original comment by oetu...@nvidia.com on 10 Jun 2015 at 5:47