Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2019293002:
Save interface block infomation to program cache. (Closed)
|
Created:
4 years, 6 months ago by xinghua.cao Modified:
4 years, 6 months ago Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionSave interface block infomation to program cache
If GPU supports program cache, and command buffer does not save interface
block information, it cannot get correct uniform information when update
uniforms.
BUG=615794
TestCase=deqp/functional/gles3/shaderstatequery.html
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/6b03758cf719953b94bcf9007bb3b7ad909e7c96
Cr-Commit-Position: refs/heads/master@{#397291}
Patch Set 1 #
Total comments: 5
Patch Set 2 : Address qiankun's comments #
Total comments: 4
Patch Set 3 : Address zhenyao's comments #
Messages
Total messages: 27 (15 generated)
Description was changed from ========== Save interface block infomation to program cache. Save InterfaceBlock info to cache BUG= ========== to ========== Save interface block infomation to program cache. Save InterfaceBlock info to cache BUG= 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 ==========
Description was changed from ========== Save interface block infomation to program cache. Save InterfaceBlock info to cache BUG= 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 ========== Save interface block infomation to program cache. Save InterfaceBlock info to cache BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. 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 ==========
xinghua.cao@intel.com changed reviewers: + qiankun.miao@intel.com, yunchao.he@intel.com
https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/disk_cache_proto.proto (right): https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/disk_cache_proto.proto:44: optional uint32 array_size = 4; variable 'layout' type is sh::BlockLayoutType(enum define in angle), I cannot conform that proto buffer could reference C++ type, so cast it to int32. It maybe an error. Please help to check it. https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/memory_program_cache.cc (right): https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/memory_program_cache.cc:180: interface_block.arraySize = proto.array_size(); cast int32 to sh::BlockLayoutType, may be an error. See another comment in disk_cache_proto.proto file.
I roughly reviewed this CL and L-G-T-M. Just a few comments. But I am not an expert on this. zmo and kbr may give more comments. https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/disk_cache_proto.proto (right): https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/disk_cache_proto.proto:44: optional uint32 array_size = 4; On 2016/05/30 09:55:46, xinghua.cao wrote: > variable 'layout' type is sh::BlockLayoutType(enum define in angle), I cannot > conform that proto buffer could reference C++ type, so cast it to int32. It > maybe an error. Please help to check it. I think you can define same enum in proto file. https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/memory_program_cache.cc (right): https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/memory_program_cache.cc:114: iter != shader->interface_block_map().end(); ++ iter) { one extra space between "++" and "iter".
xinghua.cao@intel.com changed reviewers: + kbr@chromium.org, zmo@chromium.org
zhenyao, please help to review it about uniforms, thank you. https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... File gpu/command_buffer/service/disk_cache_proto.proto (right): https://codereview.chromium.org/2019293002/diff/1/gpu/command_buffer/service/... gpu/command_buffer/service/disk_cache_proto.proto:44: optional uint32 array_size = 4; On 2016/05/30 12:42:11, qiankun wrote: > On 2016/05/30 09:55:46, xinghua.cao wrote: > > variable 'layout' type is sh::BlockLayoutType(enum define in angle), I cannot > > conform that proto buffer could reference C++ type, so cast it to int32. It > > maybe an error. Please help to check it. > > I think you can define same enum in proto file. If I define a same enum type in proto file, the namespaces are different, it cannot pass when building.
Mostly looks good. https://codereview.chromium.org/2019293002/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/memory_program_cache.cc (right): https://codereview.chromium.org/2019293002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/memory_program_cache.cc:181: interface_block.layout = (sh::BlockLayoutType) proto.layout(); This is against chromium coding style. Please use c++ cast instead. https://codereview.chromium.org/2019293002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/memory_program_cache.cc:187: &(interface_block.fields[ii])); nit: wrong indent.
I defer review of the code to zmo@, but could you please edit the description
here ("Edit issue") to clean it up? That's sufficient -- it's not necessary to
re-upload a new CL. Thanks.
Description was changed from ========== Save interface block infomation to program cache. Save InterfaceBlock info to cache BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. 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 ========== Save interface block infomation to program cache. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. ==========
Description was changed from ========== Save interface block infomation to program cache. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. ========== to ========== Save interface block infomation to program cache. If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html ==========
Description was changed from ========== Save interface block infomation to program cache. If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html ========== to ========== Save interface block infomation to program cache. If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ==========
Description was changed from ========== Save interface block infomation to program cache. If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ========== If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ==========
Description was changed from ========== If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ========== Save interface block infomation to program cache If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ==========
https://codereview.chromium.org/2019293002/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/memory_program_cache.cc (right): https://codereview.chromium.org/2019293002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/memory_program_cache.cc:181: interface_block.layout = (sh::BlockLayoutType) proto.layout(); On 2016/05/31 16:31:00, Zhenyao Mo wrote: > This is against chromium coding style. Please use c++ cast instead. Done. https://codereview.chromium.org/2019293002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/memory_program_cache.cc:187: &(interface_block.fields[ii])); On 2016/05/31 16:31:00, Zhenyao Mo wrote: > nit: wrong indent. Done.
lgtm
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/2019293002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2019293002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by xinghua.cao@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2019293002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/2019293002/40001
Message was sent while issue was closed.
Description was changed from ========== Save interface block infomation to program cache If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ========== Save interface block infomation to program cache If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Save interface block infomation to program cache If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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 ========== Save interface block infomation to program cache If GPU supports program cache, and command buffer does not save interface block information, it cannot get correct uniform information when update uniforms. BUG=615794 TestCase=deqp/functional/gles3/shaderstatequery.html 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/6b03758cf719953b94bcf9007bb3b7ad909e7c96 Cr-Commit-Position: refs/heads/master@{#397291} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/6b03758cf719953b94bcf9007bb3b7ad909e7c96 Cr-Commit-Position: refs/heads/master@{#397291}
Message was sent while issue was closed.
Patchset #4 (id:60001) has been deleted |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
