CopyTexImage2D artifacts on >=1024 pixel size due to 10-bit "mediump" TexCoordPrecision |
|||||
Issue descriptionChrome Version: src@6280ec5b6e3b4c307926eb1bf0167acc2a0d4a74 OS: (Android) What steps will reproduce the problem? (1) use Pixel XL with Daydream View (2) go to https://webvr.info/samples/03-vr-presentation.html (3) press "Enter VR" or, more fundamentally: (1) use CopyTexImage2D on a texture >= 1024 pixels on a GLES device What is the expected result? Artifact-free rendering What happens instead? Artifacts, straight lines look jagged in a repeating pattern. Looks like alternating blocks of pixels are shifted from their true position. See screenshot, especially the edges of the black FPS rectangle, the green line in it, and the "WebVR" text on the cube just above it. Looks mostly fine in the left eye, and bad in the right eye. Please use labels and text to provide additional information. What appears to be going on is that it's copying the texture with "mediump" selected for TexCoordPrecision. However, on this device a "mediump" only has 10 bits of mantissa precision, so it can not address all pixels in a texture once a dimension is >= 1024 pixels. For graphics-related bugs, please copy/paste the contents of the about:gpu page at the end of this report. # type format: range precision GL_VERTEX_SHADER GL_LOW_FLOAT: 127 23 GL_VERTEX_SHADER GL_MEDIUM_FLOAT: 127 23 GL_VERTEX_SHADER GL_HIGH_FLOAT: 127 23 GL_FRAGMENT_SHADER GL_LOW_FLOAT: 15 10 GL_FRAGMENT_SHADER GL_MEDIUM_FLOAT: 15 10 GL_FRAGMENT_SHADER GL_HIGH_FLOAT: 127 23 https://cs.chromium.org/chromium/src/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc?type=cs&q=texture2d+file:gpu&l=284 const char* kShaderPrecisionPreamble = "#ifdef GL_ES\n" "precision mediump float;\n" "#define TexCoordPrecision mediump\n" "#else\n" "#define TexCoordPrecision\n" "#endif\n"; To verify, I manually changed both "mediump" to "highp" in this source snippet, and this fixed the rendering artifacts. Is there supposed to be a restriction on texture size? If not, it would need to use highp for texture coordinates if a dimension is >= 2^(precision of mediump), assuming the device supports highp.
,
Mar 9 2017
Here's an alternate version of the preamble I've used now for testing, this also fixes the issue for me:
const char* kShaderPrecisionPreamble =
"#ifdef GL_ES\n"
"precision mediump float;\n"
"#ifdef GL_FRAGMENT_PRECISION_HIGH\n"
"#define TexCoordPrecision highp\n"
"#else\n"
"#define TexCoordPrecision mediump\n"
"#endif\n"
"#else\n"
"#define TexCoordPrecision\n"
"#endif\n";
What's the performance cost of switching the shader precision? There's only a handful of vertices in this case, and on my device it internally seems to interpolate and do texture sampling at a higher precision since I don't see low-res duplicated pixels, just areas where they get shifted from their intended position.
If this is a concern, it should be possible to compile two variants of the shader program on devices where the workaround is needed. Something like this?
One-time setup:
glGetShaderPrecisionFormat(GL_FRAGMENT_SHADER, GL_MEDIUM_FLOAT, &_, &medium_precision);
glGetShaderPrecisionFormat(GL_FRAGMENT_SHADER, GL_HIGH_FLOAT, &_, &high_precision);
# Support at least 4096 pixel textures?
kRequiredPrecisionBits = 12
info->highp_size_threshold_pixels = MAX_INT
if (medium_precision <= kRequiredPrecisionBits && high_precision >= kRequiredPrecisionBits) {
# medium precision doesn't have enough bits, but high precision does.
# high_precision will be 0 if the device doesn't support it.
...
source_highp = GetVertexShaderSource(..., use_highp)
...
info->program_highp = ...
info->highp_size_threshold_pixels = 1 << medium_precision;
Copy:
if (width >= info->highp_size_threshold_pixels || height >= info->highp_size_threshold_pixels)
glUseProgram(info->program_highp_
else
glUseProgram(info->program)
That's of course incomplete, but seems doable if a bit ugly.
What do you think? If I understand it right, this doesn't seem like a VR specific issue, though it's very noticeable in that scenario since you're literally looking at the screen with a magnifying glass right in front of your eye.
,
Mar 10 2017
Proposed patch (using the simple method) uploaded as https://codereview.chromium.org/2744793002 for reference.
,
Mar 10 2017
,
Mar 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/72efd11d727f12bc45e74a1e6f7e379b3d6d5e2d commit 72efd11d727f12bc45e74a1e6f7e379b3d6d5e2d Author: klausw <klausw@chromium.org> Date: Fri Mar 10 07:14:27 2017 Use highp for glCopyTexture2D where available This fixes artifacts in copied textures caused by insufficient precision in "mediump" floats when used for sampling textures. On a Pixel XL, a mediump only has 10 bits precision in fragment shaders, that apparently only works fully for texture dimensions < 1024 pixels. Use "highp" if both GL_ES and GL_FRAGMENT_PRECISION_HIGH are set for the shader. (See the bug for more discussion and a proposed alternative patch.) BUG= 700031 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2744793002 Cr-Commit-Position: refs/heads/master@{#456012} [modify] https://crrev.com/72efd11d727f12bc45e74a1e6f7e379b3d6d5e2d/gpu/command_buffer/service/gles2_cmd_copy_texture_chromium.cc
,
Mar 10 2017
,
Mar 10 2017
,
Mar 20 2017
Closing as fixed since the patch from comment 5 addresses it. jbauman@, I assume your LGTM on that patch implies you're ok with that, please reopen if that's not the case. Also bumping to M59 since our code which ran into this issue won't be in M58. Let me know if you think this should be merged earlier. It sounds as if this could affect other WebGL applications or other texture copies also, but I didn't find any pre-existing bug reports. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by klausw@chromium.org
, Mar 9 2017Labels: Proj-VR M-58