search

huggingface / gsplat.js

JavaScript Gaussian Splatting library.
MIT License
1.33k stars 87 forks source link

Limit Scene with {x,z,y}{Min,Max}, also add a test framework #22

Closed jeyemwey closed 10 months ago

jeyemwey commented 10 months ago

This PR will add jest as a test framework. It also adds a Scene.limit method that removes the number of visible splats based on their position.

It currently cuts out a Cuboid with the given bounding coordinates, i.e., for the bonsai scene, you can do: scene.limit(-2, 2, -2, 2, -2, 2):

image

The function is currently non-destructive, you can run the function again with a larger bounding box and the cut-out splats reappear.

Also, as always with splats, the function will only check for the positions, and edges will inevitably end up fuzzy.

dylanebert commented 10 months ago

Awesome, thank you! Jest works for me, I know @xenova has experience with it as well.

For the limit method, what do you think about the following for more future extensibility?

scene.limit(shape: BoundingShape)

Then we can define a BoundingShape.ts class and an inherited Box.ts class. Or something along those lines. There could be a helper limitBox method in scene if you want to keep the workflow the same.

jeyemwey commented 10 months ago

For the limit method, what do you think about the following for more future extensibility?

scene.limit(shape: BoundingShape)

I like the idea but the "is point in bounding shape" function seems to get extremely complicated quickly, especially if you implement it yourself.

There could be a helper limitBox method in scene if you want to keep the workflow the same.

We could make use of method overloading (as you did in Vector3/add) or rename the function right now to prevent a breaking change later.

dylanebert commented 10 months ago

Actually for now I think it's fine to just name it limitBox and push as is :)

jeyemwey commented 10 months ago

Actually for now I think it's fine to just name it limitBox and push as is :)

Updated the PR with new commit c132eb3 according to your proposal.

jeyemwey commented 10 months ago

There seem to be some issues with typecasting in the test, probably becasue of an upstream change? Could @dylanebert maybe take a look into that? Just push into this branch if you want to.

dylanebert commented 10 months ago

I checked it out and got it working with recent changes :) (at least limitBox)

Since Scene has changed a ton, I had to remove the test for now, but kept the actions