[r139 BUG?] `raycaster.intersectObject` fails because groupMaterial index is -1 in imported FBX model

I imported an FBX model, then when I try to run raycaster.intersectObject on it, I get:

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'side')
    at checkIntersection (modules.js?hash=5362f5cdc7b6201ffad984d49b05ef801a326b32:119257:16)
    at checkBufferGeometryIntersection (modules.js?hash=5362f5cdc7b6201ffad984d49b05ef801a326b32:119339:23)
    at Mesh.raycast (modules.js?hash=5362f5cdc7b6201ffad984d49b05ef801a326b32:119201:23)
    at intersectObject (modules.js?hash=5362f5cdc7b6201ffad984d49b05ef801a326b32:26018:10)

Because in checkIntersection the material parameter receives undefined due to Mesh receiving undefined when accessing const groupMaterial = material[ group.materialIndex ]; and passing that in, where group.materialIndex is -1 which is an invalid array index.

Does the code need to perhaps handle this case?

r139

Yeah, checkIntersection needs to account for the fact that material can be undefined, because the most important thing is the geometry.

It should simply assume no back face culling in case there’s no material, because there’s no way to otherwise know the sidedness.

Do not read this if you don't like TypeScript

TypeScript prevents these sorts of silly mistakes.

The type of the material parameter for checkIntersection would have | undefined and would cause a type error inside the function, or it would not have | undefined and a type error would happen at the call site, and either way the silly mistake would be caught. etc.

	if ( material && ( material.side === BackSide ) ) {

		intersect = ray.intersectTriangle( pC, pB, pA, true, point );

	} else {

		intersect = ray.intersectTriangle( pA, pB, pC, material ? material.side !== DoubleSide : false, point );

	}

Why is group.materialIndex = -1 though? I’m not sure that’s a valid input the raycaster should need to be checking for…

Yeah, not sure. All I did was import the FBX model with FBXLoader, then cast at it.

Despite that though, strictly requiring a material is debatable, because raycasting mainly depends on intersecting geometry. I think material can be considered more as a hint.

For now I patched it and I’m on my way: fix: don't crash when `material` is `undefined` during `ray.intersec… · trusktr/three.js@cd46ade · GitHub

so FBXLoader issue then?

Probably yes. Groups are define here:

@trusktr Do you mind sharing your FBX asset in this topic?

@mugen I believe it is this one:

You can clone that repo, then

  • modify the three dependency in package.json to point to ^0.139.0 instead of my fork
  • install Meteor (Node-based full-stack reactivity system) with curl https://install.meteor.com/ | sh
  • npm install
  • npm start
  • open http://localhost:3000 in two browsers/tabs
  • try making a head shot
  • see console in the browser in which you shoot

I think only the head.fbx file had that issue as I noticed during head shots. But just in case, the other models are in that folder too.

The FBX file was exported from Cinema 4D.

1 Like

I can see in the debugger that the loader actually extracts negative material indices (-1) from the FBX asset. Unfortunately, I’m not familiar with FBX so I can’t tell you what such definitions mean. However, FBXLoader does not expect them.

Looks like there are two things to iron out:

  • should ray casting just work without material? Seems reasonable, assuming the same as DoubleSide in that case.
  • does FBXLoader need a fix? Or is Cinema 4D doing something wrong?

Raycasting should honor the side property of materials.

That depends on what the FBX spec says about material indices.

@makc3d it looks like you and @looeee have some experience with negative indices in

@looeee any ideas on the above? The model is simply exported as FBX from Cinema 4D.

@trusktr see my comment from that issue:

It shouldn’t have any negative indices. If it does, then either:

  • there’s a problem with your model
  • there’s a problem in the parser

I should have added a third possibility: a problem in the exporter. I expect that’s the issue here, unless things have changed in the last year or two, the Cinema4d FBX exporter is really buggy. You could try manually editing the FBX file to change any -1 to a 0 and see if that gives the correct result. I would hesitate to add that as a fix to the loader though because I don’t think it’s valid FBX and probably can’t be expected to give correct results in every case.

2 Likes

I feel as if we can be more pragmatic:

Ideal:

  • expensive well-known product exports model, Three.js works.

Not ideal:

  • expensive well-known product exports model, Three.js doesn’t work.

I know we wish all programs exported FBX the same way, but the reality is a major program exports indices with -1 values.

I watched the designer choose parts of those simple models above, simply pick a different color for each part, then export the model as FBX. There’s not much a designer can do to fix that.

The reality is a major program (Cinema4d) exports FBX files with a large number of problems, not just this one. While working on the FBXLoader I did try to take a pragmatic approach and it already does handle many cases like this even though they are technically incorrect. However, towards the end nearly all the remaining problems were coming from models exported by the buggy Cinema4d exporter and I was finding that fixing one model often resulted in breaking other (correct) models and I was having to tediously check each fix against 50 or so models. I also don’t use Cinema4d so there wasn’t much motivation for me to work on bug fixes for it.

The fix for this particular bug doesn’t seem like it would break other models, and if you find that setting the negative indices to zero (or perhaps they wrap around to the end of the array?) then by all means please do make a PR with a fix.

I also might phrase this as expensive well-known product has had a buggy FBX exporter for years and doesn’t seem to have made any steps to fix it - even though they are presumably using the FBX SDK and should have a much easier time than we do since we have to reverse engineer the proprietary spec. Why should the onus be on us open source devs working for free to support something when the company selling a product for $4000 won’t fix things on their end?

2 Likes

well it has been 5 years, so I did not even remember that thread. looking at it now, it seems that -1 index could maybe mean that there was no material assigned to that part of the geometry? I mean, just scroll down and look at collada version.

I guess it would be nice if 3js could just not render parts of the geometry where material index is invalid without throwing any error, but they are unlikely to care about this case.

Generally speaking, BufferGeometry .groups and group materials have been a disproportionate obstacle for maintenance and support in various three.js APIs. Invalid material indices are not the only related trouble cases. I would suggest avoiding geometry.groups or multi-material meshes, and using separate meshes — optionally sharing the same vertex attributes and different indices — instead.

Looking forward — be aware that groups are not supported in WebGPURenderer and there are no plans to implement them there. See:

1 Like

Yeah, if I was working on the FBXLoader now I would probably implement this by splitting multi material meshes up into separate meshes. I think that’s what the GLTFLoader does, is that right?

1 Like

Yes, we created multi-material meshes from GLTFLoader for a while but reverted that behavior in GLTFLoader: Remove multipass and primitive merging logic by donmccurdy · Pull Request #15889 · mrdoob/three.js · GitHub.

3 Likes

Could FBXLoader issues be the cause of this too?

Do you have those tests? I could add them as e2e tests in the three.js repo.