This repository was archived by the owner on Feb 25, 2025. It is now read-only.
add DlPaint object for optional use with DisplayListBuilder #32860
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dnfield
reviewed
Apr 22, 2022
Comment on lines
+328
to
+330
| // if (flags.applies_path_effect()) { | ||
| // setPathEffect(sk_ref_sp(paint.getPathEffect())); | ||
| // } |
Contributor
There was a problem hiding this comment.
Commented out code.
We need this right? Text can do it?
Contributor
Author
There was a problem hiding this comment.
I'm waiting for the DlPathEffect PR to land which will happen any day/hour now. It's just waiting for the developer to rebase one of their PRs.
Contributor
There was a problem hiding this comment.
Can you add a TODO with a link to the issue/PR?
dnfield
reviewed
Apr 22, 2022
Comment on lines
+16
to
+25
| class DlColor { | ||
| public: | ||
| DlColor() : argb(0xFF000000) {} | ||
| DlColor(uint32_t argb) : argb(argb) {} | ||
|
|
||
| uint32_t argb; | ||
|
|
||
| bool operator==(DlColor const& other) const { return argb == other.argb; } | ||
| bool operator!=(DlColor const& other) const { return argb != other.argb; } | ||
| }; |
Contributor
There was a problem hiding this comment.
nit: any reason to not just have this be a typedef?
Contributor
Author
There was a problem hiding this comment.
A couple of nit-ty reasons.
- SkColor being a typedef means you can't overload off of it.
- Other type-safety-ish consequences
- (In my "verbose DL compare" code, for example, I had to wrap all SkColors in a class to get them to print easily)
- Potentially these can contain a 4-float version, but I haven't decided how to manage that distinction yet.
- Starting with a (simple, non-virtual) class will demonstrate if there are any negative consequences as we move forward with this
flar
added
the
waiting for tree to go green
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 23, 2022
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 23, 2022
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Apr 23, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes: flutter/flutter#102368
This PR adds a DlPaint object which can encapsulate the standard attributes that Flutter uses. It also adds overloads to the draw calls in DisplayListBuilder that take this object and makes sure the stream is updated with the necessary attributes it contains. (i.e.
builder.drawRect(rect, DlPaint()))The stream of records still remains stateful (for now), and the Dispatcher interface is still stateful, but that can evolve in the future for convenience of developers needing to interpret the DisplayLists.