[Impeller] Add offscreen texture checkerboarding #41199

bdero · 2023-04-14T08:27:01Z

Adds an offscreen texture checkerboarding feature with an overridable color picker. By default, it should look the same as our current checkerboarding for Skia. It can be turned on/off at any time while recording commands.

  Canvas canvas;

  canvas.debug_options.offscreen_texture_checkerboard = true;

  // ...draw something that uses offscreen textures...

For hooking this up with the framework, we could either snake a global setting through to the context, or sneak it in through the DL as, say, a scoped SaveLayer option.

Video of checkerboards getting rendered to offscreen textures only:

Screen.Recording.2023-04-14.at.12.29.55.AM.mov

New golden:

flar · 2023-04-14T09:22:23Z

I don't think it should be a property of a DL as you might want to render a DL with and without the effect. Making it a property of the Impeller and DlSkCanvas Dispatchers might be a better fit? DlSkCanvas might also want to support it for when we render directly to Skia.

jonahwilliams · 2023-04-14T16:55:14Z

impeller/entity/contents/content_context.h


 namespace impeller {

+using CheckerboardPipeline =


Can we conditionally include this shader in flutter runtime mode debug/profile mode only?

+1. If we have a tons of these, we could also put them in a debug folder.

chinmaygarde

Consider moving all the checkerboarding stuff into a separate debug options struct. Also, allowing the caller to control checkerboarding maybe something we don't want to do to make checkerboaring more useful in the future.

chinmaygarde · 2023-04-14T17:46:00Z

impeller/entity/contents/content_context.h


 namespace impeller {

+using CheckerboardPipeline =


+1. If we have a tons of these, we could also put them in a debug folder.

chinmaygarde · 2023-04-14T17:47:18Z

impeller/aiks/canvas.cc


-Canvas::Canvas() {
+Canvas::Canvas()
+    : checkerboard_color_proc_([]() {


This is perhaps too configurable? Can we just make up this pattern ourselves? Or make this an implementation detail?

I should clarify my thinking here. I was hoping to make the checkerboarding more useful by perhaps having nested save layers be rendered differently (via progressively darkening overlays maybe).

Making the color non-configurable/just based on depth SGTM, but what about textures at the same depth? Maybe we can kick that can down the road, but it might make sense to make the edges of checkerboarded textures look distinct by rendering a solid border a few pixels wide.

Also, should color remain consistent between frames?

It seems that hue should vary within a level, but darkness or opacity changes as you increase the level?

I made the checkerboards red with 0.25 alpha + get progressively darker based on depth. At level >= 5 it's just black. I can add a little space division identifier in a follow-up and hue shift based on it -- this kind of thing will be useful for other tooling as well, and will be deterministic for multithreaded encoding.

chinmaygarde · 2023-04-14T17:49:30Z

impeller/aiks/canvas.cc

  FML_DCHECK(base_pass_->GetSubpassesDepth() == 1u);
 }

+void Canvas::SetEnableOffscreenCheckerboard(bool enabled) {


We could have this and other debugging options separated into a debug options struct that can be assigned to the canvas.

Done. PTAL at the new DebugOptions struct.

jonahwilliams

LGTM!

chinmaygarde

LGTM.

I am not sure I followed Jims comment about this being a DL property. It doesn't seem to be.

flutter-dashboard · 2023-04-15T02:12:52Z

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #41199 at sha c754aef

bdero · 2023-04-15T02:48:04Z

My new debug define isn't present when running the goldens. Gonna hold out on merging this until I work it out later.

bdero · 2023-04-16T04:14:44Z

Oh, it's because the goldens run in release mode. I'll leave the test around, but this can't currently be verified with a golden.

flutter-dashboard · 2023-04-16T04:59:48Z

Golden file changes are available for triage from new commit, Click here to view.

Changes reported for pull request #41199 at sha 74dda19

…ngine#41199)

…124944) flutter/engine@da0805a...4a998b7 2023-04-16 [email protected] Roll Skia from cc3404330d3b to 03c8e529196d (1 revision) (flutter/engine#41255) 2023-04-16 [email protected] Roll Skia from 2d0b05335104 to cc3404330d3b (1 revision) (flutter/engine#41254) 2023-04-16 [email protected] Roll Fuchsia Linux SDK from hacWN-gQSoWudziIS... to 46T4ZTAt48yPRDEn5... (flutter/engine#41253) 2023-04-16 [email protected] Roll Fuchsia Mac SDK from gI2knJ4Rh0yoK4Syd... to AAx6fLy6ykWVhXvNh... (flutter/engine#41252) 2023-04-16 [email protected] [Impeller] Add offscreen texture checkerboarding (flutter/engine#41199) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from hacWN-gQSoWu to 46T4ZTAt48yP fuchsia/sdk/core/mac-amd64 from gI2knJ4Rh0yo to AAx6fLy6ykWV 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],[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

…lutter#124944) flutter/engine@da0805a...4a998b7 2023-04-16 [email protected] Roll Skia from cc3404330d3b to 03c8e529196d (1 revision) (flutter/engine#41255) 2023-04-16 [email protected] Roll Skia from 2d0b05335104 to cc3404330d3b (1 revision) (flutter/engine#41254) 2023-04-16 [email protected] Roll Fuchsia Linux SDK from hacWN-gQSoWudziIS... to 46T4ZTAt48yPRDEn5... (flutter/engine#41253) 2023-04-16 [email protected] Roll Fuchsia Mac SDK from gI2knJ4Rh0yoK4Syd... to AAx6fLy6ykWVhXvNh... (flutter/engine#41252) 2023-04-16 [email protected] [Impeller] Add offscreen texture checkerboarding (flutter/engine#41199) Also rolling transitive DEPS: fuchsia/sdk/core/linux-amd64 from hacWN-gQSoWu to 46T4ZTAt48yP fuchsia/sdk/core/mac-amd64 from gI2knJ4Rh0yo to AAx6fLy6ykWV 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],[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

bdero added the e: impeller label Apr 14, 2023

bdero requested a review from chinmaygarde April 14, 2023 08:27

bdero self-assigned this Apr 14, 2023

bdero force-pushed the bdero/checkerboard branch from bd97b1a to 9056d73 Compare April 14, 2023 08:31

jonahwilliams reviewed Apr 14, 2023

View reviewed changes

chinmaygarde reviewed Apr 14, 2023

View reviewed changes

bdero force-pushed the bdero/checkerboard branch 2 times, most recently from fb5b058 to 1919f03 Compare April 14, 2023 20:00

jonahwilliams approved these changes Apr 14, 2023

View reviewed changes

bdero requested a review from chinmaygarde April 14, 2023 20:53

chinmaygarde approved these changes Apr 14, 2023

View reviewed changes

bdero force-pushed the bdero/checkerboard branch 3 times, most recently from d7088c9 to c754aef Compare April 15, 2023 00:20

flutter-dashboard bot added the will affect goldens label Apr 15, 2023

bdero added 11 commits April 15, 2023 19:54

[Impeller] Add offscreen texture checkerboarding

9d44002

Add overridable color proc

801afc3

Add test

c3b29d8

Licenses

3a03d11

Don't render checkerboards on collapsed passes

ec58b1a

Licenses

b042b51

Add impeller-specific debug flag

ba34206

Debug only

7b79a40

Debug options struct, internal colors based on pass depth

06fc161

Malioc

49190e4

Revert accidental solid_fill change

74dda19

bdero force-pushed the bdero/checkerboard branch from c754aef to 0d4cff8 Compare April 16, 2023 03:10

bdero force-pushed the bdero/checkerboard branch 2 times, most recently from 3706424 to 74dda19 Compare April 16, 2023 04:06

bdero added the autosubmit Merge PR when tree becomes green via auto submit App label Apr 16, 2023

bdero merged commit a56b8f2 into flutter:main Apr 16, 2023

engine-flutter-autoroll mentioned this pull request Apr 16, 2023

Roll Flutter Engine from da0805a9cf03 to 4e6a895424c0 (4 revisions) flutter/flutter#124940

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2023

a56b8f228 [Impeller] Add offscreen texture checkerboarding (flutter/e…

51ff36a

…ngine#41199)

engine-flutter-autoroll mentioned this pull request Apr 16, 2023

Roll Flutter Engine from da0805a9cf03 to 4e6a895424c0 (4 revisions) flutter/flutter#124942

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2023

a56b8f228 [Impeller] Add offscreen texture checkerboarding (flutter/e…

a144adb

…ngine#41199)

engine-flutter-autoroll mentioned this pull request Apr 16, 2023

Roll Flutter Engine from da0805a9cf03 to 4a998b73a2df (5 revisions) flutter/flutter#124944

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 16, 2023

a56b8f228 [Impeller] Add offscreen texture checkerboarding (flutter/e…

bb57165

…ngine#41199)

chinmaygarde mentioned this pull request Apr 17, 2023

[Impeller] Enable debug checker-boarding options for offscreen rendered layers. flutter/flutter#124819

Closed

[Impeller] Add offscreen texture checkerboarding #41199

[Impeller] Add offscreen texture checkerboarding #41199

Uh oh!

Conversation

bdero commented Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flar commented Apr 14, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bdero Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bdero Apr 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahwilliams left a comment

Choose a reason for hiding this comment

Uh oh!

chinmaygarde left a comment

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Apr 15, 2023

Uh oh!

bdero commented Apr 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bdero commented Apr 16, 2023

Uh oh!

flutter-dashboard bot commented Apr 16, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

bdero commented Apr 14, 2023 •

edited

Loading

bdero Apr 14, 2023 •

edited

Loading

bdero Apr 14, 2023 •

edited

Loading

bdero commented Apr 15, 2023 •

edited

Loading