Generate tangents and normals

[gltf-rs]

Generate tangents and normals

[bwasty]

A nicer interface might be a flag in the import config. Then the primitive parsing code (user side) would not need to distinguish between 'normal' tangents and mikk_tspace tangents.

I'm not sure re-indexing is worth the effort at the moment - my guess is that big, complex models where it might make a significant performance difference will usually already have tangents.

[alteous]

Not sure about the import config flag. The specification says the client is required to generate tangents when unavailable and also to support at least two texture co-ordinates, and one vertex color (amongst others.)

Removing the distinction between regular and mikktspace tangents is a good point and would be beneficial to the user. I propose, instead of the idea in the OP, that the primitive automatically generates normals / tangents upon creation, and that process is not configurable by the user.

big, complex models where it might make a significant performance difference will usually already have tangents

Agreed, although it isn't a lot of extra effort regenerating the index list - we can use genmesh::LruIndexer.

[alteous]

Representing all the different Option<Position> / Option<Normal> / Option<Tangent> / Option<TexCoord> / Option<Color> combinations etc. etc. etc. is becoming increasingly difficult through the type system. Perhaps we should provide a basic vertex type that covers all the mandatory elements (with configurable defaults) and provide a bitflags type to let the user query what was provided / generated at runtime?

[alteous]

#[derive(Clone, Copy, Debug, PartialEq)]
pub struct Vertex {
    position: [f32; 3],
    normal: [f32; 3],
    tangent: [f32; 4],
    tex_coord_0: [f32; 2],
    tex_coord_1: [f32; 2],
    color_0: [f32; 4],
}

#[derive(Clone, Copy, Debug, PartialEq)]
pub struct SkinData {
    joints_0: [u16; 4],
    weights_0: [f32; 4],
}

bitflags! {
    pub struct Flags: u32 {
        const POSITION           = 1 << 0;
        const NORMAL             = 1 << 1;
        const TANGENT            = 1 << 2;
        const TEXCOORD_0         = 1 << 3;
        const TEXCOORD_1         = 1 << 4;
        const TEXCOORD_N         = 1 << 5 | TEXCOORD_0 | TEXCOORD_1;
        const COLOR_0            = 1 << 6;
        const COLOR_N            = 1 << 7 | COLOR_0;
        const JOINTS_0           = 1 << 8;
        const JOINTS_N           = 1 << 9 | JOINTS_N;
        const WEIGHTS_0          = 1 << 10;
        const WEIGHTS_N          = 1 << 11 | WEIGHTS_0;
        const EXTRAS             = 1 << 12;
        const GENERATED_NORMALS  = 1 << 13 | NORMAL;
        const GENERATED_TANGENTS = 1 << 14 | TANGENT;
    }
}

[alteous]

Implementation Note: A node definition does not specify whether the node should be treated as a joint. Client implementations may wish to traverse the skins array first, marking each joint node.

Perhaps we should do this too?

[alteous]

What if we split the primitive iterator into four separate iterators?

/// An `Iterator` that visits the primitives of a mesh.
struct Primitives { ... }

impl Iterator for Primitives {
    type Item = (Option<Geometry>, Option<Decoration>, Option<Skinning>, Option<Extras>);
    fn next(&mut self) -> Option<Self::Item> {
        // If positions are provided, generate normals and tangents (if necessary) and return `Some(Geometry)`.
        // If texture co-ordinates or vertex colors are provided, return `Some(Decoration)`.
        // If joints and weights are provided, return `Some(Skinning)`.
        // If anything else is provided, return `Some(Extras)`.
    }
}

[alteous]

• Geometry

  • POSITION
  • NORMAL

• Coloring

  • COLOR_0

• Texturing

  • TANGENT N.B.
  • TEXCOORD_0
  • TEXCOORD_1

• Skinning

  • JOINTS_0
  • WEIGHTS_0

• Extras

  • TEXCOORD_N
  • COLOR_N
  • JOINTS_N
  • WEIGHTS_N
  • _USER_XXX

[alteous]

^^ I very much like this idea.

[alteous]

Draft:

/// Splits the primitive into its four major categories.
///
/// * Where `POSITION` is provided, the function will return
///   `Some(Positioning)` with computed flat normals if `NORMAL`
///   was not provided also.
///
/// * Where `TANGENT` is provided, the function will return
///   `Some(Texturing)` along with any provided texture co-ordinates.
///
/// * Where `TANGENT` is not provided but `TEXCOORD_0` and `POSITION` is,
///   the function will return `Some(Texturing)` with computed tangents
///   along with any provided texture co-ordinates.
///
/// * Where `COLOR_0` is provided, the function will return `Some(Coloring)`
///
/// * Where `JOINTS_0` and `WEIGHTS_0` are provided, the function will return
///   `Some(Skinning)`.
pub fn split(&self) -> (
    Option<Positioning>,
    Option<Texturing>,
    Option<Coloring>,
    Option<Skinning>,
) {
    let positioning = match (self.positions(), self.normals()) {
        (Some(positions), Some(normals)) => {
            Some(Positioning {
                positions: positions,
                normals: normals,
            })
        },
        (Some(positions), None) => {
            let normals = unimplemented!(/* generate normals */);
            Some(Positioning {
                positions: positions,
                normals: normals,
            })
        },
        _ => None,
    };

    let (order, texturing) = match (
        positioning.as_ref(),
        self.tangents(),
        self.tex_coords(0),
    ) {
        (_, Some(tangents), Some(tex_coords_0)) => {
            let texturing = Texturing {
                tangents: tangents,
                tex_coords_0: tex_coords_0,
                tex_coords_1: self.tex_coords(1),
            };
            (Order::Regular, Some(texturing))
        },
        (Some(positioning), None, Some(tex_coords_0)) => {
            let (order, tangents) = unimplemented!(/* generate the tangents */);
            let texturing = Texturing {
                tangents: tangents,
                tex_coords_0: tex_coords_0,
                tex_coords_1: self.tex_coords(1),
            };
            (order, Some(texturing))
        },
        _ => (Order::Regular, None),
    };

    let coloring = self.colors(0).map(|colors_0| Coloring { colors_0 });

    let skinning = match (self.joints(0), self.weights(0)) {
        (Some(joints_0), Some(weights_0)) => {
            Some(Skinning {
                joints_0,
                weights_0,
            })
        },
        _ => None,
    };

    (positioning, texturing, coloring, skinning)
}

/// Geometry to be rendered with the given material.
#[derive(Clone, Debug)]
pub struct Primitive<'a>  {
    /// The parent `Mesh` struct.
    mesh: &'a Mesh<'a>,

    /// The corresponding JSON index.
    index: usize,

    /// The corresponding JSON struct.
    json: &'a json::mesh::Primitive,

    /// New: Vertex ordering.
    order: Order,
}

/// Defines the vertex ordering of a primitive.
pub enum Order {
    /// Begin from vertex 0, increasing one-by-one.
    Regular,

    /// Custom ordering scheme for when mikktspace shuffles our nicely ordered data!
    Custom(Vec<u32>),
}

[alteous]

I have a type checked prototype available in the mikk-tspace branch. The code is getting quite a bit more complex.

[alteous]

After some discussion on gitter, the (Positioning, Texturing, Coloring, Skinning) properties idea has been dropped in favour of a universal ordering type (currently Order, but naming is subject to change.) A new draft is coming soon.

[alteous]

A new draft is available - see https://github.com/alteous/gltf/blob/mikk-tspace/src/mesh.rs

[bwasty]

Nice! Looks good so far and indices_u32()/tex_coords_f32() work as expected.

[alteous]

You've probably noticed this already but N.B you'll run into unimplemented!() if normals or tangents are unavailable.

[alteous]

I'm really struggling to come up with a suitable abstraction for tangent and normal generation. It's quite straightforward to create an abstraction over normal generation or tangent generation, but not both. Even delegating to the user has its set own problems.

[alteous]

I think this should be delegated to the user and documented as such. We could instead provide a reference implementation in the documentation.

[alteous]

Closed since clone of https://github.com/alteous/gltf/issues/22