Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't use Skia BUILD.gn files #47677

Merged
merged 1 commit into from
Nov 7, 2023
Merged

Don't use Skia BUILD.gn files #47677

merged 1 commit into from
Nov 7, 2023

Conversation

zanderso
Copy link
Member

@zanderso zanderso commented Nov 4, 2023

The new BUILD.gn files in the Engine tree can't go under build/secondary because Skia still has its own, and they'd be selected first. So, this PR puts the new BUILD.gn files under flutter/skia.

@zanderso
Copy link
Member Author

zanderso commented Nov 6, 2023

@kjlubick I'd like to get your thoughts on this approach. Instead of starting from the Chrome BUILD.gn file, I copied https://github.com/google/skia/blob/main/BUILD.gn and trimmed it down.

Copy link
Contributor

@kjlubick kjlubick left a comment

Choose a reason for hiding this comment

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

In general, I think it's fine. I have some specific comments to address to further clean things up.

skia/BUILD.gn Outdated Show resolved Hide resolved
skia/BUILD.gn Outdated Show resolved Hide resolved
skia/BUILD.gn Outdated Show resolved Hide resolved
sources = [ "$_skia_root/src/ports/SkFontMgr_custom.cpp" ]
}

optional("fontmgr_custom_directory") {
Copy link
Contributor

Choose a reason for hiding this comment

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

You might be able to trim a few of these fontmgr configurations

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, good point. My plan is to do some deeper cleanups in a follow-up once this is landed and working.

@zanderso zanderso merged commit 64d5af2 into flutter:main Nov 7, 2023
23 checks passed
@zanderso zanderso deleted the mv-skia branch November 7, 2023 19:12
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 7, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Nov 7, 2023
…138034)

flutter/engine@1b20752...f8961d2

2023-11-07 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] Add support for specialization constants redux." (flutter/engine#47762)
2023-11-07 30870216+gaaclarke@users.noreply.github.com [Impeller] added tests for matrices (flutter/engine#47754)
2023-11-07 skia-flutter-autoroll@skia.org Roll Skia from 62fc1374cc5d to 030e21befbc9 (2 revisions) (flutter/engine#47756)
2023-11-07 ychris@google.com [ios] making objective-C smart pointers support ARC (flutter/engine#47612)
2023-11-07 zanderso@users.noreply.github.com Don't use Skia BUILD.gn files (flutter/engine#47677)
2023-11-07 jonahwilliams@google.com [Impeller] Add support for specialization constants redux. (flutter/engine#47678)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC bdero@google.com,rmistry@google.com,zra@google.com on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants