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

normalmap compressed with crunch -dxn are loaded with reverted B channel #183

Closed
illwieckz opened this issue Mar 17, 2019 · 5 comments
Closed

Comments

@illwieckz
Copy link
Member

illwieckz commented Mar 17, 2019

This bug was uncovered in #180.

All our normal maps are compressed with crunch -dxn and our engine loads them with reverted blue channel (Z component). This was hidden by the bug in the normalmap code that was loading the Z component from alpha channel instead of blue one. Hopefully we did not ship any heightmap in alpha channel and since crunch -dxn drops the alpha channel in any way, it looks like the engine was just using an empty Z component). The normalmap loads correctly if loaded from another format (like webp).

With r_normalMapping disabled:

dpoffsetmapping

With r_normalMapping enabled and normal map compressed with crunch -dxn:

dpoffsetmapping

With r_normalMapping enabled and normal map compressed with crunch -dxn and Z component reverted in engine to workaround the bug:

dpoffsetmapping

With r_normalMapping enabled and normal map compressed with webp:

dpoffsetmapping

The source tree in #180 is meant to be correct. I created a test map with assets I created to be sure they are the way they must be, then fixed the engine to render the test map. The correct engine in this tree is able to render correctly xonotic maps and unvanquished maps if their normal maps were not compressed with crunch -dxn.

We need someone to fix the normalmap loading when they are compressed with dxn (use the branch in #180 branch as a base as master is not correct).

The symptom is a map being almost entirely dark. No need for special map to reproduce the bug, just load released chasm with this branch (or any other map):

dpoffsetmapping

It may be possible that the whole normalmap is reverted, reverted red (X) and reverted green (Y) leads to subtle changes while reverted blue (Z) just break everything.

I'll upload the test map soon but testing with released maps is enough to reproduce the bug. The test map will be useful to check that the bug is entirely fixed. While it's very easy to check for the correct loading of the blue channel with any released map, it may be very difficult and time-consuming to check for the correct loading of red and green channel without the test map.

@illwieckz illwieckz changed the title normalmap compressed with crunch -dxtn are loaded with reverted B channel normalmap compressed with crunch -dxn are loaded with reverted B channel Mar 17, 2019
@illwieckz
Copy link
Member Author

illwieckz commented Mar 17, 2019

Is it related to DaemonEngine/crunch@902f075 ?

This crunch commit talks about things “that breaks reconstruction of the z part in DXN compressed normal maps”. And we have an issue with z part.

@gimhael, does that ring a bell to you?

The bizarre thing is that we used -dxn -renormalize -rtopmip to crunch our normal maps, -renormalize being told to be the thing to do.

@cmf028
Copy link
Contributor

cmf028 commented Mar 17, 2019

The current code for reading normal maps comes from here: https://therealdblack.wordpress.com/2010/06/26/texture-tools-and-normal-maps/

It references: http://realtimecollisiondetection.net/blog/?p=28

This is done so that all of the possible normal map formats can be loaded without additional shader permutations.

Here is the existing code with comments for reference:

vec3 N = texture2D(u_NormalMap, texNormal.st).xyw;

// multiply red with alpha
N.x *= N.z;

// convert the range [0,1] to [-1,1]
N.xy = 2.0 * N.xy - 1.0;

// compute z
// assumes z is positive (it is for tangent space normal maps)
// assumes the read vector components are from a normalized vector (this isn't the case because of texture filtering, but doesn't matter much in practice).
N.z = sqrt(1.0 - dot(N.xy, N.xy));

Some details on the formats:
Normal maps are assumed to contain normalized tangent-space normals that point to somewhere on the positive hemisphere of the surface (so the Z component of the normal is always positive). This is the common standard in every videogame out there. Because of this, we can drop the Z component, and recalculate it with z = sqrt(1 - xx + yy). Every video game engine I know of has done this for over 15 years.

Here are the normal map formats we support:

Uncompressed RGB(A) texture
Assumes R = (x + 1.0)/2.0, G = (y+1.0)/2.0, and B = (z+1.0)/2.0. A is always 1.0.
RGB textures will always read "1.0" for the alpha texture according to the opengl spec.
With the existing function, you get

N.x = r;
N.y = g

// don't change anything
N.x *= 1.0

// convert to [-1, 1]
N.xy = 2.0 * N.xy - 1.0;

// compute z
z = sqrt(1.0 - N.x * N.x - N.y  * N.y);

This is correct for the assumptions made.

DXT1/BC1 compressed texture
Assumes R = (x+1.0)/2.0, G =(y+1.0)/2.0, B = const, A = 1.0

This format obviously works with the z recalculation from before.

DXT5/BC3 compressed texture (DXT5nm)
Assumes R = 1.0, G = (y+1.0)/2.0, B = const, A = (x+1.0)/2.0,

x is stored in the alpha channel because the alpha channel is compressed separately from rgb channels in this format. This weird layout greatly enhances the quality for compressed normal maps. See this nvidia post for a comparison.

With the existing function, you get the following when simplified:

N.x = 1.0;
N.y = g;

// set N.x to alpha
N.x *= a;

// convert to [-1, 1]
N.xy = 2.0 * N.xy - 1.0;

// compute z
z = sqrt(1.0 - N.x * N.x - N.y  * N.y);

So the same shader code gives the correct result.

BC5/3Dc/Red Green compressed texture
This texture format is what modern games use for their compressed normal maps.
It offers higher quality than even the unique BC3/DXT5 formulation.
Assumes R = (x+1.0)/2.0, G = (y+1.0)/2.0
There are no B or A components, so B is loaded as 0, and A is loaded as 1.0 according to the opengl spec.

See the RGB(A) calculation for the proof that it works.

With OpenGL 3.3+ or the ARB_texture_swizzle extension, you could use texture swizzle masks to avoid the extra multiply with alpha, and similarly avoid extra shader permutations.

@illwieckz
Copy link
Member Author

illwieckz commented Mar 17, 2019

Thanks @cmf028 for that awesome piece of knowledge! And thanks to be there :D.

I guess it also explains why normal maps compressed with crunch -dxn get their alpha channel dropped: the alpha channel can't be kept since it will be abused to store some data belonging to RGB.

It would be very good to create a wiki zone for such engine design choice, something titled like “Engine design and behavior” and sub-articles to knowledge like this one or this one.

So it looks like we have a collision with “heightmap in normal map alpha channel” feature and “normal map compression using alpha channel” hack. This is calling for a proper support of lose heightmaps.

This is basically the formats we would have to support:

  • normal map in RGB format
  • normal map in RGB format with height map in A channel
  • normal map in hacky DXT format abusing the alpha channel
  • heightmap in lose file whatever the format (crn seems to compress them very well)

And the features:

  • normal map with embedded height map
  • normal map plus lose height map
  • height map generating normal map

The later is sometime called bump map but dæmon uses both normalMap and bumpMap keywords for normal maps with optional embedded alpha channel.

Note that to easily add the support of these feature a revamp of stage collapsing is required first because without a revamp the number of conditional blocks to code to support N stages is something around N×(N+1)×(2×N+1)÷6 (1²+2²+...+N²), see #184.

@illwieckz
Copy link
Member Author

Note that just for knowledge about the alpha channel DXT trick, I noticed that an heightmap in alpha channel of a PNG file with a pure blank RGB stream can compress smaller than an heightmap in a PNG grayscale stream. This even after being optimized by multiple optimisers like optipng and recompressed with pngwolf-zopfli.

@illwieckz
Copy link
Member Author

Fixed in PR that was breaking it.

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

No branches or pull requests

2 participants