tinygltf icon indicating copy to clipboard operation
tinygltf copied to clipboard

Idea: replace animation channel target string with enum

Open jdumont0201 opened this issue 5 years ago • 1 comments

I really like how tinygltf defines constants to work with gltf properties. How about expanding it to animation channel targets too?

Right now, channel targets are accessible as target_path and directly read as std::string from gltf. They only take three possible values "translation","scale","rotation" and enums would allow more efficient comparisons when using it.

I use these edits in my project, keeping the original string value but adding a new enum field

enum TargetPathType
	{
		TINYGLTF_CHANNEL_TARGET_TRANSLATION,
		TINYGLTF_CHANNEL_TARGET_ROTATION,
		TINYGLTF_CHANNEL_TARGET_SCALE,
	};

Edit 1. The class definiton with an additional property and the original for compatiblity:

	
struct AnimationChannel {
        int sampler;              
        int target_node;       
        std::string target_path; 
        TargetPathType target_path_enum;  ///new code
        // ...

Edit 2. When parsing:

 if (!ParseStringProperty(&channel->target_path, err, target_object, "path",
                             true)) {
      if (err) {
        (*err) += "`path` field is missing in animation.channels.target\n";
      }
      return false;
    }
///new code
    if (*&channel->target_path == "translation") *&channel->target_path_enum = TINYGLTF_CHANNEL_TARGET_TRANSLATION;
    else if (*&channel->target_path == "rotation") *&channel->target_path_enum = TINYGLTF_CHANNEL_TARGET_ROTATION;
    else if (*&channel->target_path == "scale") *&channel->target_path_enum = TINYGLTF_CHANNEL_TARGET_SCALE;

jdumont0201 avatar Jul 11 '20 19:07 jdumont0201

@jdumont0201 Good idea!

glTF 2.0 spec schema says

https://github.com/KhronosGroup/glTF/blob/master/specification/2.0/schema/animation.channel.target.schema.json

so we'll need to define enum as:

enum TargetPathType
	{
		TINYGLTF_CHANNEL_TARGET_TRANSLATION,
		TINYGLTF_CHANNEL_TARGET_ROTATION,
		TINYGLTF_CHANNEL_TARGET_SCALE,
		TINYGLTF_CHANNEL_TARGET_WEIGHTS,
		TINYGLTF_CHANNEL_TARGET_STRING,  // e.g. for glTF extension?
	};

Then application can refer target_path string for TINYGLTF_CHANNEL_TARGET_STRING.

PR is appreciated!

syoyo avatar Jul 12 '20 07:07 syoyo