Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Create DlCanvas interface and implement with DisplayListBuilder and SkCanvasAdapter #39762

Merged
merged 2 commits into from
Feb 24, 2023

Conversation

flar
Copy link
Contributor

@flar flar commented Feb 20, 2023

Fixes: flutter/flutter#109578

This PR introduces a new DlCanvas interface which will be the only interface used for rendering moving forward.

With the changes here, nearly all code in the engine is moved off of SkCanvas/SkPaint onto DlCanvas/DlPaint with some exceptions for platform code and for code which was already using DisplayListBuilder instead. Moving everything From DisplayListBuilder onto DlCanvas will be a follow-on task.

These changes are consistent with a new direction for the DL source code organization. The new code follows a hierarchical code tree under flutter/display_list/ and new files are named dl_... instead of display_list_.... Old code is left using the flat hierarchy with the old file naming for now, but a mass change may be coming before too long to get all of the code to conform. Also, the intent is that DlCanvas will follow Flutter style guidelines rather than Skia-like style, but the rest of the files will remain Skia-ish and converge over time.

The code is compiling on a Mac for host, iOS and Android and most unit tests pass on that host (and a brief test on Linux).

@chinmaygarde
Copy link
Member

Besides just the sheer number of lines of code touched, the patch looks relatively safe and only modifies internal interfaces. I expect a few more patches of this nature as we cleanup the internals after the display list migration. Is this a draft only till we fix the Fuchsia interfaces or is anything else remaining? Its best not to sit on this for too long and let the conflicts pile up.

@flar
Copy link
Contributor Author

flar commented Feb 20, 2023

Mainly a draft because I didn't know how many failures I'd see during CI, but now that it is down to just a couple, I'll "undraft" it...

@flar flar removed the Work in progress (WIP) Not ready (yet) for review! label Feb 20, 2023
@flar flar marked this pull request as ready for review February 20, 2023 21:59
@flar
Copy link
Contributor Author

flar commented Feb 22, 2023

I've narrowed down the test failure to a side effect of a recent optimization. I can comment out FlutterPlatformViews.mm line 801 (a ClipRect when rendering embedder slice content over the embedder layer) and the test passes, but that would eliminate the recently added optimization.

@cyanglaz any thoughts?

@flar
Copy link
Contributor Author

flar commented Feb 22, 2023

While @chinmaygarde has already approved this (pending fixing the last test failure), I'd like to call attention to other engineers who work on the DL and related systems a bit... @JsouLiang @ColdPaleLight @bdero @jason-simmons

@flutter-dashboard
Copy link

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #39762 at sha 4f2533a

@flar
Copy link
Contributor Author

flar commented Feb 23, 2023

I believe I have the final piece of the puzzle. The android embedder failures appear to be due to the way we record setTransform in DisplayList objects and play them back into SkCanvas compared to the way that used to be done by SkPicture. If I comment out all of the calls to "integralTransform" which is the only place we use setTransform, things show up fine. I need more investigation, but I think SkPicture treats a setTransform as "set the transform back to what it was before drawPicture was called", but our DL dispatcher sets it to identity.

Yes, this appears to be the case:
https://skia.googlesource.com/skia/+/refs/heads/main/src/core/SkPicturePlayback.cpp#50
and:
https://skia.googlesource.com/skia/+/refs/heads/main/src/core/SkPicturePlayback.cpp#694

@flar flar merged commit 0d5b780 into flutter:main Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 24, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 24, 2023
sourcegraph-bot pushed a commit to sgtest/megarepo that referenced this pull request Feb 24, 2023
…ayListBuilder and SkCanvasAdapter (flutter/engine#39762) (#121390)

Commit: 5db8faecfb8bfb105e5b1a4c8d535494721fb280
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 25, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 26, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Status: Done
2 participants