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

Z values stored in many normals textures are incorrect #2368

Open
erich666 opened this issue Mar 13, 2024 · 12 comments
Open

Z values stored in many normals textures are incorrect #2368

erich666 opened this issue Mar 13, 2024 · 12 comments

Comments

@erich666
Copy link

As noted in #2223:

Spec provides range mappings in the Section 3.9.3:

After dequantization, texel values MUST be mapped as follows: red [0.0 .. 1.0] to X [-1 .. 1], green [0.0 .. 1.0] to Y [-1 .. 1], blue (0.5 .. 1.0] maps to Z (0 .. 1]. Normal textures SHOULD NOT contain blue values less than or equal to 0.5.

Incorrect normal textures in the sample models repo should definitely be fixed.


I wrote an analysis and correction program for normals textures: https://github.com/erich666/NormalTextureProcessor - it's still being tested and added to, but I'm confident that it is properly detecting and correcting bad RGB normals, by fixing the B value to give a length-one normal. On that repo's page you will see two comparisons of before and after correction that show models from this repository (see here, scroll down a page).

Attached is my analysis, and the program's log file when it analyzed the PNG files.
notes.txt
gltf_samples_analysis.log

I actually have a whole set of corrected normals textures ready to submit as a PR, but I then notice this repo says "This repository has been archived by the owner on Dec 22, 2023. It is now read-only." Also, my PR would fix just the PNG images in need of work; the .glb files that include the old, faulty normals textures would not be fixed - that would need additional effort by someone (I'm happy to do it, but need to know the process).

@erich666
Copy link
Author

erich666 commented Mar 13, 2024

To give a sense of the problem, here's a heatmap view of the normals textures I've analyzed. Anywhere you see bright green is a texture that has unnormalized Z values, which should not happen in a well-formed normals texture. This usually occurs either because the creator didn’t save Z (or put something else in the blue channel, like ambient occlusion), or incorrectly saved Z as going from 0 to 1 for 0-255, or used a bad formula, or who knows what else. Darker green are Z's that are slightly wrong.

Command line to generate these using my NormalTextureProcessor: NormalTextureProcessor.exe -odir output_heatmap -oall -oheatmap -izne

heatmap

@javagl
Copy link
Contributor

javagl commented Mar 13, 2024

Just a short remark:

I actually have a whole set of corrected normals textures ready to submit as a PR, but I then notice this repo says "This repository has been archived by the owner on Dec 22, 2023. It is now read-only."

The repo has essentially been moved to https://github.com/KhronosGroup/glTF-Sample-Assets (with some updates for the CI, directory structure etc).

The GLB files should be fixed along with the (external) PNG textures. There are some libraries that could make this easy, but it may be pretty invasive (many large changed files, and the potential for accidentally changing other aspects of the models), so some thought should be put into how to tackle that.

@scurest
Copy link

scurest commented Mar 14, 2024

Does the validator check that blues in normal maps are > 0.5?

@javagl
Copy link
Contributor

javagl commented Mar 14, 2024

It does not.

It would be nice if it could check that (even though the statement in the spec is only a 'SHOULD NOT', and not a 'MUST NOT'). But ... there are some obvious caveats with validating the contents of image files. The validator does some validation there (e.g. checking for the presence of color profiles in PNG and whatnot), but for validations that refer to pixel colors, I could imagine that...

  1. claiming to be able to validate every image format could be bold (there are KTX, WEBP and AVIF, and other extensions)
  2. depending on the compression, the images may be a bit noisy...
  3. It could be really, really, really slow....

@donmccurdy
Copy link
Contributor

I'd be open to doing any of the following in glTF Transform (https://gltf-transform.dev/):

  • add a check for invalid normal textures
  • add an operator to fix invalid normal textures
  • create a batch script to reprocess glTF-Sample-Assets normal textures, including GLBs

This would handle JPG, PNG, AVIF, and WEBP. KTX2 textures would need to be regenerated from an uncompressed source.

But as @javagl mentions above, the implications of reprocessing the entire folder with a single tool are murky. The changes that a glTF Transform round-trip would apply are minimal, but non-zero. For example, vertex buffer layouts are repacked in one of two configurations. Having everything in glTF-Sample-Assets be the output of a single tool would reduce the usefulness of the repository as a testing suite... 😕

@javagl
Copy link
Contributor

javagl commented Mar 14, 2024

It could be worthwhile to look at the generator of the assets. I could imagine that many of them are actually generated by the Blender IO, and then, this issue should be propagated to an issue in the glTF-Blender-IO repo, to ensure there (if possible) that only valid normal textures are exported. (In the future - whether we should really try to re-process the models is questionable)

@donmccurdy
Copy link
Contributor

I suspect that pixel-by-pixel validation of normal maps in the Blender exporter would face similar challenges, unfortunately.

@javagl
Copy link
Contributor

javagl commented Mar 14, 2024

I don't have an idea about the inner workings of the Blender exporter, but would have thought that the exporter itself does not have to "validate" anything (in the strictest sense). I'd hope that it has some sort of internal representation of a normal map, and that it might be possible to fix this with some normalMap.channel('B').mapFrom(0.0, 1.0).to(0.5, 1.0).beforeExporting(), but ... yeah, there are many 🤞's involved in that guess.

@erich666
Copy link
Author

Thanks for discussing this problem.

The GLB files should be fixed along with the (external) PNG textures. There are some libraries that could make this easy, but it may be pretty invasive (many large changed files, and the potential for accidentally changing other aspects of the models), so some thought should be put into how to tackle that.

Please let me know how I can help. As a start, I've made a PR that gives the corrected normal maps for the PNGs: KhronosGroup/glTF-Sample-Assets#112

@erich666
Copy link
Author

erich666 commented Mar 15, 2024

It could be worthwhile to look at the generator of the assets. I could imagine that many of them are actually generated by the Blender IO, and then, this issue should be propagated to an issue in the glTF-Blender-IO repo

Good idea, I'm guessing their polygon -> normals baking code has potential problems. That said, I've never looked at Blender's code so it'd take me a good while to get up to speed.

But, ultimately that's my goal: get the tools producing normals textures to work properly. For example, this tool can be off as much as four texel levels. My guess is GIMP's similar tool is imperfect, too. If there are other (free) tools that generate normals textures, let me know - I want to test them.

@erich666
Copy link
Author

I've decided to withdraw my PR and let my normals analyzer bake a bit more. Analyzing some results, I found it was being too picky about Z values, so I switched to testing normals lengths. Anyway, for now, here's the "where images have problems" image for those files flagged by my analyzer as stuff to be checked. Anything green is where the normal stored is not close to what it should be, darker green being pretty close.
image

@erich666
Copy link
Author

New PR here.

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

No branches or pull requests

4 participants