Add debug disable layer toggles to the Performance page

[kenzieschmoll]

Fixes #2778, in combination with flutter/flutter#91499

In this PR, I also consolidated all the debugging options into a dialog that is launched by clicking a button "Debugging Options". Looking for some feedback on this layout. Optionally, we can use switches instead of checkboxes.

[kenzieschmoll]

CC @InMatrix @cobblest for UX input on how to best expose these perf debugging features

[jacob314]

I worry "debugging options" isn't an obvious. @cobblest & @InMatrix, any ideas for an alternate name for the options list to make it more discoverable? We already have the problem that too people notice "track widget rebuilds."

[jacob314]

One idea is to leave track widget builds and the performance overlay as buttons and just leave these more advanced options in the advanced dialog.

[kenzieschmoll]

I worry "debugging options" isn't an obvious. @cobblest & @InMatrix, any ideas for an alternate name for the options list to make it more discoverable? We already have the problem that too people notice "track widget rebuilds."

One idea is to leave track widget builds and the performance overlay as buttons and just leave these more advanced options in the advanced dialog.

This is what that would look like. I worry that the controls section is looking pretty busy.

[cobblest]

1. For two layers of setting input, we can consider using "advanced setting" for more granular control. https://www.toptal.com/designers/ux/settings-ux
2. I agree that putting all options in the option list is less obvious for users, especially those important options. If we want to save some space, we could maybe remove the icons in the button?
3. I also noticed the last 3 checkbox are "Disable" options to "Ignore" some effects. To make it more consistent with other options and clearer to users, I am wondering if we could make them as "Enable" options. Unchecked box naturally means "disable" and we keep it unchecked by default if that's a typical setting for most users.

[ayanogawa]

Could you please help me clarify the copy used here?

1. Does widget tree = layer tree?

2. Does clip layers / clipping effects mean Clip Behavior?
   https://flutter.dev/docs/release/breaking-changes/clip-behavior

3. Does opacity layers mean Opacity class?
   https://api.flutter.dev/flutter/widgets/Opacity-class.html

4. Does physical shape layers mean PhysicalModel or PhysicalShape?
   https://api.flutter.dev/flutter/widgets/PhysicalModel-class.html
   https://api.flutter.dev/flutter/widgets/PhysicalShape-class.html

The language used in the mock doesn't seem to be consistent with flutter.dev/docs. I would suggest to keep them aligned so the users can have clear references. Thanks.

[kenzieschmoll]

@cobblest

I also noticed the last 3 checkbox are "Disable" options to "Ignore" some effects. To make it more consistent with other options and clearer to users, I am wondering if we could make them as "Enable" options. Unchecked box naturally means "disable" and we keep it unchecked by default if that's a typical setting for most users.

I am not completely opposed to changing these to "enable", though I will push back a bit with some thoughts from the developer perspective. The user action here is truly to disable a layer for debugging purposes only, which I think may be better communicated by having "disable" in the action title. If "enable" is in the title, we need to make sure we are communicating that each of these rendering layers are enabled by default with flutter, and that they are not settings that DevTools is manually enabling, or that a user can expect to turn off to improve performance in a release build. We do have some other areas in DevTools where the setting action is phrased in the negative, like "hiding" certain classes or code in memory snapshots and CPU profiles, so this is not completely inconsistent with other DevTools settings.

@ayanogawa
You can read what each of these toggles do in rendering/debug.dart from the Flutter framework, originally added in flutter/flutter#15548.

Looping in someone from the framework to fact check me on these answers: @goderbauer

Does widget tree = layer tree?

From my understanding, no. The layer tree is generated during paint.

Does clip layers / clipping effects mean Clip Behavior?

Yes. Anything that uses a ClipRectLayer, ClipRRectLayer, or a ClipPathLayer.

Does opacity layers mean Opacity class?

This applies to anything with an OpacityLayer. The Opacity widget is one widget that uses an OpacityLayer for rendering, but there are also others. A list could be put together by tracing the code references in the framework, but there are several places where OpacityLayer is used.

Does physical shape layers mean PhysicalModel or PhysicalShape?

Yes. This applies to anything that uses a PhysicalModelLayer, which include RenderPhysicalModel and RenderPhysicalShape.

I do have a concern with using the terminology "filter" as these options will not filter data that is currently visible in the performance page, but rather will affect how Flutter is rendering your app. To see a difference after turning on one of these debug toggles, the user will have to interact with their app again to reproduce the activity they want to profile. Communicating this required "reproducing" behavior has been a sticking point with Track Widget Builds as well, since users do not know that they have to reproduce activity in their app to get widget build trace events to show up.

[cobblest]

In general UX principles, checkbox should be used with positive phrasing. For example, use do instead of do not, and notify instead of do not notify:
https://docs.microsoft.com/en-us/windows/win32/uxguide/text-ui#control-labels
https://ux.stackexchange.com/questions/5748/checkbox-label-negating

The reason is that a checked checkbox means "Yes", and users would need to wrap their head around the negative phrase "Disabled" to decide if the actual state is "on" or "off". I guess that's why I got confused the first time I saw the design.

[ayanogawa]

@kenzieschmoll
Thank you for the helpful context. I love the links you provided. Could we add them to the modal like the mock below? (Link color: Google Blue 300 - #8AB4F8)

What about "Trace events" at header?

[kenzieschmoll]

The problem with linking to Layers is that users typically don't interact with the layer tree. The user will understand the context of the widget tree, but not necessarily all the layers flutter uses behind the scenes during paint.

I don't think "Trace Events" is the right title here, as these debugging options don't all affect trace events directly. "Debugging options" is probably a more accurate title. Here's a summary of what each option does:

Performance Overlay: overlays a performance chart on your app (docs)
Track Widget Builds: adds an event to the timeline for every widget built
Ignore clip layers: ignore clipping effects while painting.
Ignore opacity layers: ignore opacity effects while painting
Ignore physical modeling layers: ignore physical modeling effects while painting

The effects for the last three debugging options (the new ones added by this PR) will likely show up to the user as faster frame times (smaller bars in top chart on performance page) or fewer or shorter raster thread timeline events for a flutter frame.

[kenzieschmoll]

After addressing review comments, here is the current state of the UI in this PR:

Performance Overlay and Track Widget Build were placed back into the top level menu bar. The layer debugging options are in a dialog that is opened by clicking the "More debugging options" button:

Debugging options dialog:

Tooltips for each debugging option:

[goderbauer]

Looping in someone from the framework to fact check me on these answers: @goderbauer

The answers to those questions LGTM.

[jacob314]

nit: should this be the default everywhere? Seems a little odd to have some of our tooltips have different padding.

[kenzieschmoll]

The default padding is only a problem when the tooltip breaks to two lines. Gave DevToolsTooltip the same default padding as Tooltip to ensure the vertical padding is set as it should be.

[jacob314]