Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Static glTF mesh loading #200

Closed
wants to merge 73 commits into from
Closed

Static glTF mesh loading #200

wants to merge 73 commits into from

Conversation

JosiahWI
Copy link
Contributor

@JosiahWI JosiahWI commented May 18, 2023 •
edited
Loading

Part of minetest/minetest#9673

We use tinygltf for the parsing and Catch2 for the unit tests. The loader currently supports static meshes in glTF text format, but tinygltf will make it easy to support binary glTF files in a future version. Because we intend to support as much of the glTF feature set as we can in the future, we need to ensure forward compatibility, and that's the main thing I want feedback on.

Feature set:

  • meshes with multiple primitives
  • floating point and unsigned normalized texcoords

Features not supported yet (planned for separate PRs):

  • static bones
  • animations
  • materials
  • probably some other things

TODO:

  • Handle byteStride that may be present on vertex buffers.
  • Handle u8 and u16 texture coordinates.
  • Print warnings and errors to log; I have the change somewhere, just need to find it.
  • Consider whether tinygltf should be modified to use a lighterweight JSON library such as jsoncpp. (discussed on Discord; keeping nlohmann-json for now)
  • Add copyright notices to the source and header file.
  • Unit test and merge the fix for the scaling done by @jordan4ibanez.

Thank you to GreenXenith and appgurueu for all their assistance, and especially to jordan4ibanez for adding a lot of improvements.

@rubenwardy rubenwardy added the enhancement New feature or request label May 18, 2023
@rubenwardy rubenwardy added this to the 5.8.0 milestone May 18, 2023
@sfan5 sfan5 self-requested a review June 17, 2023 09:12
@numberZero
Copy link
Contributor

What’s the value of glTF over Obj and B3D?

@rollerozxa
Copy link
Member

What’s the value of glTF over Obj and B3D?

An animated model format that isn't obsolete.

@JosiahWI
Copy link
Contributor Author

glTF has better tooling support. One of the major issues with b3d is that we don't have a good exporter for Blender>=3.0. Blender and Blockbench both export to glTF.

@JosiahWI
Copy link
Contributor Author

Discussed offline, and I'm hearing that my previous statement is no longer true.

@Montandalar
Copy link

Montandalar commented Jun 30, 2023 •
edited
Loading

While the latest version GreenXenith's exporter may have Blender >=3.0 support, that exporter codebase is still not really very nice to deal with. It has caused me all sorts of problems. While I can't vouch for the glTF exporters of various programs yet because I don't regularly use Blender or Blockbench for another application, I would still think they are a fair bit better than b3d tools, and more likely to improve than the old b3d tools we have basically had to maintain ourselves.

@rubenwardy
Copy link
Member

See minetest/minetest#9673

@rubenwardy rubenwardy mentioned this pull request Jun 30, 2023
Closed
@numberZero
Copy link
Contributor

See minetest/minetest#9673

Thanks. Apparently the alternatives to glTF are worse.

@nerzhul I remember you were against making Irrlicht a submodule. But, tinygltf is another story, it’s not like we’re going to commit into it ourselves—so what’s your opinion on putting it into a submodule?

@JosiahWI JosiahWI mentioned this pull request Nov 29, 2023
Closed
@JosiahWI
Copy link
Contributor Author

There are some issues with the build, but the code part has been rebased.

@jordan4ibanez
Copy link
Contributor

A rebase huh? I'll shelf my other PR and call it something dramatic

@SmallJoker SmallJoker modified the milestones: 5.8.0, 5.9.0 Dec 2, 2023
@appgurueu
Copy link
Contributor

appgurueu commented Jan 5, 2024 •
edited
Loading

Current work in progress:

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

@appgurueu appgurueu mentioned this pull request Jan 8, 2024
Closed
8 tasks
@appgurueu appgurueu added the WIP Work-in-Progress label Jan 22, 2024
@appgurueu appgurueu marked this pull request as draft January 22, 2024 22:09
@sfan5 sfan5 removed their request for review April 15, 2024 13:23
JosiahWI and others added 27 commits April 18, 2024 07:26
@JosiahWI
Move parsing methods into ModelParser class
79bfd8a
@JosiahWI
Fix function declaration indents
85fdf95
@JosiahWI
Add r-value reference constructor to ModelParser
50da042
@JosiahWI
Convert snake_case variable names to camelCase
8c9a576
@JosiahWI
Document coordinate system change at top of source
5f5dd7a
@JosiahWI
Refactor getIndices()
ea22b64
@JosiahWI
Return vector from getVertices
60a5f57
@JosiahWI
Add const to numeric parameters
fd41ee0
@JosiahWI
Document readVec3DF and getScale
22f6012
@JosiahWI
Mark ModelParser constructors noexcept
0f73c30
@JosiahWI
Rename ModelParser to MeshExtractor
3930bd8
@JosiahWI
Extract loadPrimitives method from createMesh
0896aae
@JosiahWI
Refactor vertex loading
53c92b0
@JosiahWI
Print glTF loading issues to log
16b7841
@JosiahWI
Borrow CReadFile for tests
05ede68 
The method to load a mesh from a file was removed. This is not a good
fix, but it will keep the tests working until a file loader can be
properly exposed to the tests.
@JosiahWI
Do not install tinygltf export from Irrlicht
eabc84a
@JosiahWI
Add operator<< definition for vector3d
86026f9
@JosiahWI
Handle byte stride on vertex buffers
95a4030 
Vertex buffers (and only vertex buffers) may have a byte stride
specified.
@jordan4ibanez @JosiahWI
Package glTF spec docs with comments
f8f1024 
* git --CRUSH

And try to make this line the same

Now undo my gitignore nonsense

Hail mary

Move these functions to the top

More documentation

Add some serious documentation

Move this to the top

Readability & documentation

Add some documentation

Add spec documentation

Make this more more complex to make it less complex

Move this up & document

Make this more readable for me

Document and make this more readable

Move this out of the way and document it

Update CGLTFMeshFileLoader.cpp

Documentation

Document

Update CGLTFMeshFileLoader.cpp

Document

Move entry point to bottom

Part 5

Part 4

Part 3

Part 2

Allman -> OTBS (readability for me)

Consolidate

Remove unneeded function

These files are annoying

* fix #1

* fix 2

* indentation

* Remove redundant function

* Remove redundant function

* add trailing new line

* Stop an interesting build error with a template

* Code reduction

* Document bytestride

* Add more detail to chain hierarchy

* Dump this huge vscode thing in here for no reason

* Document 3 more functions

* Document copyTcoords

* Dump in (incorrect) scale documentation for node

* Add a readable description

* Fix incorrect getScale

* Add update based on context of function

* Add documentation, fix another one

* Document another

* Document another

* And document another

* Document this nice function

* Bolt in some future use documentation

* Add highlighting for IDE

* Just shovel on more docs to this

* 2 more comments

* I think that's all of them

* Add some wild west debuggin

* And then update this comment

* Bolt on isAccessorNormalized()

* Bolt in an absolute hack job to test

* Now clean this mess up and give josiah ref material

* Fix Josiah's request for scale

* Fix josiah request

* Fix josiah request .vscode

Fix indentation

Fix indentation

Fix indentation

* Unfix getScale()

again
@appgurueu @JosiahWI
Fix memory leak caused by missing drop
e2b05f0
@JosiahWI
Axe the minimum-cmake CI build
f33b8a6 
This is useful, but has no business being in this PR. Off with its head.
@jordan4ibanez @JosiahWI
Fix getScale()
777ec9d 
* Fix getScale()

* Create blender_cube_scaled.gltf

* Add scaling unit test

* Remove comment

* Undo github's silent reversion of 1,1,1
@appgurueu @JosiahWI
Move from tinygltf to tiniergltf (#13)
0faf132 
* Remove tinygltf

* Integrate tiniergltf

* Update build.yml to include jsoncpp

* Namespace target

* Undo noexcept removal, readd move constructor

* Remove debug throw

* Remove now obsolete build code

* Bump CMake minimum version to 3.12

* Fix oops

* Remove unnecessary install/export

* Remove tinygltf from Config.cmake.in

* Take inspiration from Minetest's FindJson.cmake

* Move tiniergltf to separate repo

* CI: Install git

* Bump tiniergltf version (obtain jsoncpp via FetchContent)

* Remove jsoncpp from build dependency list
@appgurueu @JosiahWI
Fix gltf static mesh loading issues (#14)
036b40a 
* Support u8 / u32 indices

* Skip primitives without vertices

* Add support for non-indexed geometry & skipping primitives

* Fix possible memory leak on error

* Use SSkinnedMesh

* Check indices

* Properly mirror node hierarchy

* Update .gitignore

* Reorder includes

* Add some throws for logic errors

* Fix non-indexed geometry winding order, add unit test

* Address code review comments

* Add matrix transform unit test
@JosiahWI
Move files
718c865
@JosiahWI
Move libs
df4dd7b
@JosiahWI
Fix rebase mistakes in CMakeLists
c263a28
@JosiahWI
Copy link
Contributor Author

JosiahWI commented Apr 18, 2024 •
edited
Loading

Replaced by minetest/minetest#14557.

@JosiahWI JosiahWI closed this Apr 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
enhancement New feature or request WIP Work-in-Progress
Projects
None yet
Milestone
5.9.0
Development

Successfully merging this pull request may close these issues.
8 participants
@JosiahWI @numberZero @rollerozxa @Montandalar @rubenwardy @jordan4ibanez @appgurueu @SmallJoker