Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate complete mipmap chains for compressed DDS images #86360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LunaticInAHat
Copy link
Contributor

This PR adjusts the DDS image loader, to ensure that the mipmap chain of compressed images is complete. This aligns with expectations elsewhere throughout the engine, that images with mipmaps will be mipmapped all the way down to 1x1, and will not have partial mipmap chains that terminate at some higher level.

Some DDS exporters create images whose mipmap chains terminate above the 1x1 case (often 4x4 or 8x8). To create the missing mipmap levels, this PR repeatedly copies the first block of compressed data from the last mipmap level that exists in the DDS. This is not mathematically correct, however the hope is that it will create mipmaps with colors that are similar enough to correct, as to not be visually objectionable, given that these low mipmap levels will only be occupying a few pixels on the screen.

The "correct" solution would require decoding the image, resampling it, re-encoding into a compressed form, and copying the result back into the original image data. I felt it was best not to incur that performance & complexity overhead on image loading, unless and until complaints are received about last-level mipmaps of DDS textures looking wrong. The limited color palette available in DXT-compressed pixel blocks means that low-level mipmaps tend not to look much like the original image, anyways.

This PR does not adjust the handling of compressed images which did not have any mipmaps populated in the source file; those images will be left un-mipmapped, as I believe the remainder of the engine is prepared to accept them in that state.

This PR resolves #86330

@LunaticInAHat LunaticInAHat requested a review from a team as a code owner December 20, 2023 14:56
@AThousandShips AThousandShips added this to the 4.3 milestone Dec 20, 2023
@fire
Copy link
Member

fire commented Dec 20, 2023

@lyuma and a few others debugged this in the etcpak. I'll try to find the details

Edited:

See #56931

@LunaticInAHat
Copy link
Contributor Author

See #56931

Thanks for the reference. I think I have avoided the particular pitfall that the referenced PR was fixing. If I am reading it right, it was mainly concerned with ensuring that compressed image data generated by Godot was correctly formed from complete BC/DXT blocks. The implementation being fixed in that PR generated last-level mipmaps that occupied less than a full block, which resulted in garbage when sampled by the GPU. The corrected implementation from that PR ensured that even the very small mip levels generated full DXT blocks.

Similar to the corrected implementation of that PR, my PR here also generates complete blocks of BC/DXT data for the low-level mipmaps. So, they should sample correctly. They are incorrect in the sense that the colors encoded in them are not the colors that would be present in the best possible case, however they are correctly formed.

The other PR appears to have had access to the uncompressed color data of the image, so when generating the low-level mipmaps, they "smear" those colors across the 4x4 block that then goes into image encoding and pops out as BC/DXT data. The DDS loader does not have access to the uncompressed color data; we only have the compressed representation handy. So, we can't replicate the resampling technique from the other PR, unless we wanted to actually decompress the image first.

As a concrete example of how my implementation compares with a best-possible resampling of the image data: Pretend you have an image of arbitrary size with Red in the top left, Green top right, Blue bottom left, Yellow bottom right. There is a smooth gradient across the image, blending between the colors.

The best-possible 4x4 mipmap of that image will have at least two of the right colors in the right corners. The other two corners (and most of the middle pixels) will all contain incorrect colors. Exactly how incorrect the image is will depend upon how smart the encoder is when choosing which colors to encode explicitly; there are only two explicitly-specified colors in BC/DXT blocks, and all other colors referencable by the block are linearly interpolated between them. So, you can get two corners correct, and you can probably even get a band of the gradient right, but the rest is all forced to be incorrect garbage. If you chose to encode Red and Green, then there is no interpolation that gets you Blue or (full-bright) Yellow for the other corners. The 2x2 mipmap is essentially the same story; two corners are correct, the other two are wrong. The 1x1 mipmap can at least be precisely generated -- it will be some kind of grey, because that's the average of those colors.

The incorrect 4x4 mipmap that will be generated by this PR is just a duplication of the 4x4 top-left corner of the image. So, it gets the Red corner right, but everything else is varying shades of wrong. The 2x2 mipmap will be the 2x2 top-left corner of the image, and the 1x1 mipmap will just be the top-left pixel (i.e., red)

My contention is that this is probably an acceptable compromise, because most 3D game art doesn't have strongly opposing colors across it, and BC/DXT does a bad job of dealing with data like that (at low resolution), even in the best case. If complaints are received, we can reconsider and decide it's worth decompressing the image, in order to resample it, but for now it will be a big improvement just to be able to import these images at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DDS with implicit first mipmap size fails to import
4 participants