google / automotive-design-compose

Automotive Design for Compose is an extension to Jetpack Compose that allows every screen, component, and overlay of your Android App to be defined in Figma, and lets you see the latest changes to your Figma design in your app, immediately!
https://google.github.io/automotive-design-compose/
Apache License 2.0
101 stars 14 forks source link

Support premultiplied alpha in `fetch` tool. #1030

Open johnoneil opened 3 weeks ago

johnoneil commented 3 weeks ago

I'll run down the info that kicked this off below, but the bottom line is, our use of the Impeller renderer requires all textures we use to have pre-multiplied alpha.

So, to begin with: when we currently load some .png images from serialized design docs, some render incorrectly.

Note the "cloud" above the car in this image:

Screenshot from 2024-04-22 14-41-26

This image renders incorrectly because the PNG for this element packed into the serialized document file does not contain premultiplied alpha. It contains what's called "straight" or non-premultiplied alpha.

Strictly speaking that is correct because PNG files should not contain premultiplied alpha. That is addressed in the PNG spec. See here.

The question then arises, if our renderer requires premultiplied alpha, where should that pre-multiplication be applied?

We could apply this pre-multiplication after we unpack the PNGs from the serialized file (at runtime), but that would incur a runtime cost. We're already seeing some impact to start time from large image loading, so I'm wary of making that worse.

Still, applying the premultiplication to loaded images at runtime may be the correct approach.

But to obviate the need for a runtime cost, I think the proper fix is further up the pipeline. My current only thought is to do it in some way when the binary file is packed.

I would suggest we allow the fetch tool to have an additional command line parameter to allow packing PNG (or all images?) with premultiplied alpha applied. Or maybe convert them to a format which usually handles premultiplied alpha.

iamralpht commented 2 weeks ago

@johnoneil It looks like the PNG spec and the WebP spec both mandate non-premultiplied alpha. So I think DC is doing the right thing, and so long as we use PNGs we should honor the spec because we have multiple decoders (e.g.: the native Rust one and the Kotlin one... which uses the Android image decoder), so we don't get unexpected results if a DCF generated with one set of flags is used with a renderer that doesn't know how to interpret them. So I think so long as its PNGs, we need to fix our impeller binding to apply the premultiplication or see if there's a different impeller image format we need to use such that the sampler or impeller's shaders do the full multiplication.

But, I agree it's silly to spend a bunch of CPU time at startup multiplying u8s and dividing by 255, or to waste three FMAs in every shader that samples from an image. Perhaps we could include alternates for images (so only Rust impl knows to look at alternate image definitions) and look at compressed texture formats (maybe using Binomial Basis), which would give us the best startup time because we could skip the decode step and have less memory to copy to the GPU managed space.

johnoneil commented 2 weeks ago

Okay. I currently have a hack in place to eliminate the need for pre-multiplication, but when I next update our renderer, I will remove that hack and include code to pre-multiply image content where needed (I believe I can detect cases where raw image data is apparently not premultiplied). In the longer term I'd look at doing some post-processing on serialized figma files in our pipeline to provide alternate image as you suggest. I think this can be closed, but I'l leave this issue open until I take the first steps mentioned above.