Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(496)

Issue 2110543002: gpu, cmaa: don't blend the rightmost and topmost edges. (Closed)

Created:
4 years, 5 months ago by dshwang
Modified:
4 years, 5 months ago
CC:
chromium-reviews, piman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

gpu, cmaa: don't blend the rightmost and topmost edges. texelFetch() on out of range (i.e. outside of x = [0, width) or y = [0, height)) gives an undefined value. DETECT_EDGES2 pass fetches (width, _) or (_, height) texel, which is an undefiend value. So DETECT_EDGES2 can mislead the right most edge is dominant edge after comparing the undefined value and the right most pixel. This CL fixes parts of 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;tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/6da3ecce0e3397264b9412efadef70ede93e8ba8 Cr-Commit-Position: refs/heads/master@{#402890}

Patch Set 1 #

Total comments: 3

Patch Set 2 : use notEqual for component wise comparison #

Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -6 lines) Patch
M gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc View 1 7 chunks +13 lines, -6 lines 0 comments Download

Messages

Total messages: 21 (8 generated)
dshwang
piman@ could you review? It's the 2nd CL of https://bugs.chromium.org/p/chromium/issues/detail?id=535198#c16
4 years, 5 months ago (2016-06-28 18:16:05 UTC) #4
piman
lgtm
4 years, 5 months ago (2016-06-28 20:58:05 UTC) #5
piman
https://codereview.chromium.org/2110543002/diff/1/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2110543002/diff/1/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc#newcode1259 gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:1259: ivec2((screenPosI + 1) != textureSize(g_src0Texture4Uint, 0)); Actually, going back ...
4 years, 5 months ago (2016-06-28 21:09:38 UTC) #6
dshwang
Thank you for reviewing. I explained the intention below. https://codereview.chromium.org/2110543002/diff/1/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2110543002/diff/1/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc#newcode1259 gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:1259: ...
4 years, 5 months ago (2016-06-29 10:41:55 UTC) #7
dshwang
https://codereview.chromium.org/2110543002/diff/1/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2110543002/diff/1/gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc#newcode1259 gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:1259: ivec2((screenPosI + 1) != textureSize(g_src0Texture4Uint, 0)); On 2016/06/28 21:09:38, ...
4 years, 5 months ago (2016-06-29 13:53:01 UTC) #8
adrian.belgun
lgtm
4 years, 5 months ago (2016-06-29 14:59:08 UTC) #9
jon.kennedy
lgtm
4 years, 5 months ago (2016-06-29 15:53:47 UTC) #10
piman
lgtm
4 years, 5 months ago (2016-06-29 17:19:10 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2110543002/20001
4 years, 5 months ago (2016-06-29 17:41:36 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2110543002/20001
4 years, 5 months ago (2016-06-29 18:25:19 UTC) #16
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 5 months ago (2016-06-29 19:13:10 UTC) #18
commit-bot: I haz the power
CQ bit was unchecked.
4 years, 5 months ago (2016-06-29 19:13:29 UTC) #19
commit-bot: I haz the power
4 years, 5 months ago (2016-06-29 19:15:21 UTC) #21
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/6da3ecce0e3397264b9412efadef70ede93e8ba8
Cr-Commit-Position: refs/heads/master@{#402890}

Powered by Google App Engine
This is Rietveld 408576698