Corruption on VRAM-compressed texture mipmaps (S3TC compression)

[SaracenOne]

Godot version

eb318d3

System information

Windows 10, Vulkan, Nvidia GTX 1080 (471.11)

Issue description

It appears that the mipmaps of textures imported as VRAM Compressed can appear with odd skewing distortion.

The cube in the centre is using a texture with VRAM compression, while the other cubes are using the other four options. This issue does not appear to affect all textures, but it seems consistently broken on the example texture provided.

Steps to reproduce

Simply open the example project and observe the middle cube with VRAM Compressed version of the texture at an appropriate distance to see the mipmaps.

Minimal reproduction project

mipmaps.zip

[SaracenOne]

I don't know exactly how long this has been an issue, but it seems to have been around for a while at least.

[SaracenOne]

A bit more follow-up info, the main reason this particular texture seems to fail is due to it being non-power-of-two. I've also been informed that this particular issue is likely specific to S3TC compression.

[lawnjelly]

It is probably just that the texture compression works in blocks (e.g. 4x4), so you will often get compression artifacts across these borders in the base level or mipmaps. As you say, using a power of 2 texture size makes them less likely.

[lyuma]

The khronos OpenGL wiki claims the following:

S3TC is a block-based format. The image is broken up into 4x4 blocks. For non-power-of-two images that aren't a multiple of 4 in size, the other colors of the 4x4 block are taken to be black. Each 4x4 block is independent of any other, so it can be decompressed independently.

From what I can tell, on OpenGL, there seems to be no hard requirement about dimensions of S3TC compressed textures being powers of two or multiples of four... However, apparently DX11 and WebGL impose some limitations to prevent it [except for 2x2 and 1x1 mipmaps since those are special]. I can't find any info on Vulkan, however.

The problem Godot has is it also allows mipmaps for non-power of two textures... which means, even if LOD 0 is a multiple of 4, some of the lower resolution LODs might be odd dimensions. I don't know if there is a good way around this contradiction, other than either ignore the DX11 requirement, or block VRAM Compression for any non-power of two texture with mipmaps enabled?

Just to see what other engines do in these cases, interestingly, Unity completely doesn't allow mipmaps for non-power of two textures (it defaults to resizing to nearest power-of-two when mipmaps are enabled), and it also does not allow compression for non-multiple-of-4... so they went the easy route of just not allowing this case.

Regardless of what is compatible with ANGLE and DX11, it seems fair to me that Godot shouldn't produce incorrect results or crash on non-multiple-of-4 S3TC compression. So while Godot might want to consider applying stricter rules, there's still a bug here that should be fixable, I would think. See also: #48509

[GerbilSoft]

I tested importing a 958x552 image in both Godot 3 and 4 to compare the results.
Source image: https://tcrf.net/images/f/fc/SCU-TEST_RR_areaMap-bg.png

Godot 3: Creates a DXT1 format texture, looks correct.
Godot 4: Creates a DXT5 format texture, has skewed output and broken colors.

It seems Godot 4's DXTn encoder isn't padding the last tiles. (958 should be padded out to 960)

================================

Godot 3.3 has DXTn compression in module_compress_squish.cpp:

godot/modules/squish/image_compress_squish.cpp

Lines 187 to 197 in 9d0e0fe

	for (int i = 0; i <= mm_count; i++) {
	int bw = w % 4 != 0 ? w + (4 - w % 4) : w;
	int bh = h % 4 != 0 ? h + (4 - h % 4) : h;
	int src_ofs = p_image->get_mipmap_offset(i);
	squish::CompressImage(&rb[src_ofs], w, h, &wb[dst_ofs], squish_comp);
	dst_ofs += (MAX(4, bw) * MAX(4, bh)) >> shift;
	w = MAX(w / 2, 1);
	h = MAX(h / 2, 1);
	}

which calls out to the third-party squish module for the actual compression:

godot/thirdparty/squish/squish.cpp

Lines 181 to 223 in 9d0e0fe

	void CompressImage( u8 const* rgba, int width, int height, int pitch, void* blocks, int flags, float* metric )
	{
	// fix any bad flags
	flags = FixFlags( flags );
	// loop over blocks
	#ifdef SQUISH_USE_OPENMP
	# pragma omp parallel for
	#endif
	for( int y = 0; y < height; y += 4 )
	{
	// initialise the block output
	u8* targetBlock = reinterpret_cast< u8* >( blocks );
	int bytesPerBlock = ( ( flags & ( kDxt1 | kBc4 ) ) != 0 ) ? 8 : 16;
	targetBlock += ( (y / 4) * ( (width + 3) / 4) ) * bytesPerBlock;
	for( int x = 0; x < width; x += 4 )
	{
	// build the 4x4 block of pixels
	u8 sourceRgba[16*4];
	u8* targetPixel = sourceRgba;
	int mask = 0;
	for( int py = 0; py < 4; ++py )
	{
	for( int px = 0; px < 4; ++px )
	{
	// get the source pixel in the image
	int sx = x + px;
	int sy = y + py;
	// enable if we're in the image
	if( sx < width && sy < height )
	{
	// copy the rgba value
	u8 const* sourcePixel = rgba + pitch*sy + 4*sx;
	CopyRGBA(sourcePixel, targetPixel, flags);
	// enable this pixel
	mask |= ( 1 << ( 4*py + px ) );
	}
	// advance to the next pixel
	targetPixel += 4;
	}

Squish is given the original unaligned width/height, not a padded width/height, and it pads the last 4x4 tile correctly.

================================

Godot 4 has DXTn compression in image_compress_etcpak.cpp:

godot/modules/etcpak/image_compress_etcpak.cpp

Lines 148 to 178 in e5c6c77

	for (int i = 0; i < mip_count + 1; i++) {
	// Get write mip metrics for target image.
	int mip_w, mip_h;
	int mip_ofs = Image::get_image_mipmap_offset_and_dimensions(width, height, target_format, i, mip_w, mip_h);
	// Ensure that mip offset is a multiple of 8 (etcpak expects uint64_t pointer).
	ERR_FAIL_COND(mip_ofs % 8 != 0);
	uint64_t *dest_mip_write = (uint64_t *)&dest_write[mip_ofs];
	// Block size. Align stride to multiple of 4 (RGBA8).
	mip_w = (mip_w + 3) & ~3;
	mip_h = (mip_h + 3) & ~3;
	const uint32_t blocks = mip_w * mip_h / 16;
	// Get mip data from source image for reading.
	int src_mip_ofs = r_img->get_mipmap_offset(i);
	const uint32_t *src_mip_read = (const uint32_t *)&src_read[src_mip_ofs];
	if (p_compresstype == EtcpakType::ETCPAK_TYPE_ETC1) {
	CompressEtc1RgbDither(src_mip_read, dest_mip_write, blocks, mip_w);
	} else if (p_compresstype == EtcpakType::ETCPAK_TYPE_ETC2 || p_compresstype == EtcpakType::ETCPAK_TYPE_ETC2_RA_AS_RG) {
	CompressEtc2Rgb(src_mip_read, dest_mip_write, blocks, mip_w);
	} else if (p_compresstype == EtcpakType::ETCPAK_TYPE_ETC2_ALPHA) {
	CompressEtc2Rgba(src_mip_read, dest_mip_write, blocks, mip_w);
	} else if (p_compresstype == EtcpakType::ETCPAK_TYPE_DXT1) {
	CompressDxt1Dither(src_mip_read, dest_mip_write, blocks, mip_w);
	} else if (p_compresstype == EtcpakType::ETCPAK_TYPE_DXT5 || p_compresstype == EtcpakType::ETCPAK_TYPE_DXT5_RA_AS_RG) {
	CompressDxt5(src_mip_read, dest_mip_write, blocks, mip_w);
	} else {
	ERR_FAIL_MSG("Invalid or unsupported Etcpak compression format.");
	}
	}

_compress_etcpak() ends up calling either CompressDxt1Dither() or CompressDxt5() with a mipmap size value that's been aligned to 4 pixels. However, this is telling the compression function that the original image is (in this case) 960 pixels, but it's actually 958 pixels. This is why the image ends up broken.

[Zylann]

I also noticed transparent textures with VRAM compression completely fail to render mipmaps, when it worked in Godot 3:

Is this the same issue or should it be a different one?

[clayjohn]

Just to see what other engines do in these cases, interestingly, Unity completely doesn't allow mipmaps for non-power of two textures (it defaults to resizing to nearest power-of-two when mipmaps are enabled), and it also does not allow compression for non-multiple-of-4... so they went the easy route of just not allowing this case.

Regardless of what is compatible with ANGLE and DX11, it seems fair to me that Godot shouldn't produce incorrect results or crash on non-multiple-of-4 S3TC compression. So while Godot might want to consider applying stricter rules, there's still a bug here that should be fixable, I would think. See also: #48509

It looks like Unreal doesn't even let you use mipmaps if your texture is non power of two https://docs.unrealengine.com/4.27/en-US/RenderingAndGraphics/Textures/Importing/ (also tested using the texture from OP)

From the Unity docs: https://docs.unity3d.com/560/Documentation/Manual/ImportingTextures.html

It is possible to use NPOT (non-power of two) Texture sizes with Unity; however, NPOT Texture sizes generally take slightly more memory and might be slower for the GPU to sample, so it’s better for performance to use power of two sizes whenever you can. If the platform or GPU does not support NPOT Texture sizes, Unity scales and pads the Texture up to the next power of two size. This process uses more memory and makes loading slower (especially on older mobile devices). In general, you should only use NPOT sizes for GUI purposes.

So I agree, we porbably need to be stricter about texture dimensions as well. In the short term, we should give the user an appropriate error and disable mipmaps. In the medium run, we should have a nicer UX that allows users to automatically pad nonpot textures

[lyuma]

Regarding being strict, I agree that it's better to fail the import tell the user to fix their art so they aren't surprised when it's blurry (due to scaling by a few pixels. I like that Unity lets you choose how to deal with power-of-two. Currently, in Godot, you can use the "Max Size" setting but that resizes in both dimensions, and it applies a separate resize from the power-of-two one, effectively causing two resizes, so it might not be optimal (since both width and height might need to be resized differently).

About your comment, two things to note:

• Before etcpak was merged, Godot actually enforced the same rules as Unity. However, it did it by automatically scaling up to the nearest power-of-two if mipmaps enabled, and scaling to the nearest multiple-of-four if not (which probably causes aliasing)

My draft PR at #46931 only tries to restore this old behavior.

• Even if you are strict about dimensions, there will still be mipmap levels for 2x2 and 1x1 (or Nx1 if not square). These are allowed out of necessity, but because they still use integer blocks, it expects to compress a 4x4 block, which means that we need to pass a padded buffer.

The old "modules/etc" library took in width&height in pixels, so I think it somehow handled this case, while "etcpak" is more strict. Once I add the code to scale Nx2 and Nx1 buffers to multiple of 4, I'll submit that PR.