Skip to content

Conversation

Jmangles
Copy link

@Jmangles Jmangles commented May 12, 2025

Migration of everything from https://github.com/magicleap/UnityGLTF-Interactivity to UnityGLTF itself.

Some extra work required to implement behaviour engine and wrapper component creation in ImporterPlugin/Context files before merging.

Supports every node in the current spec along with core pointers and most extension pointers. Includes unit tests for every node that can be run in Unity and automatically export as json for consumption in other engines.

@hybridherbst
Copy link
Collaborator

Thanks! Are you going to push those extra work as well in the next days?
Also, please rebase the PR on dev, not on main, to ensure there are no conflicts with latest changes. Thank you!

@Jmangles Jmangles changed the base branch from main to dev May 13, 2025 13:21
@Jmangles Jmangles force-pushed the jrobinson/KHR_interactivity-Playback branch from c80e195 to 48b5cf7 Compare May 13, 2025 13:24
@Jmangles
Copy link
Author

@hybridherbst Rebased on dev, I should be able to finish the importer context stuff this week I think.

@Jmangles Jmangles marked this pull request as ready for review May 13, 2025 15:03
@Jmangles
Copy link
Author

Behaviour engine and wrappers are now added in the InteractivityImporterContext so no additional work needs to be done by devs to enable runtime playback beyond the checkbox in their GLTFSettings asset.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are this logs still necessary?

Copy link
Author

Choose a reason for hiding this comment

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

The majority of the logs in this PR are using Util.Log which is a wrapper using the [Conditional] attribute. As a result they are excluded from builds as if they were wrapped in #if DEBUG_MESSAGES/#endif.

This way the package doesn't spam someone's console but if I want to get all my debug messages back I can do it package-wide by adding DEBUG_MESSAGES to my script compilation symbols in the player settings or add define DEBUG_MESSAGES at the top of classes that I specifically want to enable them for.

@robertdorn83
Copy link
Collaborator

i get an error in Unity:
",,,NodeRegistry.cs(27,51): error CS0246: The type or namespace name 'DebugLog' could not be found (are you missing a using directive or an assembly reference?)"

@robertdorn83
Copy link
Collaborator

and i would suggest to change the plugin names to "KHR_interactivity Playback Importer" and "KHR_interactivity Playback Exporter", just for cleareance :)

@robertdorn83
Copy link
Collaborator

Do you have an exactly reason why are you not using "OnAfterImportScene" , so you're required to add OnLoadingComplete? Between these both callbacks are mostly only memory disposables and cleanups

@Jmangles
Copy link
Author

Do you have an exactly reason why are you not using "OnAfterImportScene" , so you're required to add OnLoadingComplete? Between these both callbacks are mostly only memory disposables and cleanups

I found that in OnAfterImportScene that scene.Extensions is null. I guess that's more of a bug to be fixed than a reason to add another callback.

@Jmangles Jmangles force-pushed the jrobinson/KHR_interactivity-Playback branch from 0a13ec5 to a242aef Compare May 14, 2025 10:10
@robertdorn83
Copy link
Collaborator

Do you have an exactly reason why are you not using "OnAfterImportScene" , so you're required to add OnLoadingComplete? Between these both callbacks are mostly only memory disposables and cleanups

I found that in OnAfterImportScene that scene.Extensions is null. I guess that's more of a bug to be fixed than a reason to add another callback.

OnAfterImportScene has the Scene not the Root as parameter. Don't get confused here, the Extensions of the Gltf are on the Root, not the scene. But you already get correctly the Root in your OnLoadComplete callback with _context.SceneImporter.Root. So just use the OnAfterImportScene with the same code as in OnLoadComplete.
image

@robertdorn83
Copy link
Collaborator

I tried to import a Gltf Asset with interactivity, sadly... your BehaviourEngine is not serializable. Would be really great if you can make it serializable, so your implementation would also work with Gltf Assets. Or maybe save your imported Graph as an subasset, not sure.
Also the EventWrapper name is a bit confusing. Maybe BehaviourEnginePlayer or something like that would match better :)


private void CheckForObjectHoverOrSelect()
{
var ray = Camera.main.ScreenPointToRay(Input.mousePosition);
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Jmangles I believe this code path (and other places where inputs are used) should use Unity's event system. Otherwise this won't work in e.g. XR, for multitouch, and accessibility cases, where inputs come from "somewhere else than he mouse".

@Jmangles Jmangles force-pushed the jrobinson/KHR_interactivity-Playback branch from 5b6a790 to 6aa9c4d Compare May 29, 2025 10:19
Jmangles added 13 commits May 30, 2025 11:19
…ed in the InteractivityImporterContext automatically. Unit tests updated to create their own engine and wrappers as the tests are using generated graphs and non-interactive glbs. Added import events to GltfImportPlugin for mesh, camera, vertex colored material, and load complete.
…in the editor and run via playmode. Previously only interactive GLTFs imported at runtime could play back an interaction.
…ately starts playback. Colliders now get added to any GLTF node object in Unity that if hover/select events are present and the object does not have a collider (if selectable/hoverable extension is present it only adds the collider if it is marked as hoverable/selectable).
…lectability or hoverability extension if a collider is not already present.
…des that don't have a collider during import
@Jmangles Jmangles force-pushed the jrobinson/KHR_interactivity-Playback branch from 216ae7a to 8141209 Compare May 30, 2025 15:20
@Jmangles Jmangles force-pushed the jrobinson/KHR_interactivity-Playback branch from 60dbaad to 9fc86bd Compare June 24, 2025 15:07
Jmangles added 6 commits June 24, 2025 11:55
…deserialization stage rather than each time the node with that configuration runs. Reduces memory alllocations for graphs with heavy use of pointers.
…various math nodes to match current spec. Removed indentation from generated test json.
…th nodes with multiple output values. Added isValid output value to matDecompose.
…pose tests use approximation subgraphs to avoid floating point errors in some engines.
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