Idea: Add gltf-utils crate about gltf

[alteous]

Non-exhaustive list of what would be moved.

• All accessor abstractions from the mesh module (Positions, Colors, TexCoords, Indices, etc.)
• The accessor::Iter type.
• The Source trait.
• The Glb type and its implementation from gltf-importer.

New items.

• Flat normal generation.
• Tangent generation using the mikktspace algorithm.

[alteous]

See the simplify branch for draft implementation.

[bwasty]

I'm still not entirely convinced of the idea, though it's started to grow on me.

As for Glb/gltf-importer - it would be good to refactor the code to make it more reusable. As far as I saw (in master), I'd have to copy large parts with minor changes to re-implement my old HTTP Source (esp. relating to GLB and Validation).
Also - data URI decoding is gone at the moment, right? #46 says it's moved, but I don't see it anywhere.

General remark: I hope the crate stabilises soon. So far I've had no problem with the breaking changes, but this feels like one too much (in such a short time).

[bwasty]

I had the feeling this will create an imbalance between the crates, so maybe it's helpful to list what each would contain:

• gltf-json
  • json structs, mapping 1:1
  • validation
• gltf
  • Wrapper structs around json structs, replacing indices with references
• gltf-importer
  • File-based import helper
  • Config (currently only for validation)
• gltf-utils
  • Convenience iterators for geometry
  • Source
  • Glb
  • normal + tangent generation

If I haven't forgotten something, gltf looks a bit thin and doesn't provide much on its own.

Another topic: Extensions. On Gitter it came up that there are two struct variants per entity, e.g. extensions::Primitive, json::extensions::Primitive. I don't quite understand the design (-> are those structs really needed?). How would implementing an extension work? Perhaps it would be worth trying to add KHR_materials_pbrSpecularGlossiness to see if it works well. It's pretty simple and the only finished extension so far.

[alteous]

gltf looks a bit thin and doesn't provide much on its own

I think this ok. It means build times will be reasonable and from the user's perspective the crate won't be trying to do too much on their behalf.

The way Gltf is constructed needs to be revised. Calling gltf::json::from_x then Gltf::from_json is not intuitive.

.glb files seem ubiquitous enough that its parser should be in the main gltf crate and not gltf-utils. Would you agree?

As for extensions, we should probably implement one and think about the design then. KHR_materials_pbrSpecularGlossiness is a simple candidate, as you mention.

White board ideas for Gltf / Glb parsing:

//! All included within the `gltf` crate.

pub struct Error(...);

impl Gltf {
    pub fn from_slice(data: &[u8]) -> Result<Self, Error> {
        let json = json::from_slice(data)?;
        Gltf::from_json(json)
    }
}

/// `.glb` parsing module.
pub mod glb {
    pub struct Glb<'a> {
        gltf: Gltf,
        blob: Option<&'a [u8]>,
    }

    pub struct Error(...);

    pub fn is_glb(data: &[u8]) -> bool {
        data.starts_with(b"glTF")
    }

    impl<'a> Glb<'a> {
        pub fn from_slice(data: &'a [u8]) -> Result<Self, Error> {
            ...
        }
    }

    impl<'a> ::std::ops::Deref for Glb<'a> {
        type Target = Gltf;
        fn deref(&self) -> &Self::Target {
            &self.gltf
        }
    }
}

[alteous]

General remark: I hope the crate stabilises soon. So far I've had no problem with the breaking changes, but this feels like one too much (in such a short time).

I understand where you're coming from here, and I do hope so too. I'm solely to blame for the wildly different design choices in each release, but I do this because once the crate is in real-world use it's much harder to make breaking changes. I hope for our combined sanity that the next release is the last major redesign!

[bwasty]

.glb files seem ubiquitous enough that its parser should probably be in the main gltf crate and not gltf-utils. Would you agree?

Yes.

About the code snippet: I don't quite like the Deref-pattern. It's good for (smart) pointers and strings; for other types I don't normally expect it and may overlook it in the docs (happened twice already). But if the docs fit on "one page" it might be fine.

[alteous]

I agree. It's probably best to avoid using Deref where possible.

In the simplify branch I've added the glb parsing code and leave the user to construct Gltf from it.

What do you think of this idea to simplify validation?

[bwasty]

Looks pretty good.
Very minor nitpick: The return type Result<Unvalidated, Error> looks a bit weird on it's own (...what's Unvalidated supposed to be?).

[alteous]

gltf::Unvalidated means 'unvalidated glTF`. Please suggest a better name if you can!

[alteous]

I tried using a type parameter Unvalidated<T> and having an Unvalidated<Gltf> elsewhere but it felt overkill.

[bwasty]

Couldn't think of something better; my only idea was UnvalidatedGltf :)

[alteous]

Moving the discussion over to #78