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

glTF model support #14234

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
Draft

Conversation

appgurueu
Copy link
Contributor

@appgurueu appgurueu commented Jan 8, 2024

  • Goal of the PR: Add glTF model support.
  • How does the PR work? The Minetest side of things is trivial; just add glTF to the list of supported formats, and add some glTF test models & entities. (Also, I renamed an Irrlicht method which was wrongly named. That is a separate commit.)
  • Does it resolve any reported issue? Closes Support for more modern mesh format(s) (.gltf) #9673.
  • Does this relate to a goal in the roadmap? glTF support is on the roadmap.
  • If not a bug fix, why is this PR needed? What usecases does it solve? Has been explained in the issue already. Still, top points (IMO) are:
    • Lowers the bar to model creation.
    • Provides a solid basis for adding features beyond what .b3d / .x support (better materials, animations, etc.)
    • Brittle .b3d / .x toolchains need not be relied on anymore.

Depends on:

  1. Static glTF mesh loading irrlicht#200
  2. Fix gltf static mesh loading issues JosiahWI/irrlicht#14
  3. Complete glTF loader, add animation support JosiahWI/irrlicht#12

(WIP, PRs 2 and 3 are pending review by Josiah.)

To do

  • Move tiniergltf to a separate repo. (I'm willing to maintain this.)
  • Sparse accessor support (and related accessor refactoring).
  • Fix Irrlicht PR MinGW builds (either by bundling the jsoncpp amalgamation like Minetest or getting sfan to add Jsoncpp to the available system packages fixed by forking jsoncpp and fetching it via FetchContent).
  • Thorough testing and code review, resulting in fixes and refactorings.
  • Warnings, like not using all nodes in a scene or embedding images (which we won't read).
  • Profile to be sure that the loader has acceptable performance (highly likely IMO).
  • (low priority, maybe out of scope, but also low effort) add .glb support for smaller file sizes.
  • (very low-priority, likely out of scope) rewrite the base64 implementation for fun and profit.

How to test

  1. Check out & build https://github.com/appgurueu/irrlicht/tree/feat/gltf-loader-tinier-fixmes-animated.
  2. Run the Irrlicht unittests: ctest --output-on-failure (inside the irrlicht dir).
  3. Check out & build this PR.
  4. Spawn the test entities.
  5. 👁️ 👄 👁️
Screencast.from.09-01-24.00.47.45.webm

Trivia

Spider Bunny


The work to make this possible was done by what I would call Minetest's "inofficial gltf working group" - Josiah, Jordan, and me. The spider model used for testing was made by archfan.

@appgurueu appgurueu added WIP The PR is still being worked on by its author and not ready yet. Roadmap The change matches an item on the current roadmap. labels Jan 12, 2024
@appgurueu appgurueu added this to the 5.9.0 milestone Jan 15, 2024
@sfan5 sfan5 added the Waiting (on dependency) Waiting on another PR or external circumstances (not for rebases/changes requested) label Jan 17, 2024
@Zughy Zughy added the Feature ✨ PRs that add or enhance a feature label Jan 22, 2024
@sfan5 sfan5 added Rebase needed The PR needs to be rebased by its author. and removed Waiting (on dependency) Waiting on another PR or external circumstances (not for rebases/changes requested) labels Apr 13, 2024
@sfan5
Copy link
Member

sfan5 commented Apr 13, 2024

Rebase needed after IrrlichtMt merge.

@rubenwardy rubenwardy removed this from the 5.9.0 milestone Apr 15, 2024
@rubenwardy
Copy link
Member

Too WIP, removed from 5.9 milestone. Please work on this as gltf is a very desired feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@ Client rendering Feature ✨ PRs that add or enhance a feature Rebase needed The PR needs to be rebased by its author. Roadmap The change matches an item on the current roadmap. WIP The PR is still being worked on by its author and not ready yet.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for more modern mesh format(s) (.gltf)
5 participants