BinomialLLC / basis_universal

Basis Universal GPU Texture Codec
Apache License 2.0
2.7k stars 263 forks source link

Alpha best practices and/or documentation #249

Open kg opened 3 years ago

kg commented 3 years ago

I'm using the BC3 transcoder and feeding pngs with alpha channels into basisu and not getting the results I would expect or at least hope for, and I don't see much anywhere in the docs or github issues that describe what basis expects (if anything) out of its source images - i.e. do they need to be premultiplied, must they NOT be premultiplied - and what promises it makes about its output. For raw RGBA source images I can happily fix this up after loading them by just going in and mucking with the pixels but I can't really go in and do that with BC3 data, so I have to understand what the transcoder can do for me and whether there are knobs I can turn to make basisu.exe do the "right" thing at compile time when I feed in my source assets.

I was hoping perhaps there would be a flag to basisu that would premultiply the alpha, but it doesn't appear that exists. Should I be manually running a custom filter over all my PNGs that premultiplies them and then dumps out a new non-standard premultiplied PNG for basisu to consume? Is there a way to nudge the transcoder into doing what I want at runtime? I would also expect that getting this "right" will influence the quality of the mips that basisu generates when you ask it to generate mips.

kg commented 3 years ago

From testing, it appears that basisu implicitly premultiplies any images loaded from TGA, but not from PNG.

kg commented 3 years ago

From some more testing it's probably also worth explicitly stating how alpha is handled in the mipmap generation. I think the way it works right now produces suboptimal outcomes, but I'm also not sure basisu is in a position to do anything about it.

As I understand things:

PNG and TGA seemingly handling alpha in different ways means it's not possible for everything to be correct right now, but were that fixed I'm still not sure basisu would be able to magically do everything right. This has me considering doing my own complicated mip generation (based on the above) and then feeding those mips into basis to try and get the alpha correct everywhere.

kg commented 3 years ago

Here is an example of the problems that arise from not getting alpha right (this appears in DXT3, DXT5 and BC7 output from basis, and is present in both UASTC and classic ETC1S modes, while UASTC is better):

bc7-alpha

bc7-red

bc7-composite

If you look closely you can see there's dark haloing around the red because of the disconnect between the alpha and red values (not basis's fault, just the nature of the format), and in some cases there will also be bright halos from the disconnect being in the opposite direction. The expected "correct" output would be for there just to be appropriately-shaded red, but that would require the alpha value to match up with the implied alpha of the premultiplied red we're getting out of the texture unit, and it doesn't.

I think in this case the "fix" would be for me to compress without premultiplication applied, and then premultiply in my shader or in the ROPs - but right now I can't do that since the TGA loader premultiplies my textures. I could swap over to PNG input to avoid that, of course.

kg commented 3 years ago

Some more spam to give credit where due: the alpha problem with TGA was elsewhere in the chain, basis appears to treat TGA alpha the same as PNG alpha. So that's good, and it means the only remaining problem (other than documentation/best practices) is how to deal with mipmaps. It's possible the right answer is just to tell the user to generate their own mips and feed them to basisu, but I don't know how you feel about that. I think average or even above-average devs might struggle to get it right, but on the other hand, maybe mips for block-compressed textures with alpha are not especially common or important?

Left side below is unpremultiplied RGBA input -> basis uastc -> bc7 -> multiply alpha in GPU ROP, right side is premultiplied RGBA input -> basis uastc -> bc7. Naturally this does not show the known issues with mips

image

richgel999 commented 3 years ago

It's possible the right answer is just to tell the user to generate their own mips and feed them to basisu, but I don't know how you feel about that. I think average or even above-average devs might struggle to get it right, but on the other hand, maybe mips for block-compressed textures with alpha are not especially common or important?

Right now, you have to call our encoder directly to feed in your own generated mipmaps. We haven't exposed that to the command line interface yet, but it's on the list.

richgel999 commented 3 years ago

From testing, it appears that basisu implicitly premultiplies any images loaded from TGA, but not from PNG.

The .tga loader just loads whatever alpha data is in the file (see read_tga() in basisu_enc.cpp). We don't do anything special with the alpha data from .tga vs. png.

  • Ideally, your texture data being compressed isn't premultiplied, because the post-compression A and RGB channels might be mismatched, which would mess up the premultiplication.

The mipmap generator in basisu doesn't treat alpha in any special way (i.e. it doesn't premultiply or postmultiply) - it's just another channel of data. One challenge is, sometimes the alpha channel doesn't represent opacity, but something else special to the application.

I think adding support for premultiplying before resampling makes sense, and we'll get to this next year. Also, the compressor API already supports custom user-supplied mipmap level images, and we'll expose this to the command line tool.

kg commented 3 years ago

From testing, it appears that basisu implicitly premultiplies any images loaded from TGA, but not from PNG.

The .tga loader just loads whatever alpha data is in the file (see read_tga() in basisu_enc.cpp). We don't do anything special with the alpha data from .tga vs. png.

  • Ideally, your texture data being compressed isn't premultiplied, because the post-compression A and RGB channels might be mismatched, which would mess up the premultiplication.

The mipmap generator in basisu doesn't treat alpha in any special way (i.e. it doesn't premultiply or postmultiply) - it's just another channel of data. One challenge is, sometimes the alpha channel doesn't represent opacity, but something else special to the application.

Right, you definitely can't make it the default. It might make sense to try and add intelligence for this to the command line tool (perhaps an optional argument that describes how to treat the alpha when generating mips) but I can understand why you might see that as out of scope for basisu.exe. (Would you accept a PR to add it if I find time to implement that?)

I think adding support for premultiplying before resampling makes sense, and we'll get to this next year. Also, the compressor API already supports custom user-supplied mipmap level images, and we'll expose this to the command line tool.

Great! If I haven't gotten around to invoking the compressor API manually by then I'll definitely be happy to use that.

richgel999 commented 3 years ago

I was hoping perhaps there would be a flag to basisu that would premultiply the alpha, but it doesn't appear that exists. Should I be manually running a custom filter over all my PNGs that premultiplies them and then dumps out a new non-standard premultiplied PNG for basisu to consume?

Yea, we don't do anything special here. Alpha is another channel of information. In ETC1S, it'll be assigned another independent "slice" in the compressed file, and compressed separately vs. RGB. (For UASTC, any blocks that use non-255 alpha will be encoded using either LA or RGBA modes.) The transcoder will give you the alpha channel back if the GPU format you select supports it. If alpha=0, we don't assume RGB will always be 0, i.e. we don't assume the data is premultiplied (because it may not be).

If you want to experiment with premultiplying in the mipmap generator, the compressor does support an alternative resampler (stb_image_resize.h). See BASISU_USE_STB_IMAGE_RESIZE_FOR_MIPMAP_GEN and STBIR_FLAG_ALPHA_PREMULTIPLIED in basisu_comp.cpp. It's not enabled by default to avoid introducing more dependencies.

richgel999 commented 3 years ago

I think what you want for mipmaps is support for this - see "ALPHA CHANNEL": https://www.cs.unh.edu/~cs770/lwjgl-javadoc/lwjgl-stb/org/lwjgl/stb/STBImageResize.html

richgel999 commented 3 years ago

Would you accept a PR to add it if I find time to implement that?

Ideally, the easiest PR's to accept are very focused. So if you only modify the mipmap generator, that would be a relatively PR to review & test. The next major update (Dec/Jan timeframe) may be touching the mipmap code, but only to make the low-level code faster.