Allow creating slimpeller engine variants.

[chinmaygarde]

Fixes flutter/flutter#144432

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[chinmaygarde]

Just adding a new target to start getting this wired up on CI (I'll probably need some help here). Doesn't actually enable any of the savings.

[matanlurey]

Nit, let's use a less cute name, like remove_skia_gpu_backend please.

[zanderso]

Guessing that this is going to be used for ifdef guards around the Skia GPU code. In that case, we might want to avoid double negatives (#ifndef REMOVE_SKIA_GPU) and instead call it include_skia_gpu so that it would be #ifdef INCLUDE_SKIA_GPU in the code.

[matanlurey]

Yeah seems reasonable, just something that isn't a codename or a double negative!

[chinmaygarde]

The dashboard tracks more than just removing Skia GPU with this effort (like removing the built-in codecs as well as the Skia software backend).

let's use a less cute name

Why? Did I miss a memo? To borrow your phrase, I am not taking suggestions for an alternate name at this time.

[matanlurey]

If you are making a joke and I don't get the reference (the link you provided is an internal link that probably should not be linked here in general) feel free to ignore.... but if not - this is team project, and I would not possibly be able to know that "slimpeller = true" means, for example, the built-in codecs change, and I suspect that goes the same for our gardeners and rollers.

If include_skia_gpu_and_built_in_codecs is not sufficient, then another idea is to break it into two flags (include_skia_gpu, include_skia_built_in_codecs).

tl;dr: Not trying to be persnickety, I'm trying to make future debugging easier.

[chinmaygarde]

The link I provided is obfuscated behind a UUID. So you can't read it if you aren't on the channel (you are). But, yeah, the context is that that we added DeathRattle as an alternative to fml::ScopedCleanupClosure. Thought it was cute (and less readable, made future debugging harder as you put it). Which is to say, I don't think there is a moratorium on using cute names in the codebase. So I reject the notion that we should stop introducing them now.

Back to the point though, I need one simple engine variant I can wire up on CI where the various action items listed on the dashboard can be implemented and their size binary size impact tracked. All items related to this effort are tracked on a dashboard and labelled. What the flag does is documented in GN file as well as the help string for ./flutter/tools/gn. Trying to figure out what slimpeller is supposed to be is trivial. include_skia_gpu_and_built_in_codecs is pedantic and as the effort continues to evolve and, say implements the removal of the codecs, becomes out of date. Hence the name.

I am not trying to do this to be cute. Need one variant I can put on CI that tracks an evolving effort (that does more than Skia GPU removal).

[matanlurey]

aside: I am perfectly fine with renaming DeathRattle (which is a test only appliance that cannot impact user code) - flutter/flutter#146105.

I strongly don't think we should be using code names like this. An engine variant does not have to be tied to a single flag (you can setup the variant and flip the flags however you want), that is, you can create a slimpeller variant that turns both the flags to false.

What I am asking for is to pick flags that accurately explain what something is doing, in so far that a gardener, sheriff, google3 roller, or even a client name downstream will be able to understand why something might have impacted or broke them. This is a very reasonable request, but I'm also happy to bring up at the weekly and get other opinions if comes to that.

[chinmaygarde]

Impeller is a codename, so is flow, so is Skia. I obviously feel less strongly about codenames. No Flutter user/developer cares about these. But these are already part of the lexicon of the sheriffs and gardeners (who are neither real sheriffs nor gardners).

[matanlurey]

Ok, request withdrawn.