Ignore fully opaque alpha channels, preserve alpha when writing grayscale images - #98
Ignore fully opaque alpha channels, preserve alpha when writing grayscale images#98illwieckz wants to merge 9 commits into
Conversation
This allows opaque images to use formats without alpha channels when converting to CRN, DDS, or KTX. This matches the existing behavior when converting to PNG or TGA.
|
It looks like this was an overlook, as the tool has clear intention to do heuristics by default:
and to select the smallest format for the output file:
Tools like Urcheon had to implement some image pre-processing that did the same alpha channel opaque look-up and had to write the stripped image as a temporary file before calling |
|
Can be tested with this file as input (1×1 RGBA TGA image with opaque alpha channel). Before: After: |
|
And for control to make sure it didn't break transparent images, tested with this file as input (2×2 RGBA TGA image with color variation and non-opaque alpha channel): Before: After: |
cef756e to
59d948b
Compare
|
As explained, Urcheon already did the opaque alpha channel stripping, so this will not reduce Unvanquished game file size. But for other games using the |
224fde9 to
6b2df13
Compare
0ed6f52 to
df6a315
Compare
|
I added test samples to the test suite. |
d22f99b to
c6a83e8
Compare
|
The output from processing |
087ab9a to
ed46490
Compare
Good catch! I did a mistake. I did a 32-bit TGA image but without the alpha attribute saying the 4th byte is an alpha channel. Now all the images have the alpha attribute. I also committed the generator. Actually, this disclosed a feature request: 32-bit images should be converted to 32-bit images, even if the 4th channel isn't said to be alpha. In Crunch context, alpha means nothing. An file can store normal map in RGB and height map in alpha channel, that's not transparency information. So, every conversion from an input with 4 channels should produce an output with 4 channels, and if the output format only recognize RGBA, so it should be it. So we discovered the current Crunch drops the alpha channel when the TGA has a 4th channel but that 4th channel isn't flagged as alpha. I don't know yet if that's something to fix in Crunch or something to fix in |
675fa35 to
8344fd5
Compare
|
Hu, no, it looks like the image is still opaque when the color is white, but transparent when the color is red… |
Images marked as grayscale with a valid alpha component were converted to grayscale-only data when preparing them for output, causing the alpha channel to be discarded. Preserve the alpha channel by keeping grayscale images with alpha as grayscale+alpha data, while continuing to discard alpha for formats such as JPEG that do not support it.
608307a to
28241bf
Compare
e9ebec2 to
f87e7e3
Compare
|
I now implemented the RLE storage format variant in the test samples. That means there is now 45 new TGA samples to test, which, with the various TGA-to-something conversion, adds 315 tests to run… In the end are tested RGB/grayscale white, RGB red, RGB red+green+blue+white, with noalpha, opaque alpha, transparent alpha, and RLE/flat storage. When testing RLE, the RGB red+green+blue+white with all its noalpha/opaque/transparent variants are tested both with raw pixel packets and RLE pixel packets (1 contiguous pixel being the same, 2 contiguous pixels being the same). |
|
There exists some more obscure TGA formats, like paletted (both flat and RLE!), 16-bits RGBA5551, 16-bit RGB565, and storing the attribute bits in the descriptor byte. Those are more rare, what I have done is good for now. Also, all the bugs I found were in Crunch, where Crunch destroyed data properly decoded by |
|
Hmmm, we now have a problem. It happened that sometime, the MinGW CI running tests on Wine was randomly failing because of an error related to wine. But with those 315 more tests, we hit those random Wine errors everytime: |
b8a326e to
0070311
Compare
0070311 to
f11ab4c
Compare
|
It looks like I’ve found a way around it! |
015b942 to
9b2554c
Compare
|
Why are we testing on Wine and not actual Windows? |
We test the MinGW build on Wine and the MSVC build on Windows. |
|
So, my latest improvements is that the “red/green/blue/white” color image is now also tested as 8×8 RLE TGA image, giving 4×4 pixels to each quadrant, making possible to not only test RLE (because of repeated pixels) but to also make possible to visually check the output on JPEG, CRN, DDS and KTX thanks to the amount of pixels being high enough to properly code each of these colors in those lossy formats. And since the output is visually correct, those conversions are now added to the tests. Everything I wanted to cover in that PR is now covered. |
9b2554c to
e7f487b
Compare
| "grayscale8", | ||
| "rgb8", | ||
| ]: | ||
| for color_name, base_width in [ |
There was a problem hiding this comment.
How about color_value/alpha_value instead of color_name/alpha_name
There was a problem hiding this comment.
Value isn't meaningful given it can be different given the storage format. I would expect numbers or things like that if named value. In the end those “names” are some human readable strings that somewhat means something, so “name” looks good.
There was a problem hiding this comment.
I would expect numbers or things like that if named value.
They do correspond to numbers. For example opaque meaning an alpha of 1 and transparent meaning an alpha of 0. name is a poor name because most or all of things being looped over contribute to the file names, not just the RGBA contents. content could be another alternative to name/value.
There was a problem hiding this comment.
opaque also means no alpha channel at all. Here the names are what the people see: is it read, white, transparent or opaque, disregarding the storage. Also color is just for a given arbitrary colored sample with more than one color, thad could have been multicolor or rgbw. We need a differentiated name good enough to have unique file names in the end, but those name should be short.
If I implement paletted testing, I will keep the color name for the red+green+blue+white test image even if the value may be impossible to compare with RGB values. I will only rename color to something else if I face the need for another colored test (not just one channel or luminance) for a specific testing.
It's on purpose that I name both the storage (what I named the type) and the expected visual apparence (what I named the name). The name here is explicitly not about a technical implementation but about the visual apparence, a word we agree on to describe something we recognize, so a name.
| "alpha8", | ||
| "other8", | ||
| ]: | ||
| for alpha_name in [ |
There was a problem hiding this comment.
When alpha_type is None the same images are generated twice.
There was a problem hiding this comment.
Now taken care of. Also I renamed None as noalpha, meaning names are not implicit anymore:
sample-storage-flat-rgb8-noalpha-color-opaque-2x2.tga
Instead of:
sample-storage-flat-rgb8-color-2x2.tga
so the naming matches this pattern:
sample-storage-flat-rgb8-alpha8-color-opaque-2x2.tga
The things are now always named, the filename construction isn't conditional anymore.
| else: | ||
| clone_name = f"rgba_{color_name}_{width}_{in_format}_{out_format}" | ||
|
|
||
| mkdir(out_dir) |
There was a problem hiding this comment.
This could be outside the loop since there is only one directory.
There was a problem hiding this comment.
Right, at first it was f"build/test/{in_format}-to-{out_format}" so it had to be in the loop, but then since there was no collision I found it better to just write everything in a single directory. Can be moved before any loop indeed.
There was a problem hiding this comment.
The mkdir is now looped again in the inner loop because it now outputs files in different directories according to their output being opaque or not, and this depend on the output format being JPEG or not.
| image_format_list = [ | ||
| "jpg", | ||
| "tga", | ||
| "bmp", |
There was a problem hiding this comment.
I see opaque results for several white transparent BMP images, such as sample-storage-flat-grayscale8-alpha8-white-transparent-1x1.bmp
There was a problem hiding this comment.
Ah! I lately remembered that BMP also supported alpha (the kid in me wrongly believing that Microsoft Paint limitation is authoritative on BMP features is still alive 😅️), so it was one of the latest thing I added. I have to double-check that.
There was a problem hiding this comment.
BMP doesn't have L8A8 output, so we had to convert to R8G8B8A8 the grayscale transparent images. Now done.
| ]: | ||
| for alpha_type, has_alpha in [ | ||
| ("alpha8", True), | ||
| ("other8", False), |
There was a problem hiding this comment.
it's a 4th 8-bit channel, but without attribute saying it's for storing transparency. This is valid from the specification, but purpose (what to store there) is undefined (left to the user).
32-bit images should be converted to 32-bit images, even if the 4th channel isn't said to be alpha. In Crunch context, alpha means nothing. An file can store normal map in RGB and height map in alpha channel, that's not transparency information. So, every conversion from an input with 4 channels should produce an output with 4 channels, and if the output format only recognize RGBA, so it should be it.
#98 (comment)
Every other tools I tested considered that other 4th channel not flagged as alpha, to be alpha (i.e. storing it in alpha when converting).
Grayscale images with a valid alpha component are internally represented as A8L8. Expand them to RGBA8 when writing BMP files so the alpha channel is preserved.
a0d1540 to
7a75020
Compare
7a75020 to
e83d19d
Compare
…ent and actually write all jpg into tga-to-all-opaque
Example of errors that would randomly surface in some test runs when running them on wine in the Azure CI: 009c:err:rpc:I_RpcReceive we got fault packet with status 0x1c010003 wine client error:0: recvmsg: Connection reset by peer
e83d19d to
5b9e044
Compare
|
@slipher I made all the converted test files from this test suite being written to either Also it means we can actually consider JPEG output of transparent TGA to be clones of any output of opaque TGA. clones, in this test tool, designates files whose output should be exactly the same to the bit, whatever the input format or the conversion operation, for example: So we now not only validate that converting each TGA to JPG produces the same file, but that converting a transparent TGA to JPG produces the exact same file as converting an opaque TGA to JPG. |
Ignore fully opaque alpha channels (when loaded by
crn_mipmapped_texture.cpp).This allows opaque images to use formats without alpha channels when converting to CRN, DDS, or KTX.
This matches the existing behavior when converting to PNG or TGA.
Preserve alpha when writing grayscale images (when processed in
crn_image_utils.cpp).Images marked as grayscale with a valid alpha component were written
without alpha, causing transparent grayscale images to become opaque.
Preserve the alpha channel by writing such images as grayscale+alpha,
while continuing to discard alpha for output format without alpha channel
support like JPEG.