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

Experimental Exporterでjsonに空の配列が含まれglTF違反になってしまうことがあるのを修正 #486

Merged
merged 2 commits into from
Jul 27, 2020

Conversation

saturday06
Copy link
Contributor

@saturday06 saturday06 commented Jul 26, 2020

Experimental Exporterで空配列の除外設定に漏れがあったため、glTF違反のデータを出力してしまうことがあるの修正しました。また、Experimental Exporterが古くなっていたので再生成しました。

こちらのVRM をUniVRM masterブランチ c9b383b でインポートし「Use Experimental Exporter」をON、「Use Sparse Accessor」をOFFの状態でエクスポートした結果の拡張子をglbとし、 https://github.khronos.org/glTF-Validator/ にかけた結果を次に示します。

修正前 修正後

@@ -48,10 +48,21 @@ class Generator : IDisposable
{
{"gltf/animations", "if(value.animations!=null && value.animations.Count>0)" },
{"gltf/cameras", "if(value.cameras!=null && value.cameras.Count>0)" },
{"gltf/buffers", "if(value.buffers!=null && value.buffers.Count>0)" },
Copy link
Contributor Author

@saturday06 saturday06 Jul 26, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -48,10 +48,21 @@ class Generator : IDisposable
{
{"gltf/animations", "if(value.animations!=null && value.animations.Count>0)" },
{"gltf/cameras", "if(value.cameras!=null && value.cameras.Count>0)" },
{"gltf/buffers", "if(value.buffers!=null && value.buffers.Count>0)" },
{"gltf/bufferViews", "if(value.bufferViews!=null && value.bufferViews.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/bufferViews[]/byteStride", "if(false)" },
{"gltf/bufferViews[]/target", "if(value.target!=0)" },
{"gltf/animations[]/channels", "if(value.channels!=null && value.channels.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/animations[]/channels[]/target", "if(value!=null)" },
{"gltf/animations[]/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/animations[]/channels[]/target", "if(value!=null)" },
{"gltf/animations[]/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
{"gltf/accessors", "if(value.accessors!=null && value.accessors.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/animations[]/channels[]/target", "if(value!=null)" },
{"gltf/animations[]/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
{"gltf/accessors", "if(value.accessors!=null && value.accessors.Count>0)" },
{"gltf/accessors[]/max", "if(value.max!=null && value.max.Length>0)"},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/animations[]/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
{"gltf/accessors", "if(value.accessors!=null && value.accessors.Count>0)" },
{"gltf/accessors[]/max", "if(value.max!=null && value.max.Length>0)"},
{"gltf/accessors[]/min", "if(value.min!=null && value.min.Length>0)"},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/accessors[]/sparse", "if(value.sparse!=null && value.sparse.count>0)"},
{"gltf/images", "if(value.images!=null && value.images.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/accessors[]/sparse", "if(value.sparse!=null && value.sparse.count>0)"},
{"gltf/images", "if(value.images!=null && value.images.Count>0)" },

{"gltf/meshes", "if(value.meshes!=null && value.meshes.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/images", "if(value.images!=null && value.images.Count>0)" },

{"gltf/meshes", "if(value.meshes!=null && value.meshes.Count>0)" },
{"gltf/meshes[]/primitives", "if(value.primitives!=null && value.primitives.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -67,17 +78,27 @@ class Generator : IDisposable
{"gltf/meshes[]/primitives[]/attributes/WEIGHTS_0", "if(value.WEIGHTS_0!=-1)"},

{"gltf/meshes[]/primitives[]/extras", "if(value.extras!=null && value.extras.targetNames!=null && value.extras.targetNames.Count>0)"},
{"gltf/meshes[]/weights", "if(value.weights!=null && value.weights.Length>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@@ -67,17 +78,27 @@ class Generator : IDisposable
{"gltf/meshes[]/primitives[]/attributes/WEIGHTS_0", "if(value.WEIGHTS_0!=-1)"},

{"gltf/meshes[]/primitives[]/extras", "if(value.extras!=null && value.extras.targetNames!=null && value.extras.targetNames.Count>0)"},
{"gltf/meshes[]/weights", "if(value.weights!=null && value.weights.Length>0)" },
{"gltf/materials", "if(value.materials!=null && value.materials.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/materials[]/alphaCutoff", "if(!string.IsNullOrEmpty(value.alphaMode))" },
{"gltf/nodes", "if(value.nodes!=null && value.nodes.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/nodes[]/camera", "if(value.camera!=-1)"},
{"gltf/nodes[]/mesh", "if(value.mesh!=-1)"},
{"gltf/nodes[]/skin", "if(value.skin!=-1)"},
{"gltf/nodes[]/children", "if(value.children != null && value.children.Length>0)"},
{"gltf/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/nodes[]/camera", "if(value.camera!=-1)"},
{"gltf/nodes[]/mesh", "if(value.mesh!=-1)"},
{"gltf/nodes[]/skin", "if(value.skin!=-1)"},
{"gltf/nodes[]/children", "if(value.children != null && value.children.Length>0)"},
{"gltf/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
{"gltf/scenes", "if(value.scenes!=null && value.scenes.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/nodes[]/camera", "if(value.camera!=-1)"},
{"gltf/nodes[]/mesh", "if(value.mesh!=-1)"},
{"gltf/nodes[]/skin", "if(value.skin!=-1)"},
{"gltf/nodes[]/children", "if(value.children != null && value.children.Length>0)"},
{"gltf/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
{"gltf/scenes", "if(value.scenes!=null && value.scenes.Count>0)" },
{"gltf/scenes[]/nodes", "if(value.nodes!=null && value.nodes.Length>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/samplers", "if(value.samplers!=null && value.samplers.Count>0)" },
{"gltf/scenes", "if(value.scenes!=null && value.scenes.Count>0)" },
{"gltf/scenes[]/nodes", "if(value.nodes!=null && value.nodes.Length>0)" },
{"gltf/skins", "if(value.skins!=null && value.skins.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/skins[]/skeleton", "if(value.skeleton!=-1)"},
{"gltf/extensionsRequired", "if(false)"},
{"gltf/skins[]/joints", "if(value.joints!=null && value.joints.Length>0)"},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/skins[]/skeleton", "if(value.skeleton!=-1)"},
{"gltf/extensionsRequired", "if(false)"},
{"gltf/skins[]/joints", "if(value.joints!=null && value.joints.Length>0)"},
{"gltf/extensionsUsed", "if(value.extensionsUsed!=null && value.extensionsUsed.Count>0)"}, // dummy
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{"gltf/extensionsRequired", "if(false)"},
{"gltf/skins[]/joints", "if(value.joints!=null && value.joints.Length>0)"},
{"gltf/extensionsUsed", "if(value.extensionsUsed!=null && value.extensionsUsed.Count>0)"}, // dummy
{"gltf/extensionsRequired", "if(false && value.extensionsRequired!=null && value.extensionsRequired.Count>0)"},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらです。https://github.com/KhronosGroup/glTF/blob/dae8fe778b01aaffdde9d69a6116e30af11e3a1d/specification/2.0/schema/glTF.schema.json#L17-L25

すでにfalse固定になっていますが、いちおう防御的に記述しておきます。

{"gltf/extensions/VRM/humanoid/humanBones[]/axisLength", "if(value.axisLength>0)"},
{"gltf/extensions/VRM/humanoid/humanBones[]/center", "if(value.center!=Vector3.zero)"},
{"gltf/extensions/VRM/humanoid/humanBones[]/max", "if(value.max!=Vector3.zero)"},
{"gltf/extensions/VRM/humanoid/humanBones[]/min", "if(value.min!=Vector3.zero)"},
{"gltf/textures", "if(value.textures!=null && value.textures.Count>0)" },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hiroj
Copy link
Contributor

hiroj commented Jul 27, 2020

LGTM

@hiroj hiroj merged commit 7385c89 into vrm-c:master Jul 27, 2020
@saturday06 saturday06 deleted the experimental_exporter_gltf_validation branch May 13, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants