warrenm/GLTFKit2

Sketchfab GLTF SCNView.hitTest() problem

lenkawell opened this issue · 10 comments

I’ve run into a problem using SCNView.hitTest() with some GLTF models auto-converted by Sketchfab.com and loaded into a SceneKit scene with GLTFSCNSceneSource().

The models load and render just fine. The problem is that, using default options, SCNView.hitTest() is not returning any hits when passed a location that is correctly on a model node geometry. Hit test works fine with other GLTF and GLB models, including USD, SCN, etc. Just these models seem to be a problem.

I’ve tried debugging it and analyzing the GLTF geometry and materials when loaded into SceneKit but I don’t can’t see what’s wrong.

If I change the hitTest options to be [.boundingBoxOnly: true] it will correctly return hits, but using the bounding box for all tap handling and selection is too coarse and not really usable. I’ve tried many other SCNHitTestOption option settings and have not found any others that work for these models.

For example, this never returns a hit on a Sketchfab-converted GLTF model node:

    @objc func handleTapGesture(_ tap: UITapGestureRecognizer) {

        if tap.state != .ended {return}

        let options: [SCNHitTestOption : Any] = [.searchMode: SCNHitTestSearchMode.closest.rawValue]
        let hits = sceneView.hitTest(tap.location(in: sceneView), options: options)

However, replacing the options with this will correctly return hits:

let options: [SCNHitTestOption : Any] = [.searchMode: SCNHitTestSearchMode.closest.rawValue, .boundingBoxOnly: true]

I’ve attached an example Sketchfab GLTF model (zipped folder) that fails (CC). I've also attached a zipped .glb of it that I edited with Gestaltor to remove the animation, but it still fails the same way.

barracuda_fish.zip
BaracudaFish.glb.zip
.

I've debugged this problem further and found that it happens with many models that have a skinner, including the CesiumMan Khronos sample from: https://github.com/KhronosGroup/glTF-Sample-Models

I have not found any solution for the problem, but the Babylon.js developers ran into a problem that may be similar: https://doc.babylonjs.com/divingDeeper/importers/glTF/glTFSkinning

If I comment out the "skinnedNode.skinner = skinner;" code in GLTFSCNSceneSource.convertAsset, hitTest works correctly (of course the animation no longer works). So possibly it's something like what the Babylon developers found that the transforms are being applied twice, or possibly hitTest is ignoring nodes in the skeleton or bones incorrectly. Any thoughts or ideas would be appreciated.
CesiumMan.glb.zip

I imported CessiumMan.glb into Blender and re-exported it as another .glb file and it works fine: renders correctly, animates correctly and hitTest finds it. The node hierarchy is the same as the original, though the geometry is slightly different (more vertices). I also imported CessiumMan.glb into Apple's Reality Converter and exported it as a .usdz file and it also works just fine and it seems to have the exact same geometry, skinner parameters, etc.

I tried messing with the inverse bind matrixes and other parameters, but nothing seems to resolve the hitTest problem. I've moved the skinner to another node and that doesn't seem to help either.

I set sceneView.debugOptions = [.showBoundingBoxes] and the bounding boxes look fine and identical for all three models. And as I mentioned in the original post, using the .boundingBoxOnly: true option in hitTest correctly returns the original CessiumMan nodes.

So in the original CessiumMan.glb file if the skinner is enabled on the geometry node, a normal hitTest fails to return any nodes in the model. If I simply set the node.skinner to nil long after it's loaded, hitTest works fine, though of course skeleton animations stop working. I'm stumped.

CessiumManBlender.glb.zip
CesiumMan.usdz.zip

This could be a red herring, but I was testing making changes to the CessiumMan.glb file with Gestaltor and found that just editing the file and resaving it to change things like node names, child node order, etc. did't solve the problem. However, I noticed that Blender and Reality Converter both use unsigned bytes for the mesh joints accessor but the original CessiumMan.gltf/.glb files use unsigned shorts. Just for fun, I used Gestaltor to change (convert) then to unsigned bytes and amazingly, hitTest works with the modified file. Of course, there could be some other changes that I don't see because changing that accessor size changes the offsets, etc. of all the following ones. Anyway, it seems interesting that there is something odd about the original file that is not being handled by GLTFKit2 correctly. I did a code inspection and stepping of code in the relevant paths of GLTFSceneKit.m and cgltf.h and as far as I can tell, it all looks OK.

After more debugging I've discovered that the hit testing problem does seem to be caused when JOINTS_0 is UnsignedShorts instead of UnsignedBytes. It appears that SceneKit / Metal rendering works OK with UnsignedShorts, but SceneKit hit testing does not. I don't know if it's the best solution, but I added a conversion of the UnsignedShort data to the GLTFSCNGeometrySourceForAccessor method implementation right before creating the SCNGeometrySource and it solves the problem for all the GLTF models that I've tested it with.

// If a Joints node indices data is Unsigned Short (e.g., from Sketchfab, etc.) convert it to Unsigned Bytes because
// although SceneKit/Metal rendering works OK with UInt16, SceneKit hit testing appears to only support UInt8.
if ([semanticName isEqualToString:GLTFAttributeSemanticJoints0] && accessor.componentType == GLTFComponentTypeUnsignedShort && accessor.dimension == GLTFValueDimensionVector4) {
    NSInteger byteElementSize = elementSize / 2;
    NSInteger bufferLength = accessor.count * byteElementSize;
    void *bytes = malloc(bufferLength);
    for (int i = 0; i < accessor.count; ++i) {
        UInt16 *joints = (UInt16 *)(attrData.bytes + (i * elementSize));
        UInt8 * byteJoints = (UInt8 *)(bytes + (i * byteElementSize));
        for (int jointIndex = 0; jointIndex < 4; ++jointIndex) {
            byteJoints[jointIndex] = joints[jointIndex];
        }
    }
    attrData = [NSData dataWithBytesNoCopy:bytes length:bufferLength freeWhenDone:YES];
    bytesPerComponent = 1;
    elementSize = byteElementSize;
}

Amazing detective work. We have similar workarounds for other incompatibilities (like 8-bit indices), so I think this would be right at home. I'll try to incorporate it after further testing.

It'd be great if you could include this workaround (or one like it), then I think I can use your branch as-is. Thanks!

This SceneKit hitTest problem seems to have been fixed in SceneKit in iOS 17.0 / macOS 14.0. However, it's still broken in iOS 16 at least as of 16.4.

This may be addressed as of fc5f82a. Would love to know if it works for you. I applied an availability check based on your report and my own testing, and I also guarded against doing the conversion if it risked causing errors in rendering. I think most framework clients would prefer correct rendering over correct hit-testing in the vanishingly rare cases where there are more than 256 bones in a skinner, but if that's a deal-breaker we can expose an import option.

Yes, that fix works fine for my test cases in iOS 16.x. Thanks very much!

Fantastic, thanks for confirming, and for your patience. Closing as resolved.