Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2605343002:
The great shader refactor: Delete instances of GetShaderSource (Closed)
DescriptionThe great shader refactor: Delete instances of GetShaderSource
There comes a time, around the 20th time that we copy-paste the same
function into a new subclass, that we should consider moving the
function to the base class.
For GetShaderSource, that moment has arrived.
Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that
- it is indeed the base class of all fragment programs
- even the ones that don't use custom blend modes
- even the ones that don't use texturing
Also also, merge fragment "head" and "body" distinction in shaders. They
will all be deleted soon, and merging them makes seeing common patterns
easier.
TBR=ericrk
BUG=667966
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
Committed: https://crrev.com/c9e49bd0436fa86076fb867417d9462b815db6c1
Cr-Commit-Position: refs/heads/master@{#441013}
Patch Set 1 #Patch Set 2 : Rename base class #Patch Set 3 : Remove FRAGMENT_PROGRAM macro entirely #Patch Set 4 : Just GetShaderSource #
Depends on Patchset: Dependent Patchsets: Messages
Total messages: 24 (20 generated)
Description was changed from ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 14th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentBase to reflect that - it is the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing BUG=667966 ========== to ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 14th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentBase to reflect that - it is the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
Description was changed from ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 14th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentBase to reflect that - it is the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
The CQ bit was checked by ccameron@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.
The CQ bit was checked by ccameron@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: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
The CQ bit was checked by ccameron@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.
Description was changed from ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing Also also, merge fragment "head" and "body" distinction in shaders. They will all be deleted soon, and merging them makes seeing common patterns easier. TBR=ericrk BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ==========
ccameron@chromium.org changed reviewers: + ericrk@chromium.org
I'm going to be doing this as a TBR because: - everyone is gone - this is part of a sequence of changes, and i want to have space between the elements of the sequence (so we can bisect, if there are any problems).
The CQ bit was checked by ccameron@chromium.org
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": 60001, "attempt_start_ts": 1483062118180000,
"parent_rev": "30af8201fa60189bad08ea7c4bba1b9c9544387d", "commit_rev":
"3dc30f8bf1865bf08c23de88366597978b4607dc"}
Message was sent while issue was closed.
Description was changed from ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing Also also, merge fragment "head" and "body" distinction in shaders. They will all be deleted soon, and merging them makes seeing common patterns easier. TBR=ericrk BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel ========== to ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing Also also, merge fragment "head" and "body" distinction in shaders. They will all be deleted soon, and merging them makes seeing common patterns easier. TBR=ericrk BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2605343002 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing Also also, merge fragment "head" and "body" distinction in shaders. They will all be deleted soon, and merging them makes seeing common patterns easier. TBR=ericrk BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Review-Url: https://codereview.chromium.org/2605343002 ========== to ========== The great shader refactor: Delete instances of GetShaderSource There comes a time, around the 20th time that we copy-paste the same function into a new subclass, that we should consider moving the function to the base class. For GetShaderSource, that moment has arrived. Also rename FragmentTexBlendMode to FragmentShaderBase to reflect that - it is indeed the base class of all fragment programs - even the ones that don't use custom blend modes - even the ones that don't use texturing Also also, merge fragment "head" and "body" distinction in shaders. They will all be deleted soon, and merging them makes seeing common patterns easier. TBR=ericrk BUG=667966 CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel Committed: https://crrev.com/c9e49bd0436fa86076fb867417d9462b815db6c1 Cr-Commit-Position: refs/heads/master@{#441013} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/c9e49bd0436fa86076fb867417d9462b815db6c1 Cr-Commit-Position: refs/heads/master@{#441013} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
