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

Skin raycast #6911

Closed
wants to merge 2 commits into from
Closed

Skin raycast #6911

wants to merge 2 commits into from

Conversation

makc
Copy link
Contributor

@makc makc commented Jul 27, 2015

Here is an example of one way #6440 could be solved. I did not do much testing with this, basically I made sure decal example keeps working (means Mesh is not broken) and modified another example to test SkinnedMesh with old geometry. I did not test this with SkinnedMesh + buffer geometry, although very similar code is working with this combination in my r70-based project (see #3187).

@@ -17042,6 +17043,24 @@ THREE.Mesh.prototype.getMorphTargetIndexByName = function ( name ) {

};

THREE.Mesh.prototype.makeVertexGetter = function () {

// we need this to avoid geometry type checks in actual getter
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about adding getVertex directly to Geometry and BufferGeometry instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the getter is different for Mesh and SkinnedMesh

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah...

@mrdoob
Copy link
Owner

mrdoob commented Jul 27, 2015

Also, try not to include /builds in PRs.

@makc
Copy link
Contributor Author

makc commented Jul 27, 2015

Force-pushed last commit without builds.

@dubejf
Copy link
Contributor

dubejf commented Jul 27, 2015

Maybe it would be simpler to implement what you need in THREE.SkinnedMesh.prototype.raycast and leave THREE.Mesh alone?

@makc
Copy link
Contributor Author

makc commented Jul 27, 2015

May be, if you are code duplication fan. As it says, this is only

an example of one way #6440 could be solved.

You can solve it in another way.

@makc
Copy link
Contributor Author

makc commented Sep 24, 2015

Closing as it now has conflicts.

@makc makc closed this Sep 24, 2015
@donmccurdy
Copy link
Collaborator

@makc do you recall if you did benchmarks to see roughly how expensive this is for raycasting? Now that SkinnedMesh no longer supports THREE.Geometry (#15314), this PR could probably be quite a bit simpler if folks are still interested.

@makc
Copy link
Contributor Author

makc commented Nov 28, 2018

@donmccurdy the only difference is that you would have to loop over array of numbers rather than array of vertices. you can try THREE.Mesh with same Geometry and BufferGeometry to see the difference, it would be roughly the same for skins

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 28, 2018

I personally made no good experiences with raycasting and skinning. It might work for simple models but in all other cases the computational overhead is too high. So raycasting per simulation step on geometry level just kills the performance. Working with bounding volumes is one approach to solve this problem.

@makc
Copy link
Contributor Author

makc commented Nov 28, 2018 via email

@Mugen87
Copy link
Collaborator

Mugen87 commented Nov 28, 2018

That's true.

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

Successfully merging this pull request may close these issues.

None yet

5 participants