BufferAttribute: Make normalize() and denormalize() internal functions

[donmccurdy]

Related:

• BufferAttribute: Support (de)normalization in accessors. #22874
• BufferGeometryUtils: Add MikkTSpace version of .computeTangents() #23716

As pointed out by @WestLangley, vertex attribute "normalization" refers to the int-to-float conversion done by the graphics API when reading integer values in a vertex shader and converting them to floats. In MathUtils.normalize() we currently mean the opposite — converting floats to integers that will be stored in a "normalized" vertex attribute.

If we use the term as the graphics APIs do, then moving data from a type = f32, normalized = false attribute to a type = u8, normalized = true attribute, you would actually call MathUtils.denormalize. That will probably be confusing, so I've replaced them with more explicit intToFloat() and floatToInt() methods here.

This PR reverses the naming, and makes both functions internal APIs of BufferAttribute to limit confusion here. I've also added a type argument to both functions (UnsignedByteType, FloatType, ...) instead of passing the array. This requires the addition of a getArrayType(array) function but feels cleaner, happy to go either way on this.

The original methods were added in r139, so we don't necessarily need to rename them in the same release as #22874 if we need more time.

/cc @WestLangley @gkjohnson

[donmccurdy]

Hm, on further discussion... @gkjohnson do you think it would be OK if we pulled these methods inside of BufferAttribute as private helpers, rather than exporting them from three as part of MathUtils? Do you need to use them directly to support normalized attributes in three-mesh-bvh and elsewhere, or just use the setters/getters on BufferAttribute?

[gkjohnson]

No right now I don't need them for three-mesh-bvh or any other projects at the moment.

[gkjohnson]

What's the value of this field?

[Mugen87]

Is it possible to avoid this property and just evaluate the typed array in normalize()/denormalize()?

[donmccurdy]

I think passing the entire array into normalize/denormalize as a huge 'type' argument was never ideal as a public MathUtils API... if this went back into MathUtils at some point, I think it is better to use types like IntType / ShortType. But since this is just internal, I suppose it does not matter so much. Any preference between:

• (A) limit visibility of the property, e.g. __type or #type
• (B) continue passing the array

/cc @WestLangley

[gkjohnson]

I prefer B for now until there's a practical reason to change it. It makes it hard to see what the other meaningful changes are in this PR, as well

[WestLangley]

TBH, I prefer any form of (A). Passing the array is horrible, IMO.

[gkjohnson]

imo passing a three.js enum can result in unnecessarily verbose user code with no value. If something like "A" is used I'd prefer to pass in something like the array type / constructor: normalize( value, Int8Array ).

A lot of the code I've been working with has been written to generally support array types within three. For example packing buffers into textures for three-gpu-pathtracer, generating data structures in three-mesh-bvh, etc. A lot of the code in three.js isn't designed to support these types of use cases which is fine but it would be nice to keep it in mind when adding new functionality to the system - these use cases have been the driver for #24515.

[donmccurdy]

What if we keep these functions accessible through THREE.MathUtils, but rename them much more explicitly, and export getArrayType(array) as well. Then usage might look something like:

import { MathUtils } from 'three';

const i = 127;
const type = MathUtils.getArrayType( Uint8Array );

const f = MathUtils.decodeNormalizedInt( i, type );
const i2 = MathUtils.encodeNormalizedInt( f, type );

I think this naming might be less likely to confuse people, as it's a more explicit about the use of integers as an encoding of floating-point values, per the OpenGL definition:

A Normalized Integer is an integer which is used to store a decimal floating point number...

[gkjohnson]

I'm still not fully understanding the need for a separate set of "type" constants. I guess I'm just not understanding the need to redefine constants that the language already has built in with the typed array constructors. It just feels a bit roundabout and redundant - similar to the Float32BufferAttribute, Uint8BufferAttribute, etc classes. But I won't fight it if it's what the project wants.

[donmccurdy]

My preference would be the API below...

import { MathUtils } from 'three';

const i = 127;
const type = MathUtils.getArrayType( Uint8Array );

const f = MathUtils.decodeNormalizedInt( i, type );
const i2 = MathUtils.encodeNormalizedInt( f, type );

But there are disagreements on the change, and I don't feel strongly enough either way to push for a consensus right now. Closing the PR!