Traverse-Research / ispc-downsampler

Image downsampler using a Lanczos filter implemented in ISPC
Other
11 stars 1 forks source link

📏 Add renormalization based on pixel format to Lanczos kernel #48

Closed KYovchevski closed 6 months ago

KYovchevski commented 8 months ago

When downsampling normal maps we need to renormalize the values to get a correct normal map. Unfortunately, we can't turn it into a separate pass unless we split the entire ISPCprogram up, since the program does two passes, one vertical and one horizontal, and we should be normalizing after both of them for complete correctness. As a result, the renormalization was made a step of the writing process for the pixels, which is only ran for Snorm formats. To accommodate this, Rgba8Snorm and Rgb8Snorm formats were added, while previous formats were renamed to *Unorm. The formats were also reflected to the ISPC code so that we can make decisions in the program and not rely on having many more different functions that the Rust side needs to call correctly.

KYovchevski commented 8 months ago
  1. Unorm vs snorm is not the right nomenclature for this. They just indicate value ranges, not intended use
  2. We shouldn't lanczos filter our normal maps as discussed offline. Lanczos filters can result in negative values even if all it's inputs are positive, resulting in incorrect normals (for regular images would result perceived sharpness).

I've implemented a different path that can be taken by the user which applies a box filter, as discussed offline. This means that the format is not connected to the way the downsampling is done, and instead the user must explicitly say that it is a normal map.

  1. This doesn't appear to take into account the type of normal map, and so most likely it's incorrect reconstructing the normals. Notice that we use tangent space normals for which we reconstruct one of the components.

I am not sure why non-tangent space normals would require a special treatment. At lower mips there will be noticeable mistakes in the normal map, but I believe that would be due to the fact that we only have a few pixels to represent any direction for the normals.

  1. It appears this removes srgb support as well? We're we ignoring that during downsampling? If so, we probably need a follow up also to adresss this issue, for more info see http://www.ericbrasseur.org/gamma.html?i=1

I don't think there was anything for SRGB textures before this, unless something was lost with #15 getting merged. I can investigate this and if that is the case create a separate PR about this afterwards. I don't think it belongs in this PR.

Jasper-Bekkers commented 8 months ago

I am not sure why non-tangent space normals would require a special treatment. At lower mips there will be noticeable mistakes in the normal map, but I believe that would be due to the fact that we only have a few pixels to represent any direction for the normals.

You'd need to decode the values, normalize them, and encode them again in the proper format.

For example for our tangent space normals, we store them in R & G and then use the following routine to decode them;

        float3 normalTs = float3(material.unpackNormalTsXy(sampleData) * 2.0 - 1.0, 0.0);
        normalTs.z = sqrt(max(0.01, 1.0 - dot(normalTs.xy, normalTs.xy)));

If you'd just take the texture data's rgb value and normalize that you wouldn't end up with the correct normal in the first place.

KYovchevski commented 7 months ago

I am not sure why non-tangent space normals would require a special treatment. At lower mips there will be noticeable mistakes in the normal map, but I believe that would be due to the fact that we only have a few pixels to represent any direction for the normals.

You'd need to decode the values, normalize them, and encode them again in the proper format.

For example for our tangent space normals, we store them in R & G and then use the following routine to decode them;

        float3 normalTs = float3(material.unpackNormalTsXy(sampleData) * 2.0 - 1.0, 0.0);
        normalTs.z = sqrt(max(0.01, 1.0 - dot(normalTs.xy, normalTs.xy)));

If you'd just take the texture data's rgb value and normalize that you wouldn't end up with the correct normal in the first place.

Ok, this makes sense now. I will add something so that it can be indicated whether the normal needs to be decoded, however I would like to point out that our framework does not downsample R8g8 formatted normal maps. Looking at the code, it always does the R8g8b8 -> R8g8 conversion after the downsampling, and it does that for each mip level.