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

Mesh.raycast: Check material.skinning before applying bone transform #20830

Merged
merged 1 commit into from
Feb 6, 2021

Conversation

zach-capalbo
Copy link
Contributor

#19178 introduced bone transformations during raytracing. However, if the object's material does not have skinning set to true, then it should not be applying the bone transformations. Since material.morphTargets is checked before applying the morph targets, it seems reasonable that material.skinning should be checked before applying bone transformations as well.

@Mugen87
Copy link
Collaborator

Mugen87 commented Dec 5, 2020

Since material.morphTargets is checked before applying the morph targets, it seems reasonable that material.skinning should be checked before applying bone transformations as well.

Well, the difference is that morphed meshes don't have a separate class like MorphedMesh. Hence, checking for the material property is the only way to figure out whether morph target animation is enabled or not.

In context of skeletal animation, there is SkinnedMesh. Using a mesh without setting skinning to true does not really make sense. So the additional check is somewhat redundant (although it's of course correct).

@zach-capalbo
Copy link
Contributor Author

Using a mesh without setting skinning to true does not really make sense.

For context, in my use case (a model texture editor), I use material.skinning to toggle between skinning enabled and disabled. It's a lot easier and less error-prone than converting back and forth between Mesh and SkinnedMesh. Especially important to me to be able to toggle, since #19178 can cause a very large slowdown when raycasting a skinned mesh vs a non-skinned mesh.

I also have a quick-and-dirty workaround, if anyone needs it:

let oldRayCast = THREE.Mesh.prototype.raycast
var wasSkinned = false

THREE.Mesh.prototype.raycast = function(...args) {
  wasSkinned = this.isSkinnedMesh
  this.isSkinnedMesh = this.isSkinnedMesh && this.material.skinning
  let res = oldRayCast.call(this, ...args)
  this.isSkinnedMesh = wasSkinned
  return res
}

@Mugen87 Mugen87 added this to the r124 milestone Dec 9, 2020
@mrdoob mrdoob modified the milestones: r124, r125 Dec 24, 2020
@mrdoob mrdoob modified the milestones: r125, r126 Jan 27, 2021
@mrdoob mrdoob changed the title Check material before applying bone transform to skinned mesh during raycasting Mesh.raycast: Check material.skinning before applying bone transform Feb 6, 2021
@mrdoob mrdoob merged commit 3acdc73 into mrdoob:dev Feb 6, 2021
@mrdoob
Copy link
Owner

mrdoob commented Feb 6, 2021

Thanks!

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.

3 participants