Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2103893003:
gpu, cmaa: don't blend the leftmost and bottom-most edges. (Closed)
Descriptiongpu, cmaa: don't blend the leftmost and bottom-most edges.
texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height))
gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel,
which is an undefiend value. So COMBINE_EDGES can mislead the left most
edge is dominant edge after comparing the undefined value and the left most
pixel.
This CL fixes 3 WebGL conformance tests
https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multisample-corruption.html
https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fragcoord-multisampling-bug.html
https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premultiplyalpha-test.html
BUG=535198
TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders"
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel
Committed: https://crrev.com/994b8ad38039a9d1c432ef081788a47be90354e7
Cr-Commit-Position: refs/heads/master@{#402877}
Patch Set 1 #
Total comments: 3
Patch Set 2 : use notEqual for component wise comparison #Messages
Total messages: 21 (9 generated)
Description was changed from ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. CMAA_COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So CMAA_COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" ========== to ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. CMAA_COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So CMAA_COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
dongseong.hwang@intel.com changed reviewers: + adrian.belgun@intel.com, piman@chromium.org
piman@ could you review? It's the 3rd CL of https://bugs.chromium.org/p/chromium/issues/detail?id=535198#c16
https://codereview.chromium.org/2103893003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2103893003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:1373: uvec2 notBottomLeft = uvec2(screenPosIBase.xy != ivec2(0, 0)); "screenPosIBase.xy != ivec2(0, 0)" is a boolean scalar. So notBottomLeft.x == notBottomLeft.y. What is the intention in setting this to a uvec2? Or did you mean to use equal(screenPosIBase.xy, ivec2(0, 0))?
Thank you for reviewing. I explained the intention below. https://codereview.chromium.org/2103893003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2103893003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:1373: uvec2 notBottomLeft = uvec2(screenPosIBase.xy != ivec2(0, 0)); On 2016/06/28 21:08:09, piman wrote: > "screenPosIBase.xy != ivec2(0, 0)" is a boolean scalar. So notBottomLeft.x == > notBottomLeft.y. What is the intention in setting this to a uvec2? Or did you > mean to use equal(screenPosIBase.xy, ivec2(0, 0))? sorry for obfuscated code, because I want to avoid to use "if statement", which is not good for shader. Let me explain what this code intends. Following two elements stores one pixel below the current pixel. packedEdgesArray[(1) * 3 + (0)] = sampA.w; packedEdgesArray[(2) * 3 + (0)] = sampA.z; so if current pixel is (_, 0), the shader must avoid storing the value. The intuitive code would be if (screenPosIBase.y == 0) { packedEdgesArray[(1) * 3 + (0)] = 0; packedEdgesArray[(2) * 3 + (0)] = 0; } else { packedEdgesArray[(1) * 3 + (0)] = sampA.w; packedEdgesArray[(2) * 3 + (0)] = sampA.z; } However, in general, multiplying is fater than "if statement" in shader. So I replace "if" to multiplying. If you want more intuitive code, let me use "if statement". WDYT?
https://codereview.chromium.org/2103893003/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2103893003/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:1373: uvec2 notBottomLeft = uvec2(screenPosIBase.xy != ivec2(0, 0)); On 2016/06/28 21:08:09, piman wrote: > "screenPosIBase.xy != ivec2(0, 0)" is a boolean scalar. So notBottomLeft.x == > notBottomLeft.y. What is the intention in setting this to a uvec2? Or did you > mean to use equal(screenPosIBase.xy, ivec2(0, 0))? I actually wanted component wise comparison; notEqual(screenPosIBase.xy, ivec2(0, 0)) I updated this CL to use notEqual for component wise comparison, instead of scalar boolean. Adrian, Thank you for advicing me how to fix it.
lgtm
lgtm
Description was changed from ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. CMAA_COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So CMAA_COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ==========
lgtm
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ==========
The CQ bit was unchecked by dongseong.hwang@intel.com
The CQ bit was checked by dongseong.hwang@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ========== to ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel ========== to ========== gpu, cmaa: don't blend the leftmost and bottom-most edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. COMBINE_EDGES pass fetches (-1 , _) or (_, -1) texel, which is an undefiend value. So COMBINE_EDGES can mislead the left most edge is dominant edge after comparing the undefined value and the left most pixel. This CL fixes 3 WebGL conformance tests https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/multis... https://www.khronos.org/registry/webgl/sdk/tests/conformance/glsl/bugs/gl-fra... https://www.khronos.org/registry/webgl/sdk/tests/conformance/context/premulti... BUG=535198 TEST=Run a WebGL app with Chrome started with "--enable-cmaa-shaders" CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel Committed: https://crrev.com/994b8ad38039a9d1c432ef081788a47be90354e7 Cr-Commit-Position: refs/heads/master@{#402877} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/994b8ad38039a9d1c432ef081788a47be90354e7 Cr-Commit-Position: refs/heads/master@{#402877} |
