-
Notifications
You must be signed in to change notification settings - Fork 6k
Add function restoreToCount to Canvas. #35798
Conversation
Signed-off-by: xieguo <[email protected]>
Signed-off-by: xieguo <[email protected]>
Signed-off-by: xieguo <[email protected]>
flar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor suggestions on the doc comment which I would like to review when done. Otherwise it looks great!
lib/ui/painting.dart
Outdated
| external void restore(); | ||
|
|
||
| /// Pops the save stack until the count layer, if there is anything to pop. | ||
| /// Otherwise, does nothing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment about how this relates to getSaveCount(). Something along the lines of "the count layer that was previously retrieved with [getSaveCount]" or "After this method completes, the [count] should match the return value of [getSaveCount]".
Also, describe what happens if count > getSaveCount(). I believe it just gets ignored, but we should document that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about:
/// Restores the save stack to a previous level as might be obtained from [getSaveCount].
/// If [count] is less than 1, the stack is restored to its initial state. If [count] is
/// greater than the current [getSaveCount] then nothing happens.
I hinted at what happens with a negative count and also implied that the initial save count is 1 (is that true?) - perhaps we should add a test that verifies these behaviors. I believe that one of the web implementations may not be protected against restoring too far which means that behavior will end up blowing up compared to the other implementations (check that restore protects against an empty stack and also that restoreToCount of values less than 1 all behave the same on all platforms).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get that, I have added test to cover those corner cases.
Signed-off-by: xieguo <[email protected]>
flar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the paragraph mentioned before pushing this change.
lib/ui/painting.dart
Outdated
| /// | ||
| /// Use [save] and [saveLayer] to push state onto the stack. | ||
| /// | ||
| /// If the state was pushed with [saveLayer], then this call will also |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit - this paragraph is worded as if there is only one stack level being restored. Please reword it:
/// If any of the state stack levels restored by this call were pushed with
/// [saveLayer], then this call will also cause those layers to be composited
/// into their previous layers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Get, it's done.
Signed-off-by: xieguo <[email protected]>
Signed-off-by: xieguo <[email protected]>
Signed-off-by: xieguo <[email protected]>
Signed-off-by: xieguo <[email protected]>
ColdPaleLight
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…110796) * f30fed5ab Ignore all translations in Matrix::TransformDirection (flutter/engine#35854) * d8928a0b6 [Impeller] More aggressively downscale gaussian blur textures (flutter/engine#35853) * 5454c09a0 Roll Fuchsia Mac SDK from sgXD5SyRPOxGjWV4q... to VCkCQ9VaBkOXaOnQU... (flutter/engine#35858) * 4ef0eb243 Roll Skia from d505b9445bf6 to 74d7dcaf7a4e (1 revision) (flutter/engine#35857) * 5cab38584 Do frustum culling after the MSAA backdrop draw (flutter/engine#35847) * 4e6bb5da5 Roll Skia from 74d7dcaf7a4e to ca89d2781f78 (1 revision) (flutter/engine#35859) * 5dc718f92 Roll Skia from ca89d2781f78 to 8c7e99e0546e (2 revisions) (flutter/engine#35860) * c3a956050 Roll Skia from 8c7e99e0546e to defbe6b80d7b (1 revision) (flutter/engine#35861) * 1f7fe40d8 Roll Skia from defbe6b80d7b to d8a45111ba9a (1 revision) (flutter/engine#35862) * f74d2439e Add function restoreToCount to Canvas. (flutter/engine#35798) * 152228e27 Roll Skia from d8a45111ba9a to 97f1748bbfc0 (1 revision) (flutter/engine#35863) * 08394d74c Fixed the nullability annotation on codec `writeValue:` (flutter/engine#35814) * 185f4f595 Revert "roll CanvasKit to 0.36.1 (attempt 2) (#35839)" (flutter/engine#35864)
This PR is to fix issue: flutter/flutter#110324
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.