Adds useful functions for working with models.

[Kleptine]

Still wrapping my head around Rust, so definitely let me know if my code is odd.

Additionally, I wrote a number of functions for my own usage, which I've copied below. I wasn't sure they were worth submitting, but let me know if you would prefer me to add them to fbxcel-dom as well.

// could be worth contributing, not sure

#[derive(Debug, PartialEq)]
struct CoordinateAxis {
    up: Vector3<f64>,
    front: Vector3<f64>,
    coord: Vector3<f64>,
}

fn get_coordinate_axis(doc: &Document) -> Option<CoordinateAxis> {
    let global_settings = doc
        .global_settings()
        .ok_or("Count not find global settings in file.")
        .ok()?;

    let upAxis = get_axis(&global_settings, "UpAxis")?;
    let frontAxis = get_axis(&global_settings, "FrontAxis")?;
    let coordAxis = get_axis(&global_settings, "CoordAxis")?;

    Some(CoordinateAxis {
        up: upAxis,
        front: frontAxis,
        coord: coordAxis,
    })
}

fn get_axis(global_settings: &PropertiesHandle, name: &str) -> Option<Vector3<f64>> {
    let axis = if let AttributeValue::I32(v) = global_settings
        .get_property(name)?
        .value_part()
        .get(0)? {
        v
    } else {
        return None;
    };

    let sign = if let AttributeValue::I32(v) = global_settings
        .get_property(&(name.to_owned() + "Sign"))?
        .value_part()
        .get(0)?
    {
        v
    } else {
        return None;
    };

    Some(match axis {
        0 => [*sign as f64, 0f64, 0f64].into(),
        1 => [0f64, *sign as f64, 0f64].into(),
        2 => [0f64, 0f64, *sign as f64].into(),
        _ => return None
    })
}

fn get_models<'a>(doc: &'a Document) -> impl Iterator<Item=TypedModelHandle<'a>> {
    return doc.objects().filter_map(|o| {
        if let TypedObjectHandle::Model(model) = o.get_typed() {
            return Some(model);
        }
        return None;
    });
}

fn get_model_roots<'a>(doc: &'a Document) -> Vec<TypedModelHandle<'a>> {
    let mut results: HashMap<ObjectId, TypedModelHandle> = HashMap::new();
    for model in get_models(doc) {
        let root = model.root_model();
        results.insert(root.object_id(), root);
    }
    let vec = results.values().cloned().collect();
    return vec;
}

---

This change is 

[lo48576]

Additional request: Apply rustfmt by cargo fmt and fix warnings emitted by cargo clippy. Clippy may emit a warning for the code where you didn't changed, but you can ignore that.

[lo48576]

About CoordinateAxis

Coordinate system can have theoretically 6 * 5 * 4 (= 120) variations, and this can be represented by a small size type as well as primive integer types. I think this type should be reasonably small, and also the crate should provide a function to swap components of vectors (for example, CoordinateSystemConverter::new(source: CoordinateSystem, dest: CoordinateSystem) -> CoordinateSystemConverter and CoordinateSystemConverter::convert(vec: Vector3) -> Vector3 or something like that). Having three Vector<f64>s is too large, and may make coordinate system converter more complex than necessary.

About get_models()

I don't understand when it is useful. To iterate things without considering hierarchy (for example, extracting meshes or textures?), simply iterating all objects and filtering them by more specific object type is enough. (See https://github.com/lo48576/fbx-viewer/blob/develop/src/fbx/v7400.rs#L66-L70 for example.) Getting all nodes of any model types seems too broad for me.

If objects hierarchy is important, then iterating models in doc.objects() order is meaningless, because it does neither guarantee nor provide any meaningful ordering. Users should perform tree traversal instead of iterating all models in such cases.

About get_model_roots()

I checked only three FBX files, but all root models are children of scene nodes. If this can be said also for your files, you can iterate toplevel models as below:

doc.scenes()
    .filter_map(|scene| scene.root_object().ok())
    .filter_map(|scene_root| match scene_root.get_typed() {
        TypedObjectHandle::Model(o) => Some(o),
        _ => None,
    })

This version does not emit duplicates, and iteration itself does not allocate. (Yeah this is less semantically clear than your version, due to lack of scene-related APIs...)

[Kleptine]

Thanks for all of the comments and thoughts! We have a milestone we are attempting to ship this week, so I'll circle back around on all of this a little later this week or next.

[lo48576]

About local_scaling and local_rotation

I found that TextureHandle type exposes properties through TextureHandle::properties(&self), which returns TextureProperties<'_>. Additionally, some of methods of that type return concrete scalar / vector types such as f64 and Vector3<f64>. I don't think this is ideal since it would cause redundant type conversions when users wanted f32 or Vector3<f32>, but I might have compromised when I wrote it.

So, I changed my mind to think that returning mint::Vector3<f64> from local_scaling() and local_rotation() may be consistent and can be acceptable at least for now.

[Kleptine]

Re: CoordinateAxis

~~I can see this point, although I wasn't clear whether it's actually a guarantee that the coordinate axis can't be diagonal.~~ NVM

Thinking over it seems there would be fewer than 120 options, in fact, because some permutations aren't valid. For instance, +z,-z,y is invalid. Each axis has to be used at least once. I'm a little sleepy though, so I'll have to think a little more about how to build a good mapping.

I wonder though if this is actually a performance boost? The raw data is in the format of:

{
   Up: [i8; 3],
   Forward: [i8; 3],
   Coord: [i8; 3],
}

Would it be more performant to expose a struct closer to the underlying values than to pay for converting to and from the u8 representation? At the very least it's a little unclear:

• This is also a type that only seems used in GlobalSettings. It doesn't seem likely that you'd have a large array of these objects where size becomes important.
• When using the type you'd generally transform it into a [i8,i8,i8], anyway?

I don't have strong feelings, just doesn't seem like an obvious win, and it complicates the code.

About get_models()

Agreed, this was mostly just in service of having more semantic APIs.

About get_model_roots()

Ah, I wasn't aware that all of the roots were children of a scene object. Agreed that it would be nice to have a semantic API here.

[Kleptine]

I think this is good to go, unless I'm mistaken.

Separately, I also have a Pre-Rotation accessor, which I found was output often by 3DS Max. I can make a separate PR for that, or include it here. Really I suppose there should be an accessor for each of the following: https://download.autodesk.com/us/fbx/20112/FBX_SDK_HELP/index.html?url=WS1a9193826455f5ff1f92379812724681e696651.htm,topicNumber=d0e7429

Question: it seems like you'd eventually want to just expose the final composite transform for a model, with the same equation as the doc there. But right now the only math library included is mint which doesn't seem to actually implement mathematical operations. Would you have to add cgmath, or something similar to get this behavior, or am I misunderstanding mint?

[lo48576]

Good catch! I know that expression to compute transformation matrix, but I've got tired before implementing accessors for every term in the expression and I gave up at that time 😩

I think the computation of the transformation matrix should be (at least for now) done by users of the library, rather than in this library. It is because I don't want to introduce cgmath or some other relatively large libraries for mathematical computation (for now). Some may want to use their fast or specialized computation methods, and for such users this additional dependency is undesirable. Providing such computation functions through a feature flag would be good idea, but it is extra goal.

mint crate does not implement computations of vectors and matrices, so users should convert them for types they really use (e.g. types in cgmath, nalgebra, glam, or anything else). I don't have good idea to do the computation efficiently without pulling additional dependencies (i.e. using types and algorithms provided by the user), so I want to postpone implementing the function.

[Kleptine]

Makes sense. I guess you could return a Vec<mint::Matrix4x4>. A little weird though.

The lack of a solid standardization around math libraries is really a stumbling block as a new rust user! Converting between mint types and cgmath is really awkward, and makes cases like these quite challenging!