[Engine] Add no op surface #54694
[Engine] Add no op surface #54694
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). 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. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
|
|
||
| namespace flutter { | ||
|
|
||
| class GPUSurfaceNoop : public Surface { |
There was a problem hiding this comment.
A short docstring saying something like "Accepts rendering intent but doesn't render anything. Useful for tests that don't care about checking rendering output but still need to run a Flutter app." Or something like that.
shell/gpu/gpu_surface_noop.h
Outdated
| // |Surface| | ||
| std::shared_ptr<impeller::AiksContext> GetAiksContext() const override; | ||
|
|
||
| FML_DISALLOW_COPY_AND_ASSIGN(GPUSurfaceNoop); |
There was a problem hiding this comment.
Here and elsewhere, avoid using the macros.
chinmaygarde
left a comment
There was a problem hiding this comment.
The creation of the context happens in
. Perhaps (in simulator builds only) we can add a hook for tests to return nullptr from there to test the fallback logic?|
I don't think overriding it at that location would be sufficient, nor would a unit test cover it. |
|
I think no matter what we do, we can't really be certain thtat it works for all of the various engine APIS. for example, what happens if the noop surface is asked to use the snapshot controller? or upload an image. Today we may count on that falling back to software. I'd rather not spin up an entire new test shard for this. Maybe we land and then explore the impact in g3? |
flutter/engine@41f539f...ba1168f 2024-08-23 jonahwilliams@google.com [Engine] Add no op surface (flutter/engine#54694) 2024-08-23 jonahwilliams@google.com [engine] make Platform thread the UI thread for iOS Impeller. (flutter/engine#54655) 2024-08-23 mdebbar@google.com [web] Multi-view support for Skwasm (flutter/engine#48893) 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 jimgraham@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
…4020) flutter/engine@41f539f...ba1168f 2024-08-23 jonahwilliams@google.com [Engine] Add no op surface (flutter/engine#54694) 2024-08-23 jonahwilliams@google.com [engine] make Platform thread the UI thread for iOS Impeller. (flutter/engine#54655) 2024-08-23 mdebbar@google.com [web] Multi-view support for Skwasm (flutter/engine#48893) 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 jimgraham@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
Still needs to be tested in metalless environment. This should probably print out some kind of error message that tells folks that this is WAI, but I'm not sure if that would be too disruptive for g3.
Fixes flutter/flutter#153883