Skip to content

What's the state of the libpng path? #616

Open
@darksylinc

Description

@darksylinc

Hi!

I started using this project which basically reimplements texconv and texassemble CLI interface for all platforms while relying on DirectXTex to perform most of the operations.

Because there is no WIC on Linux, it relies on libpng and libjpeg for supporting these formats on non-Windows platforms.

However yesterday I found the hard way that the libpng path is horribly broken:

Textures without alpha are assigned an alpha of 0

This is what png_read_rows() is returning. This would be fine except that DirectXTexPNG.cpp sets the format to either DXGI_FORMAT_R8G8B8A8_UNORM or DXGI_FORMAT_R8G8B8A8_UNORM_SRGB, which indicates the alpha is valid.

Textures are converted to linear space in 8-bit

This is awful. An input texture of 256x1 with pixels set in ascending order "0 1 2 3 4 ... 255" end up being quantized as "0 0 0 0 0 16 16 16 .... 255".

This happens because of this code:

png_set_alpha_mode(st, PNG_ALPHA_STANDARD, PNG_DEFAULT_sRGB);

Which tells libpng to do this awful conversion. Here's a text file to see what's I'm talking about:

Input:
Image

Output (-f R8G8B8A8_UNORM):
0to255.dds.zip

Output (-f R8G8B8A8_UNORM_SRGB):
0to255.dds.zip

The fix is to perform the following:

	// Request libpng the alpha change, but keep RGB untouched.
	png_set_alpha_mode(st, PNG_ALPHA_STANDARD, PNG_GAMMA_LINEAR);
	
	// Deal with custom color profiles
	if( png_get_valid( st, info, PNG_INFO_gAMA ) )
	{
		double gamma = 0;
		double screen_gamma = 2.2;

		if( png_get_gAMA( st, info, &gamma ) )
		{
			// If gamma == 1.0, then the data is internally linear.
			if( abs( gamma - 1.0 ) > 1e6 )
				png_set_gamma( st, screen_gamma, gamma );
		}
	}

This change also requires another change here:

DXGI_FORMAT GuessFormat() const noexcept(false)
{
	// 1 or 4. 1 is for gray
	auto c = png_get_channels(st, info);
	if (c == 1)
	{
		if (png_get_bit_depth(st, info) == 16)
			return DXGI_FORMAT_R16_UNORM;
		// with `png_set_expand_gray_1_2_4_to_8`, libpng will change to R8_UNORM
		return DXGI_FORMAT_R8_UNORM;
	}

	// 8 or 16. expanded if 1, 2, 4
	auto d = png_get_bit_depth(st, info);
	if (d == 16)
		return DXGI_FORMAT_R16G16B16A16_UNORM;
	if (d != 8)
		throw std::runtime_error{ "unexpected info from libpng" };

	int intent = 0;
	if (png_get_sRGB(st, info, &intent) == PNG_INFO_sRGB)
		return DXGI_FORMAT_R8G8B8A8_UNORM_SRGB;
	
	// There is no intent declared, but 99% PNGs out there are sRGB except for those with explicit Gamma
	if( png_get_valid( st, info, PNG_INFO_gAMA ) )
	{
		double gamma = 0;
		if( png_get_gAMA( st, info, &gamma ) )
		{
			// This PNG is explicitly linear.
			if( abs( gamma - 1.0 ) <= 1e6 )
				return DXGI_FORMAT_R8G8B8A8_UNORM;
		}
	}

	return DXGI_FORMAT_R8G8B8A8_UNORM_SRGB;
}

In this fix, it assumes that most PNGs are authored for sRGB content (it'd probably need support for --ignore-srgb to override this behavior). However I suspect this change of mine could make texconv behavior differ from the WIC path for the same inputs.

We know for certain that if gamma is 1.0 / 2.2 then the PNG is explicitly in sRGB, even intent did not say PNG_INFO_sRGB.

Here's an input example of a PNG that contains a gamma of 1.0 / 2.2 due to an embedded color profile:

Image

Conclusion

As things state, it looks like the libpng path is currently broken and should not be used. And if it were to be used, my fixes could work but still would need a lot of testing.

I don't know if texconv can use WIC inside of WSL; or if it relies on libpng. If it does, and Microsoft intends to support this path, it needs maintenance.

Cheers

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugwslRelated to Windows Subsystem for Linux support

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions