Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2548733002:
Set correct internalformat info for TexStorageEXT for WebGL1 context (Closed)
DescriptionSet correct internalformat info for TexStorageEXT for WebGL1 context
WebGL1 context must behave like ES2 context, which crrev.com/2458103002 defines.
BUG=535198, 628064
CQ_INCLUDE_TRYBOTS=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/2548733002
Cr-Commit-Position: refs/heads/master@{#452718}
Committed: https://chromium.googlesource.com/chromium/src/+/b1424a997d92b500ccd797d4ee690246e40182b4
Patch Set 1 #Patch Set 2 : add unittests #
Total comments: 2
Patch Set 3 : fix nits #
Messages
Total messages: 26 (15 generated)
Description was changed from ========== Set correct internalformat info for TexStorageEXT for WebGL1 context WebGL1 context must behave like ES2 context, which crrev.com/2458103002 defines. BUG=535198, 628064 ========== to ========== Set correct internalformat info for TexStorageEXT for WebGL1 context WebGL1 context must behave like ES2 context, which crrev.com/2458103002 defines. BUG=535198, 628064 CQ_INCLUDE_TRYBOTS=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 ==========
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dongseong.hwang@intel.com changed reviewers: + piman@chromium.org, qiankun.miao@intel.com, zmo@chromium.org
piman@, could you review? https://codereview.chromium.org/2458103002 decides to keep unsized format for immutable texture in ES2 context. It should be same in WebGL1 context also.
Per WebGL 1.0 spec, there is no TexStorage* API in WebGL 1.0. We don't need to do this for WebGL1.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
On 2016/12/02 03:14:25, qiankun wrote: > Per WebGL 1.0 spec, there is no TexStorage* API in WebGL 1.0. We don't need to > do this for WebGL1. However, WebGL1 implementation uses TexStorage* in the WebGL1 context. e.g. DrawingBuffer
On 2016/12/02 05:38:17, dshwang wrote: > On 2016/12/02 03:14:25, qiankun wrote: > > Per WebGL 1.0 spec, there is no TexStorage* API in WebGL 1.0. We don't need to > > do this for WebGL1. > > However, WebGL1 implementation uses TexStorage* in the WebGL1 context. e.g. > DrawingBuffer Thanks for reminding. I didn't note this before.
Looks good to me with one small comment. https://codereview.chromium.org/2548733002/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2548733002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:16752: feature_info_->context_type() == CONTEXT_TYPE_WEBGL1; You can use feature_info_->IsWebGL1OrES2Context() here.
The CQ bit was checked by dongseong.hwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
piman@, could you review? https://codereview.chromium.org/2548733002/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_decoder.cc (right): https://codereview.chromium.org/2548733002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_decoder.cc:16752: feature_info_->context_type() == CONTEXT_TYPE_WEBGL1; On 2016/12/02 10:10:38, qiankun wrote: > You can use feature_info_->IsWebGL1OrES2Context() here. Good point! Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
LGTM, thanks!
The CQ bit was checked by dongseong.hwang@intel.com
The CQ bit was unchecked by dongseong.hwang@intel.com
On 2016/12/02 20:38:14, piman wrote: > LGTM, thanks! Thank you for review! This CL needs https://codereview.chromium.org/2543123003/ landed to avoid regression. I'll land it after landing the another CL.
On 2016/12/02 23:41:14, dshwang wrote: > On 2016/12/02 20:38:14, piman wrote: > > LGTM, thanks! > > Thank you for review! This CL needs https://codereview.chromium.org/2543123003/ > landed to avoid regression. I'll land it after landing the another CL. I forgot this CL. As the CL was landed and nothing was changed, let me land it.
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...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487897661916270,
"parent_rev": "fef42f9a072d257f7f4ef0300ea172f45754167a", "commit_rev":
"b1424a997d92b500ccd797d4ee690246e40182b4"}
Message was sent while issue was closed.
Description was changed from ========== Set correct internalformat info for TexStorageEXT for WebGL1 context WebGL1 context must behave like ES2 context, which crrev.com/2458103002 defines. BUG=535198, 628064 CQ_INCLUDE_TRYBOTS=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 ========== to ========== Set correct internalformat info for TexStorageEXT for WebGL1 context WebGL1 context must behave like ES2 context, which crrev.com/2458103002 defines. BUG=535198, 628064 CQ_INCLUDE_TRYBOTS=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/2548733002 Cr-Commit-Position: refs/heads/master@{#452718} Committed: https://chromium.googlesource.com/chromium/src/+/b1424a997d92b500ccd797d4ee69... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/b1424a997d92b500ccd797d4ee69... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
