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

Add support for importing Hubs components from glTF files #63

Merged
merged 133 commits into from
Apr 17, 2024

Conversation

Exairnous
Copy link
Contributor

This PR adds support for importing Hubs components from glTF files. All components from objects, materials, and the scene should be imported correctly.

This PR operates by hooking into the default glTF importer's create object and material methods to first gather a list of glTF nodes with Hubs components and then hooks into the glTF importer's create scene method to add all of the components in their relative places and apply their settings. The data parsing is fairly robust and handles most parameters by default, so it should handle most new components added in the future automatically, as long as they don't require special handling (such as linking to objects like the Video Texture Target component does). If an error occurs while adding a component, it is printed to the terminal and the import continues on. Component information is also currently printed to the terminal.

@Exairnous Exairnous changed the title Import Add support for import Hubs components from glTF files Jan 25, 2022
@Exairnous Exairnous changed the title Add support for import Hubs components from glTF files Add support for importing Hubs components from glTF files Jan 25, 2022
@Exairnous
Copy link
Contributor Author

Here is the GLB file I've been testing importing with:
import_dev_test_6.zip

@Exairnous
Copy link
Contributor Author

Mentioning @netpro2k and @keianhzo since they've shown some interest in this topic previously. :)

@Exairnous
Copy link
Contributor Author

This PR doesn't properly support files exported from Spoke. I will attempt to rectify the problem.

@Exairnous
Copy link
Contributor Author

Updated to the new add-on architecture. Support for lightmaps, and probably bones, still needs to be added, and a bug with the text component needs to be fixed.

@keianhzo keianhzo self-requested a review July 28, 2022 13:45
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

Sorry for the delay reviewing this and thanks a lot for the contribution, this is quite an essential part of the future of the add-on.

I've been testing this and it's a great starting point. As there were quite a few things that I wanted to draft/propose I've created a PR in your fork to avoid making this PR too messy: Exairnous#1

I think that although we need to support LTS, we should also support the new public official API for a smoother transition and to avoid breakage (although it also can happen with the public one in early stages). There is not much more code necessary for supporting both so I've added support in a consolidated importer.

I've also moved all the component specific logic to the components similarly to how we handle the component specific export logic. This makes the importer/exporters much leaner and readable. There is now a new gather_import class method to handle component creation in the blender object. The base method applies the generic behavior and a few components override it for specific cases.

Other that that, there are a few things that need to be addressed, you pointed a few and I'm adding a few more:

  • Support for lightmaps
  • Support for bones
  • Fix a bug with the text component
  • Loop animation actions are not imported correctly
  • Add tests

My idea is merging my proposed PR into this branch (or cherry pick and update parts) so we can continue working on this PR once the branch has been updated. If you have a better idea of how to handle it let me know.

… avatars).

Note: this error affected any armature that is in edit mode with another non-armature selected
and so isn't import specific, but armatures apparently get changed into edit mode briefly when
importing so it gets reliably triggered on import.
@keianhzo
Copy link
Contributor

keianhzo commented Apr 3, 2024

This looks great, I think we are very close to having this working. The tests pass which is a great sign. The only issue that I see so far is the linter issues. What else is left?

@Exairnous
Copy link
Contributor Author

Thanks! Yeah, it's pretty close, I just have to remove the reflection probe changes (as discussed), make the box collider component convert the object scale which Spoke uses to determine the area to the half extents used by the ammo shape, add a warning for non [1,1,1] scales to the import report for ammo shapes/media frames, and do a final format.

Then I'm going to open separate PRs to switch the override on audio-params to false (this is required for correct imports), add the mirror component, and add back the fog color property that was accidentally removed in a recent PR.

I hope to have this all done by the end of my day.

Don't support importing reflection probes for right now because the changes needed are too extensive.
@Exairnous
Copy link
Contributor Author

I ran into a couple of problems I didn't account for and had to solve (I did get them solved, though), so I'm not going to make it by the end of my day, but I should make it by the end of my next day.

…(accounting for parents) to half extents/offsets.

Note: the half extents need Y and Z swapped and the offsets need the same, plus the value going into the Z offset field needs its positive/negative sign swapped.  Also, for some reason a timer has to be used to set the half extents/offsets otherwise they don't apply.
…ed and refactor how the scale is calculated into a function so it can be reused.

Note: I don't think the scale calculation accounts for when the host is a bone, but it's unclear whether this is needed or not.
…e in Blender versions under 3.1.

This allows for nodes without a name to have their components imported and also makes sure the correct node is imported when there are more than one with the same name (Spoke can generate files with duplicate names).
Note: the old way of getting the node by name is left as a fallback, just in case.
@Exairnous Exairnous marked this pull request as ready for review April 5, 2024 09:53
@Exairnous
Copy link
Contributor Author

@keianhzo Okay, so I think this is ready for review now. The heightfield component isn't imported, but I believe it isn't needed for Blender equivalent so I've set it to be ignored. The trigger volume component isn't imported either, but that gives the user a warning. Maybe in the future, trigger volume components could be mapped to behavior graphs.

The box collider required special support to import properly. I think everything is working properly now, but maybe give it an extra once over just to make sure.

I've opened the other PRs for the mirror and audio override so they can be imported properly as well.

The one case that doesn't import is if there is an audio-params component by itself on an object (without any audio or video components), but that's not a valid setup and I think it's highly unlikely to occur in a real use case.

@keianhzo
Copy link
Contributor

keianhzo commented Apr 5, 2024

Awesome, thanks for working on this. I'll review on Monday.

@keianhzo Okay, so I think this is ready for review now. The heightfield component isn't imported, but I believe it isn't needed for Blender equivalent so I've set it to be ignored. The trigger volume component isn't imported either, but that gives the user a warning. Maybe in the future, trigger volume components could be mapped to behavior graphs.

There are a few components in Spoke that we don't support but I don't expect many people going from Spoke to Hubs so I'm not super concerned about not supporting those.

The one case that doesn't import is if there is an audio-params component by itself on an object (without any audio or video components), but that's not a valid setup and I think it's highly unlikely to occur in a real use case.

Audio zones can have audio-params without autio/video components, those should still import.

@Exairnous
Copy link
Contributor Author

Awesome, thanks for working on this. I'll review on Monday.

You're welcome. Sounds good.

There are a few components in Spoke that we don't support but I don't expect many people going from Spoke to Hubs so I'm not super concerned about not supporting those.

Honestly, I still think importing from Spoke is going to be the main use case for this, especially with Mozilla ceasing development/hosting of Hubs and people wanting to back up all their stuff. I thought I had accounted for all the Spoke components except for the heightfield and trimesh (which I think I recall asking about and getting the answer that they weren't needed for the Blender process), and the trigger volume component (which was experimental and not advised to be used anyway). If there are any others that aren't being supported, or if I'm mistaken in any of this, let me know and I'll add support for whatever is missing.

Audio zones can have audio-params without autio/video components, those should still import.

Ah, sorry. What I probably should have said there is "without any audio or video related components". Anything that automatically adds/removes the audio params component in Blender, is fully supported for import, but in previous versions of the exporter you were able to add an audio-params component by itself without anything else, so that is the one case I was referring to that I didn't support (and I don't think needs to be supported because having a singular audio-params component on an object without anything else is an invalid setup).

This adds in a second text test because there are two valid states for clip rect:
not being there at all and an array of four values.
@Exairnous
Copy link
Contributor Author

Hey, so I've added in support for the Clip Rect property on text components. I had initially planned to do this as a separate PR, but since it needs a custom gather_import I figured it would be easier to just add it here. Also, do you know if it's valid for the Clip Rect Min values to go above zero, or for the Clip Rect Max values to go below zero? (I'm wondering if we should enforce hard limits there for the exporter even though Spoke doesn't).

@keianhzo keianhzo merged commit f3e5aee into Hubs-Foundation:master Apr 17, 2024
6 checks passed
@keianhzo
Copy link
Contributor

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment