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

BoundingBoxHelper for buffer geometry does not use drawrange neither groups #8690

Open
3 of 12 tasks
tonnot opened this issue Apr 20, 2016 · 7 comments
Open
3 of 12 tasks

Comments

@tonnot
Copy link

tonnot commented Apr 20, 2016

Description of the problem

BoundingBoxHelper.update (setFromObject) for buffer geometry does not use drawrange.count ( neither groups), so the limits can include points with 0,0,0 (now it uses positions.length)

if you have the typedarray with data still with 0,0,0, the box dimension figured out is not correct.(0,0,0 is included...) (I have predimensioned positions array and filling as I need)

I'm using by now to create pointclouds but I imagine some problems in the future if the bufferGeometry is used to drawlines or triangles (using groups)

So I this this is a mix of bug & enhance ,?
...

Three.js version
  • Dev
  • r76
  • ...
Browser
  • All of them
  • Chrome
  • Firefox
  • Internet Explorer
OS
  • All of them
  • Windows
  • Linux
  • Android
  • IOS
Hardware Requirements (graphics card, VR Device, ...)
@WestLangley
Copy link
Collaborator

See #8674

@WestLangley
Copy link
Collaborator

BoundingBoxHelper calls Box3.setFromObject().

Box3.setFromObject() does not honor drawRange or groups.

It is up for debate as to whether it should.

@tonnot
Copy link
Author

tonnot commented Apr 21, 2016

Drawrange and Groups are used to draw the buffers so IMHO they should be used at setFromObject
This code works for me:

( second part of setFromObject)

else if ( geometry instanceof THREE.BufferGeometry && geometry.attributes[ 'position' ] !== undefined ) {


                    var positions = geometry.attributes[ 'position' ].array;

                    var pseudoGroups = [];

                    var from,to;

                    if (geometry.groups.length==0)
                        pseudoGroups = [{'start': geometry.drawRange.start, 'count': geometry.drawRange.count}];
                    else
                        pseudoGroups = geometry.groups;

                    for ( var i = 0, il =pseudoGroups.length; i < il; ++i ) { 

                        from =  pseudoGroups[i].start*3;
                        to   = (pseudoGroups[i].start+pseudoGroups[i].count)*3;

                        for ( var j = from; j < to; j += 3 ) {

                            v1.fromArray( positions, j );
                            v1.applyMatrix4( node.matrixWorld );
                            scope.expandByPoint( v1 );

                        }

                    }

                }

The only thing to evaluate is wether it can be possible to have a drawrange start-count that limits the groups. (2 or 3 more lines of code).

Thanks

@WestLangley
Copy link
Collaborator

Continuing the discussion...

Groups exist for the purpose of assigning multiple materials to a geometry. I am not sure why groups should be considered here.

Also, BufferGeometry.computeBoundingBox() does not honor drawRange or groups.

Also, material.visible = false is not honored. Nor are morphs.

In any event, you would have to handle indexed and non-indexed BufferGeometry differently.

@tonnot
Copy link
Author

tonnot commented Apr 21, 2016

Ups , sorry for the mistake. I was mixing concepts

Maybe this is more reasonable...
The idea is use the right range, use a length value is wrong if I have a dynamic array I fill with new data and then, range.count is the valid value.

 else if ( geometry instanceof THREE.BufferGeometry && geometry.attributes[ 'position' ] !== undefined ) {

            var positions = geometry.attributes[ 'position' ].array;

           var  from = geometry.drawRange.start*3;
           var  to    = (geometry.drawRange.start + geometry.drawRange.count)*3;

                    for ( var j =from ; j < to; j += 3 ) {

                        v1.fromArray( positions, j );
                        v1.applyMatrix4( node.matrixWorld );
                        scope.expandByPoint( v1 );

                }

     }

Thanks and sorry

@WestLangley
Copy link
Collaborator

Like I said,

In any event, you would have to handle indexed and non-indexed BufferGeometry differently.

The draw range is over the index for indexed BufferGeometry.

But even so, it is debatable if drawRange should be honored.

@mrdoob
Copy link
Owner

mrdoob commented Apr 12, 2020

Hmm, groups are definitely a can of worms. But drawRange could be interesting...

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

No branches or pull requests

4 participants