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

Refactor/split up GltfImport #739

Open
1 task
Tracked by #736
atteneder opened this issue Dec 10, 2024 · 0 comments
Open
1 task
Tracked by #736

Refactor/split up GltfImport #739

atteneder opened this issue Dec 10, 2024 · 0 comments
Labels
enhancement New feature or request refactor Code base improvements, not necessarily feature/API changes.
Milestone

Comments

@atteneder
Copy link
Owner

atteneder commented Dec 10, 2024

The GltfImport class has grown massively over time and currently serves many purposes:

  1. Main API entry point (e.g. GltfImport.Load)
  2. Orchestration of loading glTFs
  • JSON parsing
  • Loading Resources from URI (buffers, images)
  • Manage asset conversion
    • Mesh Generation
    • Image/Texture Generation
    • Draco/Meshopt decoding
    • Animations
  1. Scene instantiation
  2. Resource Lifetime Management

That makes this class (and the file it resides in) hard to understand and maintain. We should explore ways in which it could be split up into multiple single-purpose classes/structs.

It could remain the main API entry point, or we use a builder pattern to make setting up a glTF load context easy to read/understand.

@atteneder atteneder added enhancement New feature or request refactor Code base improvements, not necessarily feature/API changes. labels Dec 10, 2024
@atteneder atteneder moved this from To do to Planned in glTFast development Dec 18, 2024
@atteneder atteneder added this to the 7.0 milestone Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request refactor Code base improvements, not necessarily feature/API changes.
Projects
Status: Planned
Development

No branches or pull requests

1 participant