[Impeller Scene] Add DisplayList OP and Dart bindings - #38676
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. |
jonahwilliams
left a comment
jonahwilliams
left a comment
There was a problem hiding this comment.
If the scene is incoporated into the ... other scene via a color source, I'm almost certain you'd have to drive all animations via the UI thread instead of the raster thread.
As long as we don't raster cache ever on impeller, it should still work, but the separate layer was needed for textures on the Skia backend so we could more easily disable raster caching of parent layers.
For animation behavior that the user is defining inline in the framework, yes. This is a bit of a learning exercise for me, so if there's animation driven by the raster thread in Flutter today, I'd like to take a look -- especially if it doesn't require repaints. :) For externally authored animations embedded in the asset (which will be incredibly intricate), high level controls are triggered on the UI thread and the Node's animation player drives it on the raster thread just before rendering. It seemed like this would be simple and fast enough for now.
Hmm, do CustomPainters get raster cached without additional intervention? |
I'm not sure what we do for external textures, I would start there. Something like video player? I hope we're not driving every frame of a video through the UI thread...
Pretty much anything can get raster cached based on complexity scoring, this caching is not very content aware and can lead to all sorts of fun bugs, like raster caching a picture with a blend mode that depends on the dest - causing the apparent color to change based on whether it is draw in the raster cache picture or normally. I don't think we should worry about this case because we don't have raster caching. |
|
Another appropriate analogy is 2D games today. If someone makes a realtime 2D platformer game powered by Flutter, on which thread will the user compute the character's position for display on the next frame? My understanding is that this will always be done on the UI thread today, as the UI thread stages render instructions for the next frame. |
|
Discussed offfline, but driving via the UI thread is fine for now |
|
Added a DL playground. |
|
|
||
| // In debug mode, allow shaders to be reinitialized. | ||
| developer.registerExtension( | ||
| 'ext.ui.window.reinitializeShader', |
There was a problem hiding this comment.
We should document these service protocol extensions in https://github.com/flutter/flutter/wiki/Engine-specific-Service-Protocol-extensions.
| } | ||
|
|
||
| static impeller::Matrix DefaultCameraTransform() { | ||
| // TODO(bdero): There's not way to know what the draw area will be yet, so |
There was a problem hiding this comment.
"There's no way". Here and elsewhere, perhaps file bugs. But if this is for a demo, we can do a pass later to rethink this.
* b6720a5 Undo axes flip on Mac when shift+scroll-wheel (flutter-team-archive/engine#38338) * 4f0cdcd Inline usage of SkIsPow2 (flutter-team-archive/engine#38722) * 716bb91 [Impeller Scene] Add DisplayList OP and Dart bindings (flutter-team-archive/engine#38676)
| FOR_EACH_DISPLAY_LIST_OP(DL_OP_TO_ENUM_VALUE) | ||
| #ifdef IMPELLER_ENABLE_3D | ||
| DL_OP_TO_ENUM_VALUE(SetSceneColorSource) | ||
| #endif // IMPELLER_ENABLE_3D |
There was a problem hiding this comment.
It seems like a lot of these ifdef blocks could be eliminated in favor of a single ifdef that controls whether or not the FOR_EACH macro contains V(SetSceneColorSource) If that V() entry doesn't exist then the code isn't generated for that op (and vice versa).
4 participants
This lays the ground work for building experimental canvas-backed Scene widgets alongside the framework.
The intention is to enable the following:
1-4 are already covered (although actual ipscene assets aren't bundled yet, of course). For 5 and 6, a simple mutation log will be pushed to the
DlColorSourceand applied on the raster thread prior to rendering (good suggested design by Chinmay).Every UI
SceneNodeis backed by an Impeller SceneNode, but not every Impeller SceneNodebelongs to a UISceneNode; Imported ipscenes are arbitrarily complex scene descriptions, but only the imported scene's root gets referenced by aSceneNode.Screenshot of the DL playground:
