Skip to content

shader warm up with canvaskit and corresponding test#113060

Merged
auto-submit[bot] merged 6 commits intoflutter:masterfrom
alanwutang11:shader-warmup-with-canvasKit
Oct 6, 2022
Merged

shader warm up with canvaskit and corresponding test#113060
auto-submit[bot] merged 6 commits intoflutter:masterfrom
alanwutang11:shader-warmup-with-canvasKit

Conversation

@alanwutang11
Copy link
Contributor

@alanwutang11 alanwutang11 commented Oct 6, 2022

shader warm up used to be conditional behind kIsWeb flag. Fixed by distinguishing between CanvasKit and HTML with another boolean.

fixes #75801

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], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Oct 6, 2022
harryterkelsen
harryterkelsen reviewed Oct 6, 2022
View reviewed changes
@harryterkelsen
Copy link
Contributor

harryterkelsen commented Oct 6, 2022

I just looked at the code more closely and saw that ShaderWarmUp.warmUpOnCanvas is called unconditionally. It is only picture.toImage on the resulting picture that is gated on !kIsWeb: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/painting/shader_warm_up.dart#L90

So what you should do in the test, in addition to what you have already, is set debugCaptureShaderWarmUpImage and verify that it is called with a valid, non-null image in the same way as you checked warmUpOnCanvas.

harryterkelsen
harryterkelsen approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@harryterkelsen harryterkelsen left a comment

Choose a reason for hiding this comment

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

LGTM

@alanwutang11 alanwutang11 added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 6, 2022
@auto-submit auto-submit bot merged commit 7462c4a into flutter:master Oct 6, 2022
This was referenced Oct 7, 2022
Merged
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 7, 2022
@engine-flutter-autoroll
7462c4a shader warm up with canvaskit and corresponding test (flutter…
3cde8b5 
…/flutter#113060)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Oct 7, 2022
@engine-flutter-autoroll
7462c4a shader warm up with canvaskit and corresponding test (flutter…
b5bfaa5 
…/flutter#113060)
@alanwutang11 alanwutang11 deleted the shader-warmup-with-canvasKit branch October 7, 2022 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@harryterkelsen harryterkelsen harryterkelsen approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[web] we need to warm-up CanvasKit renderer shaders

2 participants

@alanwutang11 @harryterkelsen