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

Conversation

@kjlubick
Copy link
Contributor

In https://skia-review.googlesource.com/c/skia/+/716476, Skia removed calls to SkCanvas::flush(). This replaces those calls that Flutter was making with what had been going on - calling GrDirectContext::flush() if the canvas was backed by a Ganesh backend. Raster-backed canvases did not need flushing.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
  • I listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on writing and running engine tests.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@flar FYI

@kjlubick kjlubick merged commit 21b8e68 into flutter:main Jul 14, 2023
@kjlubick kjlubick deleted the canvas-flushing2 branch July 14, 2023 20:34
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 14, 2023
harryterkelsen pushed a commit to harryterkelsen/engine that referenced this pull request Jul 20, 2023
In https://skia-review.googlesource.com/c/skia/+/716476, Skia removed
calls to SkCanvas::flush(). This replaces those calls that Flutter was
making with what had been going on - calling GrDirectContext::flush() if
the canvas was backed by a Ganesh backend. Raster-backed canvases did
not need flushing.

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++,
Objective-C, Java style guides].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] I added new tests to check the change I am making or feature I am
adding, or Hixie said the PR is test-exempt. See [testing the engine]
for instructions on writing and running engine tests.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [x] I signed the [CLA].
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#overview
[Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene
[Flutter Style Guide]:
https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo
[C++, Objective-C, Java style guides]:
https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style
[testing the engine]:
https://github.com/flutter/flutter/wiki/Testing-the-engine
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes
[Discord]: https://github.com/flutter/flutter/wiki/Chat
psydox pushed a commit to psydox/skia that referenced this pull request Jul 26, 2023
Client CLs:
 - https://crrev.com/c/4636561
 - flutter/engine#43684
 - flutter/engine#43902
 - flutter/engine#43965
 - http://ag/24023223
 - http://ag/24036771
 - http://cl/549054892
 - http://cl/548702476
 - http://cl/548179304
 - http://cl/548167668
 - http://cl/549996422
 - http://cl/547840356

Change-Id: I657d85bf9026dabfe8c872fe31d6e4ed49a027e7
Bug: skia:14317
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/727040
Commit-Queue: Kevin Lubick <[email protected]>
Reviewed-by: Brian Osman <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants