Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1831833006:
Turn on seamless cubemap by default on Desktop GL for ES3. (Closed)
|
Created:
4 years, 9 months ago by Zhenyao Mo Modified:
4 years, 8 months ago CC:
chromium-reviews, piman+watch_chromium.org, Ken Russell (switch to Gerrit), bajones Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionTurn on seamless cubemap by default on Desktop GL for ES3.
BUG=597794
TEST=deqp/texturefiltering_cube_*
R=piman@chromium.org
CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel
Committed: https://crrev.com/39a22dc8028987efa3ca24135922841d2d2d387c
Cr-Commit-Position: refs/heads/master@{#383362}
Patch Set 1 #
Total comments: 1
Patch Set 2 : #
Total comments: 2
Patch Set 3 : #
Messages
Total messages: 30 (11 generated)
Description was changed from ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org ========== to ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ==========
The CQ bit was checked by zmo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1831833006/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1831833006/1
Description was changed from ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ==========
piman: PTAL
https://codereview.chromium.org/1831833006/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/1831833006/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/gles2_cmd_decoder.cc:2751: } Let's just turn it on always, even for es2 contexts, it's more consistent. As is, if we use virtual contexts, the state will leak to es2 contexts if you ever create an es3 one. For repeatability, let's just be consistent. The alternative is to turn it on or off on a context switch, but that sounds like more work.
On 2016/03/24 22:24:06, piman wrote: > https://codereview.chromium.org/1831833006/diff/1/gpu/command_buffer/service/... > File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): > > https://codereview.chromium.org/1831833006/diff/1/gpu/command_buffer/service/... > gpu/command_buffer/service/gles2_cmd_decoder.cc:2751: } > Let's just turn it on always, even for es2 contexts, it's more consistent. > As is, if we use virtual contexts, the state will leak to es2 contexts if you > ever create an es3 one. For repeatability, let's just be consistent. The > alternative is to turn it on or off on a context switch, but that sounds like > more work. We haven't reached a consensus on WebGL 1 situation yet because it's impossible to support it on some platforms and we don't want developers to rely on this. I'll implement the on/off thing for now.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
I also discussed this with kbr. Since we already turned seamless cubemap in ANGLE in WebGL 1 (we don't have a choice there), so it seems OK to also enable it on Desktop GL 3.2 or above. This will make this CL much simpler than the alternative, as piman pointed out. I'll get an updated CL up soon.
The CQ bit was checked by zmo@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1831833006/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1831833006/20001
Updated. Please take another look.
https://codereview.chromium.org/1831833006/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/1831833006/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:2751: } Can you remove this? The code below will take care of it, right?
The CQ bit was checked by zmo@chromium.org to run a CQ dry run
https://codereview.chromium.org/1831833006/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/1831833006/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:2751: } On 2016/03/25 17:01:04, piman wrote: > Can you remove this? The code below will take care of it, right? Ah, nasty. I thought I deleted already.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1831833006/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1831833006/40001
piman@google.com changed reviewers: + piman@google.com
lgtm
On 2016/03/25 17:46:12, Antoine Labour wrote: > lgtm Ah, will need a review from your chromium account.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
lgtm
The CQ bit was checked by zmo@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1831833006/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1831833006/40001
Message was sent while issue was closed.
Description was changed from ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== Turn on seamless cubemap by default on Desktop GL for ES3. BUG=597794 TEST=deqp/texturefiltering_cube_* R=piman@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel Committed: https://crrev.com/39a22dc8028987efa3ca24135922841d2d2d387c Cr-Commit-Position: refs/heads/master@{#383362} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/39a22dc8028987efa3ca24135922841d2d2d387c Cr-Commit-Position: refs/heads/master@{#383362}
Message was sent while issue was closed.
A revert of this CL (patchset #3 id:40001) has been created in https://codereview.chromium.org/1840553002/ by zmo@chromium.org. The reason for reverting is: This doesn't work on Mac Intel (most likely a driver bug). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
