Handle uniform precision mismatches #2984
Conversation
|
Thanks @lilleyse! Although this doesn't change the Cesium API, this is still a public-facing change that we want to include in the release notes, CHANGES.md (for more info on when to update CHANGES.md, see CONTRIBUTING.md). Add something like the following to the 1.13 section in CHANGES.md:
If we're able to test on real hardware today, I expect we'll merge this in time for the 1.13 release tomorrow; otherwise, we'll wait for 1.14, and update CHANGES.md accordingly. |
| @@ -263,6 +263,9 @@ define([ | |||
| ContextLimits._maximumViewportWidth = maximumViewportDimensions[0]; | |||
| ContextLimits._maximumViewportHeight = maximumViewportDimensions[1]; | |||
|
|
|||
| var highp = gl.getShaderPrecisionFormat(gl.FRAGMENT_SHADER, gl.HIGH_FLOAT); | |||
There was a problem hiding this comment.
Take a closer look at this code example. We need to account for HIGH_INT, not just HIGH_FLOAT, even though it isn't a problem yet. Let's add explicit getters for highp float and int, and then just check to see if either isn't support in ShaderProgram.
| }; | ||
| expect(sp.allUniforms.u_value).toBeDefined(); | ||
| expect(sp.allUniforms.u_value_f).toBeDefined(); | ||
| expect(renderFragment(context, sp, uniformMap)).toEqual([204, 204, 204, 204]); |
There was a problem hiding this comment.
This is most likely a fragile test, where it will pass on some systems, but not others. Can we rework the shader to be more coarse-grained, e.g., black is failing, non-black is passing.
| var uniformName = uniform; | ||
| // if it's a duplicate uniform, use its original name so it is updated correctly | ||
| var duplicateUniform = shader._duplicateUniformNames[uniformName]; | ||
| if (duplicateUniform) { |
|
This looks good. That may seem like a lot of comments, but they are all minor. |
|
Updated to account for the comments here. |
|
Looks good, thanks @lilleyse. This is all good with me. We'll merge once we can test on actual hardware. Have you tried asking on the forum post? |
|
The Mali-400 I tried reported it was mediump/lowp and not highp/mediump, and Cesium loaded and ran (poorly and with artifacts) but there were no shader compile failures. This was both with master and this branch. We should probably ask people on the forum to test it out and have them use webgl report to confirm precision. I've posted a temporary copy of the built Cesium Viewer at http://cesiumjs.org/mediump/index.html. You can also use this link to load it with some data http://cesiumjs.org/mediump/index.html?source=/Cesium/Apps/SampleData/simple.czml to verify primitives work as well. |
|
This was verified on the actual hardware. |
Handle uniform precision mismatches
For #817