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

Conversation

flar
Copy link
Contributor

@flar flar commented Aug 15, 2023

DisplayListBuilder grew over time from a class that implemented a developer-unfriendly stateful API into one that implemented both the developer-friendly DlCanvas API as well as its original stateful API. Over time, the stateful API was buried under a "testing only" facade.

In the meantime, the optimization features that it applies to the DlCanvas calls before it records the operations in a DlOp store are useful without the recording process and so I've been wanting to break those into 2 parts for a while with the goal of removing all stateful APIs from DisplayListBuilder (see flutter/flutter#108303).

This PR takes a major step along that direction by splitting DisplayListBuilder into essentially a convenience class that marries 2 new classes together to achieve its old functionality:

  • DlCanvasToReceiver - a class that implements DlCanvas, optimizes common situations, and then sends commands to any object that implements DlOpReceiver
  • DlOpRecorder - an implementation of DlOpReceiver that records the operations in a buffer from which to create a DisplayList object
  • DisplayListBuilder now inherits from DlCanvasToReceiver to get the optimizations and provides it with an instance of DlOpRecorder as the receiver that it will send its results to
  • Similarly, a DlCanvasToReceiver instance could be directed to an impeller:DlDispatcher to achieve a more straight-through path from the DlCanvas interface to impeller Pictures.

@flar
Copy link
Contributor Author

flar commented Aug 15, 2023

It looks like a lot of new code, but really most of it came from just slicing the old DisplayListBuilder in 2 (I hoped to copy from dl_builder.{cc,h} into the 2 new files to get git to see the family resemblance and create diffs along those lines, but I don't think that git-magic was successful).

Other things that caused lots of diffs for no great reason:

  • Adding a & reference tag to some of the arguments in DlOpReceiver which meant editing any implementor of that interface
  • Updating lots of tests that were relying on calling the "Receiver" methods on DisplayListBuilder and getting optimizations - now that its function is split into the optimizer which only works on DlCanvas methods and the recorder which is just a direct-to-buffer stenographer they couldn't mix and match that way and had to be rewritten to the DlCanvas interface. That's actually a good thing as cleaning up all remaining uses of the Receiver APIs in the tests is on the wish list, so this PR merely prompted me to get started on that.

@flar flar force-pushed the split-dl-builder branch from 2abee91 to 8cc2f06 Compare August 20, 2023 02:29
@flar flar added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 21, 2023
@auto-submit auto-submit bot merged commit 36ab259 into flutter:main Aug 21, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 21, 2023
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Aug 21, 2023
…132937)

flutter/engine@e7d7451...36ab259

2023-08-21 [email protected] Split DisplayListBuilder into DlCanvas optimizer and DlOp recorder classes (flutter/engine#44718)
2023-08-21 [email protected] Roll Skia from d2369dac4a1d to fca8fac08117 (1 revision) (flutter/engine#44888)

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 [email protected],[email protected],[email protected] 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flar added a commit that referenced this pull request Aug 22, 2023
auto-submit bot pushed a commit that referenced this pull request Aug 22, 2023
…order classes" (#44968)

Reverts #44718

A rendering issue was discovered in internal testing (b/296975714)
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…asses (flutter#44718)

DisplayListBuilder grew over time from a class that implemented a developer-unfriendly stateful API into one that implemented both the developer-friendly DlCanvas API as well as its original stateful API. Over time, the stateful API was buried under a "testing only" facade.

In the meantime, the optimization features that it applies to the DlCanvas calls before it records the operations in a DlOp store are useful without the recording process and so I've been wanting to break those into 2 parts for a while with the goal of removing all stateful APIs from DisplayListBuilder (see flutter/flutter#108303).

This PR takes a major step along that direction by splitting DisplayListBuilder into essentially a convenience class that marries 2 new classes together to achieve its old functionality:
- `DlCanvasToReceiver` - a class that implements DlCanvas, optimizes common situations, and then sends commands to any object that implements `DlOpReceiver`
- `DlOpRecorder` - an implementation of DlOpReceiver that records the operations in a buffer from which to create a `DisplayList` object
- `DisplayListBuilder` now inherits from DlCanvasToReceiver to get the optimizations and provides it with an instance of `DlOpRecorder` as the receiver that it will send its results to
- Similarly, a `DlCanvasToReceiver` instance could be directed to an `impeller:DlDispatcher` to achieve a more straight-through path from the DlCanvas interface to impeller Pictures.
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
…order classes" (flutter#44968)

Reverts flutter#44718

A rendering issue was discovered in internal testing (b/296975714)
gaaclarke pushed a commit to gaaclarke/engine that referenced this pull request Aug 30, 2023
zanderso added a commit to zanderso/engine that referenced this pull request Aug 31, 2023
XilaiZhang added a commit to XilaiZhang/engine that referenced this pull request Sep 11, 2023
XilaiZhang added a commit to XilaiZhang/engine that referenced this pull request Sep 11, 2023
XilaiZhang added a commit that referenced this pull request Sep 11, 2023
…as optimizer and … (#45680)

…DlOp recorder classes #44718" (#45085)"

This reverts commit
1382d6d.

context: b/299886812

manual revert. the revert in main contains two commits and cannot be
cherry picked cleanly.
XilaiZhang added a commit that referenced this pull request Sep 11, 2023
…s optimizer and … (#45678)

…DlOp recorder classes #44718" (#45085)"

This reverts commit 1382d6d.

context: b/299837297

manual revert. the revert in main contains two commits and cannot be
cherry picked cleanly.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App e: impeller
Projects
Status: Done
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants