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

Issue 282253002: Remove unneeded shm versions of bucket functions from command buffer (Closed)

Created:
6 years, 7 months ago by Kimmo Kinnunen
Modified:
6 years, 6 months ago
Reviewers:
vmiura, jbauman, piman
CC:
chromium-reviews, piman+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@no-shm-for-immediate-commands
Visibility:
Public.

Description

Remove unneeded shm versions of bucket functions from command buffer Remove shm versions of bucket functions from command buffer for commands that do not need two different ways to send pointer data. The actual code uses only bucket commands. The shm versions are either unused or used only by the unit tests. Make the unit tests use the immediate commands. Solves the problem of not having to implement one useless function for the a new command when using bucket commands. Support the case of CompressedTex{,Sub}Image2D, where the function is implemented with shm and bucket commands. In the code generator function info table, replace 'immediate': False and 'bucket': True properties of function info objects with 'data_transfer_method': ['immediate', 'bucket', 'shm' ] property. Remove the handwritten GetAttribLocation command and rename GetAttribLocationBucket to GetAttribLocation. This is consistent with other handwritten commands: the command name is the gl call name, and it just happens to use buckets to communicate some of its arguments. The command does not have 'Bucket' prefix due to it not being a 'Bucket' variant. Do the same with GetUniformLocation and GetUniformLocationBucket. BUG=373763 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=277697

Patch Set 1 #

Patch Set 2 : rebase #

Patch Set 3 : rebase #

Total comments: 2

Patch Set 4 : rebase (1 new added hunk) #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+565 lines, -1399 lines) Patch
M gpu/command_buffer/build_gles2_cmd_buffer.py View 1 2 3 39 chunks +66 lines, -74 lines 2 comments Download
M gpu/command_buffer/client/gles2_cmd_helper.h View 1 2 3 1 chunk +4 lines, -29 lines 0 comments Download
M gpu/command_buffer/client/gles2_cmd_helper_autogen.h View 1 2 3 6 chunks +24 lines, -58 lines 0 comments Download
M gpu/command_buffer/client/gles2_implementation.cc View 1 2 3 2 chunks +4 lines, -4 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_format.h View 1 2 3 5 chunks +24 lines, -158 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_format_autogen.h View 1 2 3 7 chunks +109 lines, -279 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_format_test_autogen.h View 1 2 3 12 chunks +31 lines, -89 lines 0 comments Download
M gpu/command_buffer/common/gles2_cmd_ids_autogen.h View 1 2 3 1 chunk +195 lines, -200 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder.cc View 1 2 3 5 chunks +0 lines, -67 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_1.cc View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_1_autogen.h View 1 5 chunks +47 lines, -8 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_2.cc View 1 2 3 1 chunk +0 lines, -19 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_2_autogen.h View 1 2 3 4 chunks +8 lines, -50 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_3_autogen.h View 1 2 3 1 chunk +0 lines, -8 lines 0 comments Download
M gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc View 8 chunks +33 lines, -356 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
Kimmo Kinnunen
Not needed for path rendering, but kind of consistency fix if following dependency is accepted: ...
6 years, 7 months ago (2014-05-15 13:10:24 UTC) #1
vmiura
On 2014/05/15 13:10:24, kkinnunen wrote: > Not needed for path rendering, but kind of consistency ...
6 years, 6 months ago (2014-05-28 19:52:04 UTC) #2
Kimmo Kinnunen
Yeah, this is ready. Thanks.
6 years, 6 months ago (2014-05-30 04:38:11 UTC) #3
vmiura
Thanks. When ready for review please notify reviewers with 'PTAL' or similar comment.
6 years, 6 months ago (2014-05-30 16:58:47 UTC) #4
vmiura
https://codereview.chromium.org/282253002/diff/30001/gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc File gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc (right): https://codereview.chromium.org/282253002/diff/30001/gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc#newcode728 gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc:728: #if GLES2_TEST_SHADER_VS_PROGRAM_IDS This appears in other places, but is ...
6 years, 6 months ago (2014-05-31 01:17:39 UTC) #5
Kimmo Kinnunen
https://codereview.chromium.org/282253002/diff/30001/gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc File gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc (right): https://codereview.chromium.org/282253002/diff/30001/gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc#newcode728 gpu/command_buffer/service/gles2_cmd_decoder_unittest_programs.cc:728: #if GLES2_TEST_SHADER_VS_PROGRAM_IDS On 2014/05/31 01:17:39, vmiura wrote: > This ...
6 years, 6 months ago (2014-06-02 04:59:45 UTC) #6
Kimmo Kinnunen
Ping vmiura, would you have time to look at this? Do you need any more ...
6 years, 6 months ago (2014-06-09 12:28:41 UTC) #7
vmiura
lgtm
6 years, 6 months ago (2014-06-09 17:23:01 UTC) #8
piman
lgtm
6 years, 6 months ago (2014-06-09 17:54:08 UTC) #9
Kimmo Kinnunen
The CQ bit was checked by kkinnunen@nvidia.com
6 years, 6 months ago (2014-06-12 05:06:34 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kkinnunen@nvidia.com/282253002/30001
6 years, 6 months ago (2014-06-12 05:09:26 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_aosp on tryserver.chromium ...
6 years, 6 months ago (2014-06-12 07:52:27 UTC) #12
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-06-12 07:58:37 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: linux_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/26795) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15953)
6 years, 6 months ago (2014-06-12 07:58:38 UTC) #14
Kimmo Kinnunen
When rebasing, I had to add one very small hunk of code on top of ...
6 years, 6 months ago (2014-06-16 06:09:25 UTC) #15
piman
lgtm
6 years, 6 months ago (2014-06-16 19:13:03 UTC) #16
Kimmo Kinnunen
The CQ bit was checked by kkinnunen@nvidia.com
6 years, 6 months ago (2014-06-17 04:50:40 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kkinnunen@nvidia.com/282253002/50001
6 years, 6 months ago (2014-06-17 04:51:33 UTC) #18
commit-bot: I haz the power
6 years, 6 months ago (2014-06-17 10:06:07 UTC) #19
Message was sent while issue was closed.
Change committed as 277697

Powered by Google App Engine
This is Rietveld 408576698