Don't compile the Skia GL backend on iOS. #34924
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
It isn't clear what needs to be done to land the Skia patch in lockstep with this one. Also, the !OS_FUCHSIA stuff can probably be avoided but decided I could do that in another patch to keep the differences small as I couldn't test the Fuchsia stuff on Mac. |
chinmaygarde
changed the title
| sk_sp<GrDirectContext> ShellIOManager::CreateCompatibleResourceLoadingContext( | ||
| GrBackend backend, | ||
| sk_sp<const GrGLInterface> gl_interface) { | ||
| #if !OS_FUCHSIA && SK_GL |
There was a problem hiding this comment.
Is SK_GL any different from SHELL_ENABLE_GL in this context?
There was a problem hiding this comment.
Yes, SHELL_ENABLE_GL is only available in the GPU specific code. Unfortunately, this is in shell/common. Ideally, this selection should be moved to shell/gpu but that is a larger change.
There was a problem hiding this comment.
nit: add a TODO and file a bug to move this to shell/gpu
There was a problem hiding this comment.
Removed the confusing OS_FUCHSIA stuff in #34959. Followup for moving to shell/gpu in flutter/flutter#108502.
tools/gn
Outdated
| gn_args['skia_use_expat'] = args.target_os == 'android' | ||
| gn_args['skia_use_fontconfig'] = args.enable_fontconfig | ||
|
|
||
| if args.target_os == 'ios': |
There was a problem hiding this comment.
There's similar code starting around line 407.
|
Presumably the Skia roll with the above CL will fail, but will pass when patched into this PR. That's probably how we'll have to land them. |
|
Roger. That doesn't have a review yet. Once that lands and a roll is attempted, I'll rebase. |
|
The Skia roller is blocked on an unrelated issue and I am waiting for the roll with the linked CL to be attempted. |
chinmaygarde
force-pushed
the
skia_use_gl_false_ios
branch
from
deabec6 to
1aa87aa
Compare
|
The patch has rolled in and I've rebased here #34949. |
chinmaygarde
added
the
autosubmit
|
auto-submit
bot
removed
the
autosubmit
chinmaygarde
added
the
autosubmit
`SK_GL` should already be unset on Fuchsia builds. Followup based [this thread](#34924 (comment)).
3 participants
Depends on https://skia-review.googlesource.com/c/skia/+/562341