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
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Jul 14, 2023
…130620)

flutter/engine@2fabf4e...21b8e68

2023-07-14 [email protected] Remove calls to SkCanvas::flush() (flutter/engine#43684)
2023-07-14 [email protected] Roll Skia from c14fda1cb615 to 315c7f08c731 (3 revisions) (flutter/engine#43700)

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] 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
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]>
LouiseHsu pushed a commit to LouiseHsu/flutter that referenced this pull request Jul 31, 2023
…lutter#130620)

flutter/engine@2fabf4e...21b8e68

2023-07-14 [email protected] Remove calls to SkCanvas::flush() (flutter/engine#43684)
2023-07-14 [email protected] Roll Skia from c14fda1cb615 to 315c7f08c731 (3 revisions) (flutter/engine#43700)

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] 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
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