BufferAttribute: Add generic vector "component" getter / setter

[gkjohnson]

Related issue: #--

Description

When writing functions that process arbitrary buffer attributes it's common to need code that can handle a buffer attribute storing components of arbitrary size. However there's currently no function for reading x, y, z, or w based on component index. This leads to writing for loops with conditions to branch and use getX, getY, getZ, etc which is far more verbose.

Some examples of current workarounds are here, here, here, and here.

All these cases would be much cleaner and more understandable if I could just write

for ( let i = 0; i < attr.count; i ++ ) {

  for ( let k = 0; k < attr.itemSize; k ++ ) {

    let value = ...;
    attr.setComponent( i, k, value );

  }

}

instead of something like

for ( let i = 0; i < attr.count; i ++ ) {

  for ( let k = 0; k < attr.itemSize; k ++ ) {

    if ( k === 0 ) attr.setX( i, ... );
    if ( k === 1 ) attr.setY( i, ... );
    if ( k === 2 ) attr.setZ( i, ... );
    if ( k === 3 ) attr.setW( i, ... );

  }

}

cc @donmccurdy

[donmccurdy]

I would be glad if we could make setX+setY[+setZ[+setW]] a bit more concise, yes. Do you think it's possible to collapse the inner loop? E.g. explicitly setting a single item¹ by array value. For example:

for ( let i = 0, el = []; i < attr.count; i += 2 ) {

  // VectorN → Array → BufferAttribute
  attr.setItem( i, vector.toArray( el ) );

  // BufferAttribute → Array → BufferAttribute
  attr.setItem( i, attr2.getItem( i, el ) );

}

Arguably the .set( array, offset ) API can do this, but because would be doing double-duty if we interpret it both as a full array setter and a single-item setter (see questions in #24512 and #24511 about normalization).

¹ Alternatively, 'element' → setElement/getElement?

[LeviPesin]

Please also change current usages of if ( k === 0 ) attr.setX( i, ... ); if ( k === 1 ) attr.setY( i, ... ); if ( k === 2 ) attr.setZ( i, ... ); if ( k === 3 ) attr.setW( i, ... ) in the three.js to the new function.

[gkjohnson]

I would be glad if we could make setX+setY[+setZ[+setW]] a bit more concise, yes. Do you think it's possible to collapse the inner loop? E.g. explicitly setting a single item1 by array value. For example:

We could - in my code I'd probably keep the two loops in some cases though because I find it more readable. It makes more sense to me to think of the elements in the array as vectors and access individual components.

Arguably the .set( array, offset ) API can do this, but because would be doing double-duty if we interpret it both as a full array setter and a single-item setter (see questions in #24512 and #24511 about normalization).

These can be fairly hot functions in tight loops so keeping the implementations as simple as possible for performance is important imo.

1 Alternatively, 'element' → setElement/getElement?

If we change to a setter that just sets the individual elements rather than taking an index / component then this name change makes sense.

cc @mrdoob @Mugen87 any thoughts?

[donmccurdy]

Let's wait on this and #24512 for r145 if that's ok?

[gkjohnson]

Any updated thoughts on this?

[donmccurdy]

I'm OK with merging this after it's rebased. 👍🏻

[Mugen87]

Me too!

[github-actions]

📦 Bundle size

Full ESM build, minified and gzipped.

Filesize dev	Filesize PR	Diff
645.2 kB (159.9 kB)	645.4 kB (160 kB)	+203 B

🌳 Bundle size after tree-shaking

Minimal build including a renderer, camera, empty scene, and dependencies.

Filesize dev	Filesize PR	Diff
438.4 kB (106 kB)	438.6 kB (106.1 kB)	+203 B

[gkjohnson]

Great! Just updated.

Also updating the docs is becoming quite inconvenient and error prone now that there are 6 pages that need to be updated for new changes. I wonder if there's any way we could shift this so only the english docs are updated and other language maintainers can look at a diff of some kind.

[donmccurdy]

Also updating the docs is becoming quite inconvenient and error prone now that there are 6 pages that need to be updated for new changes...

My thoughts: #24984 (comment)