Skip to content

Add support for using BC5 and EAC_RG compressed textures as normal maps. #31695

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.

Sign up for GitHub

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

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from
+80 −3
Open

Add support for using BC5 and EAC_RG compressed textures as normal maps. #31695

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6f87f91
Add NormalMapNode projectZ attribute.
castano Aug 20, 2025
0426e41
Add unpack normal helpers to Packing.js
castano Aug 25, 2025
6e7478a
Add normal packing mode constants to constants.js
castano Aug 25, 2025
1a4e77f
Import normal packing mode constants.
castano Aug 25, 2025
dca8fc4
Rename projectZ to unpackNormal.
castano Aug 25, 2025
31195ee
Material sets unpackNormal attribute based on normal map user data at…
castano Aug 25, 2025
3d843f6
Fix lint errors.
castano Aug 25, 2025
5413f13
Fix linter error.
castano Aug 25, 2025
344bec8
Replace unpackNormal functions with reconstructZ.
castano Sep 9, 2025
2e9e502
Merge branch 'dev' into normalmap-projectz
castano Sep 9, 2025
0e25686
Apply suggested renaming.
castano Oct 3, 2025
cdcabdf
Rename unpackNormalZ and update invocation.
castano Oct 3, 2025
407e673
Or better, keep unpackNormal function and rename attribute.
castano Oct 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/nodes/accessors/MaterialNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,12 @@ class MaterialNode extends Node {
node = normalMap( this.getTexture( 'normal' ), this.getCache( 'normalScale', 'vec2' ) );
node.normalMapType = material.normalMapType;

if ( material.normalMap.projectZ ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not rely on monkey-patched code here.

How about you introduce the property in the CompressedTexture class? Or is it also relevant for Texture?

Alternatively, move the property into the userData field of normalMap for now.

Copy link
Collaborator

@Mugen87 Mugen87 Aug 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When thinking about this: Why not checking for specific RG texture formats here and only then set the node property to true? In this way we need no additional texture property and it would not be necessary to update loaders.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is most useful in CompressedTexture, but in my particular application I'm using this with ExternalTexture and it can also be useful with uncompressed textures, to reduce their size by storing only the RG components.

I agree this should be done in some other way. A potential problem with this PR is that the attribute won't be cloned with the texture and that may cause issues. Adding it to the userData field as you propose would solve that problem.

I'm not sure enabling the Z reconstruction is the right thing to do for all RG texture formats. RG textures can be used for other purposes in addition to normal maps (heat distortion maps, 2d vector fields, chrominance values, etc). It may not cause any harm to just compute the Z component this way, but this won't extend to ASTC normal maps if we ever want to support that.

Copy link
Author

@castano castano Aug 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went with the userData approach. Let me know if that looks good to you.

Copy link
Collaborator

@Mugen87 Mugen87 Aug 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure enabling the Z reconstruction is the right thing to do for all RG texture formats.

My idea was to only do this for specific map types. So for a normal map, a format like bc5-rg-snorm (or SIGNED_RED_GREEN_RGTC2_Format) must hold packed RG data, otherwise it would be unusable for a normal map. This is something that could be implemented in the node classes so loaders don't have to worry about this detail.

But it's true a separate property allows more flexibility than deriving from texture formats.

It would be interesting to hear the opinion of others about this PR. Would you be okay if we label this PR as a "Draft PR" for now? I have the feeling we need to discuss the API a bit more in the team/community before making a decision.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicit unpack mode allows supporting ASTC LA normal maps or DXT5nm and in these cases it's not possible to infer the packing mode from the format alone.

@donmccurdy I'll ping you directly since you have done so much work in context of texture compression.

Similar to Texture.colorSpace, what do you think about introducing a new property Texture.packing that describes the packing strategy of a texture. The default would be NoPacking (as a pendant to NoColorSpace).

Packing strategies usually apply on certain texture types so probably most of the new constants for Texture.packing would be quite specific (like RGNormalPacking or GANormalPacking). But that is a similar situation like with Texture.colorSpace where certain constants only apply to color textures.

So...what do you think about this approach? Do you have maybe alternatives in mind?

Copy link
Collaborator

@donmccurdy donmccurdy Aug 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't be opposed to adding a Texture#packing property, I think @bhouston had proposed something similar once? A few other thoughts:

  1. If we see texture.format = THREE.RGFormat (or similar) assigned to a normal map slot, possibly that is enough in this case?

  2. In the newer TSL/NodeMaterial APIs with WebGPURenderer, is a property on THREE.Texture sure to be accessible? Or could the Texture instance have been wrapped in other nodes, obscuring the underlying Texture? If so, we could handle this by a new node to handle the unpacking, or a material property similar to material.normalMapType, instead.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I've been looking at the possibility of enabling the RGNormalPacking mode automatically based on the texture format. It could be something along these lines:

if ( material.normalMap.format == THREE.RGFormat || material.normalMap.format == RED_GREEN_RGTC2_Format ) {
	node.unpackNormal = NormalRGPacking;
}

Let me know if it's worth adding this. The default behavior could still be overridden with the user data attribute: normalMap.userData.unpackNormal.

Note, this only handles RG textures and BC5. I think there's no identifier for EAC_RG11 in three.js. If support for it is added in the future, this could be handled here as well.

We could also add support for signed RG textures and eliminate the 2*x-1 unpacking in the normal map node, but barely anybody uses the signed formats, so I think it's not worth it.

I'm ambivalent about adding a Texture#packing property. I can experiment with that if you think that's the way to go.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect checking .format would make sense for WebGLRenderer but not for TSL in WebGPURenderer. Since the focus of this PR is WebGPURenderer, we can skip that for now, but I wouldn't be opposed to adding this support in WebGLRenderer later.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should work with either renderer. The main caveats are that it requires the format attribute to be set (which may not be the case if it's an external texture), and it does not support EAC_RG, since that format is not natively supported in three.js.

Happy to include it for now if you think it makes sense to pave the way in that direction.


node.projectZ = true;

}

} else if ( material.bumpMap ) {

node = bumpMap( this.getTexture( 'bump' ).r, this.getFloat( 'bumpScale' ) );
Expand Down
19 changes: 17 additions & 2 deletions src/nodes/display/NormalMapNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import TempNode from '../core/TempNode.js';

import { normalView, transformNormalToView } from '../accessors/Normal.js';
import { TBNViewMatrix } from '../accessors/AccessorsUtils.js';
import { nodeProxy, vec3 } from '../tsl/TSLBase.js';
import { nodeProxy, vec3, float } from '../tsl/TSLBase.js';
import { dot, sqrt, saturate } from '../math/MathNode.js';

import { TangentSpaceNormalMap, ObjectSpaceNormalMap } from '../../constants.js';
import { directionToFaceDirection } from './FrontFacingNode.js';
Expand Down Expand Up @@ -57,14 +58,28 @@ class NormalMapNode extends TempNode {
*/
this.normalMapType = TangentSpaceNormalMap;

/**
* Controls whether the Z component of the normal map is reconstructed projecting it to the hemisphere.
*
* @type {bool}
* @default false
*/
this.projectZ = false;

}

setup( { material } ) {

const { normalMapType, scaleNode } = this;
const { normalMapType, scaleNode, projectZ } = this;

let normalMap = this.node.mul( 2.0 ).sub( 1.0 );

if ( projectZ ) {

normalMap = vec3( normalMap.xy, sqrt( saturate( float( 1.0 ).sub( dot( normalMap.xy, normalMap.xy ) ) ) ) );

}

if ( scaleNode !== null ) {

let scale = scaleNode;
Expand Down
Loading