Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2405893002:
gpu, cmaa: reduce one-copy (Closed)
Descriptiongpu, cmaa: reduce one-copy
Merge copy into the detect edges pass 0. As the pass reads all texel of source
texture, it can copy source texture to RGBA8 by itself.
So it removes one glCopyTexSubImage2D call, which means one read memory
bandwidth is saved.
In addition, it cleans up inconsistent image texture variable names. 'float4'
doesn't mean anything. Change float4 to rgba.
* edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_
* edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_
* remove edges_combine_shader_result_texture_float4_slot1_
* process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_
* edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_
* introduce edges0_shader_target_texture_slot2_
BUG=535198
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
Committed: https://crrev.com/2058173d586111ff8b4d19ddf5cb53fc6a3233ca
Cr-Commit-Position: refs/heads/master@{#425668}
Patch Set 1 #Patch Set 2 : gpu, cmaa: reduce one-copy #
Total comments: 2
Patch Set 3 : add helper comment #
Messages
Total messages: 32 (22 generated)
Description was changed from ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 ========== to ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_optional_gpu_tests_rel on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_optional_...)
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: + adrian.belgun@intel.com, zmo@chromium.org
zmo@, could you review this optimization?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm with nits. https://codereview.chromium.org/2405893002/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2405893002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:252: if (do_copy) { Nit: Add comment for clarity stating that rgba8_texture_ is bound to copy_framebuffer_. It took me a while to understand why you've removed the call to glFramebufferTexture2DEXT.
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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
zmo@, could you review this optimization? https://codereview.chromium.org/2405893002/diff/20001/gpu/command_buffer/serv... File gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc (right): https://codereview.chromium.org/2405893002/diff/20001/gpu/command_buffer/serv... gpu/command_buffer/service/gles2_cmd_apply_framebuffer_attachment_cmaa_intel.cc:252: if (do_copy) { On 2016/10/11 13:47:23, adrian.belgun wrote: > Nit: Add comment for clarity stating that rgba8_texture_ is bound to > copy_framebuffer_. > It took me a while to understand why you've removed the call to > glFramebufferTexture2DEXT. ah sorry. Done.
Description was changed from ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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 ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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 ==========
dongseong.hwang@intel.com changed reviewers: + piman@chromium.org
piman@, could you review this optimization?
lgtm
The CQ bit was checked by dongseong.hwang@intel.com
The patchset sent to the CQ was uploaded after l-g-t-m from adrian.belgun@intel.com Link to the patchset: https://codereview.chromium.org/2405893002/#ps40001 (title: "add helper comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)
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: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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 ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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 ========== gpu, cmaa: reduce one-copy Merge copy into the detect edges pass 0. As the pass reads all texel of source texture, it can copy source texture to RGBA8 by itself. So it removes one glCopyTexSubImage2D call, which means one read memory bandwidth is saved. In addition, it cleans up inconsistent image texture variable names. 'float4' doesn't mean anything. Change float4 to rgba. * edges1_shader_result_texture_float4_slot1_ -> edges0_shader_result_rgba_texture_slot1_ * edges1_shader_result_texture_ -> edges1_shader_result_edge_texture_ * remove edges_combine_shader_result_texture_float4_slot1_ * process_and_apply_shader_result_texture_float4_slot1_ -> process_and_apply_shader_result_rgba_texture_slot1_ * edges_combine_shader_result_texture_slot2_ -> edges_combine_shader_result_edge_texture_ * introduce edges0_shader_target_texture_slot2_ BUG=535198 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 Committed: https://crrev.com/2058173d586111ff8b4d19ddf5cb53fc6a3233ca Cr-Commit-Position: refs/heads/master@{#425668} ==========
Message was sent while issue was closed.
Patchset 3 (id:??) landed as https://crrev.com/2058173d586111ff8b4d19ddf5cb53fc6a3233ca Cr-Commit-Position: refs/heads/master@{#425668} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
