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

BufferAttribute getters #5729

Closed
rmarques-realstatus opened this issue Dec 5, 2014 · 8 comments · Fixed by #5735
Closed

BufferAttribute getters #5729

rmarques-realstatus opened this issue Dec 5, 2014 · 8 comments · Fixed by #5735

Comments

@rmarques-realstatus
Copy link

Hi guys,

Is there a way of getting the xyz coords stored in a BufferAttribute without having to do something like:

var myObjectIndex = 2;
var positions = someObject.geometry.getAttribute('position');
var myPosition =  new THREE.Vector3(positions.array[myObjectIndex*3], positions.array[myObjectIndex*3 + 1], positions.array[myObjectIndex*3 + 2],);

This is kinda verbose... =(

There's already setXYZ(...), and more, that work with itemSize. Equivalent getters would be awesome =)

The resulting code would be something like:

var myObjectIndex = 2;
var positions = someObject.geometry.getAttribute('position');
var myPosition = positions.getXYZ(myObjectIndex);

Thank you!

@mrdoob
Copy link
Owner

mrdoob commented Dec 5, 2014

What should it return? Object, Array, or Vector3?

@rmarques-realstatus
Copy link
Author

Perhaps an Array of length itemSize?

Assuming we are working with positions (which is what the getters would be nice for), we could have

var myVector = new THREE.Vector3().fromArray(positions.getXYZ(myObjectIndex));

or

var objectPos = positions.getXYZ(myObjectIndex);
var myVector = new THREE.Vector3().fromArray(objectPos);

Vector3 could also gain a constructor that receives an Array of size 3 (if we really want to reduce verbosity) =P

EDIT: Overall this would make it easier to combine Raycasting and BufferGeometry since intersectObjects returns an Array of objects each with an index. But that index is transformed, and because of that we have to multiply it by itemSize to access the respective value in a BufferGeometry. However, we can use it as is when calling setXYZ(...)

@WestLangley
Copy link
Collaborator

What should it return? Object, Array, or Vector3?

I am not a big fan of using new behind the back of the user. One solution we have used in the past is the pass in an optionalTarget.

But perhaps we can do something much simpler... Something like this:

var myObjectIndex = 2;

var positions = someObject.geometry.getAttribute( 'position' );

var myPosition = new THREE.Vector3().fromAttribute( positions, myObjectIndex );

We can define the required new methods using the following pattern:

THREE.Vector3.prototype.fromAttribute = function ( attribute, index ) {

    index *= attribute.itemSize;

    this.x = attribute.array[ index ];
    this.y = attribute.array[ index + 1 ];
    this.z = attribute.array[ index + 2 ];

    return this;

};

@rmarques-realstatus
Copy link
Author

After reading your comment I went to read the source code and found that that snippet is very similar to the implementation approach of

setXYZ: function ( index, x, y, z )

My question now is: Why bother multiplying the index by itemSize if we're just gonna return 3 components? Is it expected to have a user use setXYZ and hypothetical getXYZ outside of a itemSize == 3 scenario?

Retrospectively, the same situation happens with setXYZW.

@WestLangley
Copy link
Collaborator

Why bother multiplying the index by itemSize if we're just gonna return 3 components?

A user can create a BufferAttribute having item.size = 17. The proposed fromAttribute() method would populate a Vector3 with the first 3 elements at the specified index location.

BTW, we could add an optional offset argument to the method:

THREE.Vector3.prototype.fromAttribute = function ( attribute, index, offset ) {

    if ( offset === undefined ) offset = 0;

    index = index * attribute.itemSize + offset;

    this.x = attribute.array[ index ];
    this.y = attribute.array[ index + 1 ];
    this.z = attribute.array[ index + 2 ];

    return this;

};

@rmarques-realstatus
Copy link
Author

Fair enough.

I like the optional parameter, though =)

@mrdoob
Copy link
Owner

mrdoob commented Dec 5, 2014

Looks good to me! 👍

@dubejf
Copy link
Contributor

dubejf commented Jul 1, 2015

Fixed by #5735.

@dubejf dubejf closed this as completed Jul 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants