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 1.1/2.0 support #65

Closed
stevenvergenz opened this issue Mar 7, 2017 · 13 comments
Closed

glTF 1.1/2.0 support #65

stevenvergenz opened this issue Mar 7, 2017 · 13 comments
Milestone

Comments

@stevenvergenz
Copy link
Contributor

Khronos has issued a Request For Quotes for upgrading this addon for glTF 2.0 support. It sounds like the glTF 2.0 spec is close to completion, so we should look at getting the codebase cleaned up for whoever they bring on to do the support.

@Kupoman
Copy link
Owner

Kupoman commented Mar 9, 2017

Are there any cleaning tasks you had in mind?

I think starting a list of tasks for supporting glTF 2.0 would be a good idea, and has been on my personal todo list for a while. We could probably organize those tasks under a milestone.

@stevenvergenz
Copy link
Contributor Author

There are a couple things I'd like to do. Some of this will depend on if you plan to update BRTE for glTF 2.0 or not.

  1. Change the signature of the export_gltf function to return a tuple of the JSON metadata and the various output buffers, instead of just the metadata. I think it's bad design to write all the external files inside export_gltf, then return the JSON data to be output in a different place. By consolidating the I/O operations, we gain clarity of purpose, improved module usability, and output format agnosticism (e.g. better binary glTF support).
  2. Clean up the options system. This has several parts.
    • We should remove any options that can be safely assumed (combine buffers and prune unused come to mind).
    • We should do what we can to make the options panel more compact, including moving the various embed options into a single row of toggle buttons like the FBX exporter has, and combining option groups.
    • We should change the "shader" option to a "material" drop-down, which would include "Simple" and "GLSL shader". This will make the inclusion of PBR easier for the contractor.
    • We should try to consolidate the option defaults. They are currently in both files independently. Not sure how this would work exactly, but if you agree I can start thinking about it.

@stevenvergenz
Copy link
Contributor Author

Also, we should double down on the BLENDER_physics extension. Being able to define things like a collision mesh will be useful for me.

@Kupoman
Copy link
Owner

Kupoman commented Mar 10, 2017

  1. I agree that something like that is probably the way to go. It is also technically API breaking, so we should be careful about when we do it. I do not think it would interfere with glTF 2.0 support to leave it as is for now.

  2. I will take another pass over the options as I update the documentation for them. I am hesitant to remove the option for combining buffers as I could see some desire in having the file broken up into smaller pieces. For pruning unused data, I could see some issues with creating some kind of data libraries where some data is not used within the glTF file. I do not know how well that concept works with glTF, so I will need to do some investigation. I agree with making things more compact and updating the material setting to reflect the direction glTF is going with materials.

The two sets of defaults are there for a reason. One acts as defaults for the addon, while the other acts as defaults for the module. This allows us to provide reasonable defaults to different users of the converter. I agree that it can be a bit tedious, but I feel the separation is worth it. We could look into having the addon defaults start with the module defaults, and only define differences.

I do not see any of these tasks as blocking glTF 2.0 development. Instead, I think it is best to focus on a solid 1.0 release for blendergltf and defining tasks for supporting glTF 2.0. That being said, I do appreciate this feedback and will be looking for opportunities to address these issues as they line up with other tasks (such as cleaning up the UI and documentation). I will also begin organizing them into their own tasks.

@stevenvergenz
Copy link
Contributor Author

I admit number 1 does break the API, but that's what tags are for. Release the current master as v0.9.0 or something, then after we clean everything up, release v1.0. We don't have to choose with git :)

I'm okay with leaving those options and defaults in place, as long as it was done on purpose. It seemed like they were left over from some previous iteration.

@stevenvergenz
Copy link
Contributor Author

stevenvergenz commented Mar 10, 2017

A few more cleanup suggestions:

  1. Come up with a better way for objects in one block to reference objects in another. For example, right now an object only knows its mesh because the export_node and export_mesh functions use the same name generation algorithm. It would be great if we changed that to a lookup table, combined with dependency-based ordering of the export functions. This would also make updating to index-based lookup easier.
  2. Improve the export speed by not looping over all objects for every block. Instead, export_gltf could loop over the objects, then call export_*. How this is structured is up for debate, but our exporter is very slow right now.

@Kupoman
Copy link
Owner

Kupoman commented Mar 11, 2017

  1. I have already been planning to do something similar to help support 1.0 and 2.0. I have a couple of designs in mind, I just need to figure out which one I like best.
  2. Before going too crazy with optimizing, we should gather some large/slow blend files and profile the exporter with them.

By the way, we have already released a 0.9.0. The tag was created shortly after animations were added. I'll think about changing the interface for 1.0, but I think it works for now and is not really getting in the way.

@kevzettler
Copy link

What other differences are there between the current supported version and matching the 2.0 spec?

@stevenvergenz
Copy link
Contributor Author

stevenvergenz commented Mar 15, 2017

Plenty of spec changes: KhronosGroup/glTF#605

I'd say the most challenging changes for this project are 1) the use of top-level arrays instead of top-level objects, because we don't have a unified lookup system yet, and 2) the new PBR material system, since Blender doesn't have anything similar enough to a PBR model to map the inputs.

@stevenvergenz
Copy link
Contributor Author

@Kupoman @Moguri would you guys object if I started working on a 2.0 branch for this repo? I don't think I can create branches via PR, so if you wanna create an empty branch I'd appreciate it. Alternatively, I could open a PR with the incremental progress and you don't merge it? This is the repo linked in the RFQ, so for discoverability's sake it needs to be here.

@kevzettler
Copy link

kevzettler commented Mar 15, 2017

@stevenvergenz common workflow is for you to fork the repo and create a 2.0 branch on your repo then create a PR against Kupoman/blendergltf for discussion

@stevenvergenz
Copy link
Contributor Author

@kevzettler That is generally the case, yes, and that's how my PRs typically come in. However, because we want to preserve glTF 1.0 capability alongside 2.0, I don't want to open a PR against master, and think it would be cleaner to open the PR against a branch created specifically for 2.0.

@Kupoman Kupoman added this to the 1.1 milestone Aug 11, 2017
@Kupoman
Copy link
Owner

Kupoman commented Aug 11, 2017

With commit df173b9 we now have some initial glTF 2.0 support. It still needs some work, but we can open up issues for specific items.

@Kupoman Kupoman closed this as completed Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants