Skip to content

Put shaders under a 'shaders' section in the manifest#106752

Merged
zanderso merged 1 commit intoflutter:masterfrom
zanderso:flutter-shader-assets
Jun 29, 2022
Merged

Put shaders under a 'shaders' section in the manifest#106752
zanderso merged 1 commit intoflutter:masterfrom
zanderso:flutter-shader-assets

Conversation

@zanderso
Copy link
Member

@zanderso zanderso commented Jun 28, 2022

This is a step towards no longer identifying shaders that need compilation by way of the .frag extension.

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 28, 2022
@zanderso
Copy link
Member Author

zanderso commented Jun 28, 2022

@jonahwilliams @christopherfujino Thoughts on introducing AssetKind to signal that a transform is needed? Maybe ideally AssetKind would be a class instead of an enum, and the class could encode how the asset needs to be transformed, but that might be overkill at the moment.

@jonahwilliams
Copy link
Contributor

jonahwilliams commented Jun 28, 2022

I like it

Thoughts on introducing AssetKind to signal that a transform is needed? Maybe ideally AssetKind would be a class instead of an enum

YAGNI. If it gets too unwieldy for whatever reason we can refactor.

jonahwilliams
jonahwilliams approved these changes Jun 28, 2022
View reviewed changes
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@zanderso zanderso merged commit 9c0f239 into flutter:master Jun 29, 2022
@zanderso zanderso deleted the flutter-shader-assets branch June 29, 2022 02:31
@zanderso
Copy link
Member Author

zanderso commented Jun 29, 2022

FRoB isn't running on pre-merge PRs at the moment, so I expect that this will need some follow-up after it runs post-merge.

@CaseyHillers
Copy link
Contributor

CaseyHillers commented Jun 29, 2022

FRoB isn't running on pre-merge PRs at the moment, so I expect that this will need some follow-up after it runs post-merge.

Is there a bug for this? I wasn't aware presubmit testing was broken \cc @XilaiZhang @muditatandon it might be related to b/236975711

CaseyHillers added a commit that referenced this pull request Jun 29, 2022
@CaseyHillers
Revert "Put shaders under a 'shaders' section in the manifest (#106752)"
59c10c6 
This reverts commit 9c0f239.
@CaseyHillers CaseyHillers mentioned this pull request Jun 29, 2022
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
67cea57 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
7c211ee 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
60ee54d 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
58681fc 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
3aa6ebd 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
b51b538 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
183f8e3 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
903327e 
…r/flutter#106752)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jun 29, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 29, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
a40dfbf 
…r/flutter#106752)
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
@zanderso @camsim99
Put shaders under a 'shaders' section in the manifest (flutter#106752)
5fd8059
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
c612c45 
…r/flutter#106752)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
@engine-flutter-autoroll
9c0f239 Put shaders under a 'shaders' section in the manifest (flutte…
4a03955 
…r/flutter#106752)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jonahwilliams jonahwilliams jonahwilliams approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@zanderso @jonahwilliams @CaseyHillers