-
Notifications
You must be signed in to change notification settings - Fork 417
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
[1.0] Migration の修正 #1305
[1.0] Migration の修正 #1305
Conversation
ousttrue
commented
Oct 14, 2021
- Migration の UnitTest でマイグレーション後ロードするようにした
- SpringBone の nullcheck(bones key)
- 頂点カラーなどが含まれるモデル
- TANGENT, COLOR_0, TEXCOORD_1 の処理
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
よさそう。
気になった点はあるが、コーディングスタイルの好みの範疇なのでどちらでも
var data = new GlbLowLevelParser(gltf.FullName, migrated).Parse(); | ||
UniGLTF.Extensions.VRMC_vrm.GltfDeserializer.TryGet(data.GLTF.extensions, out UniGLTF.Extensions.VRMC_vrm.VRMC_vrm vrm); | ||
Assert.NotNull(vrm); | ||
Vrm10Data.TryParseOrMigrate(gltf.FullName, true, out Vrm10Data vrm); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
結果の bool を Assert してもいいかも?(まあ要らないか)
@@ -129,10 +129,13 @@ public static UniGLTF.Extensions.VRMC_springBone.VRMC_springBone Migrate(UniGLTF | |||
// その差異に対応して、7cmの遠さに node を追加する。 | |||
foreach (var x in sa["boneGroups"].ArrayItems()) | |||
{ | |||
foreach (var y in x["bones"].ArrayItems()) | |||
if (x.ContainsKey("bones")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (!x.ContainsKey("bones")) continue;
のほうがインデントは少なくて見通しはよさそう(好みによる)
@@ -163,30 +166,33 @@ public static UniGLTF.Extensions.VRMC_springBone.VRMC_springBone Migrate(UniGLTF | |||
// 5 | |||
// ] | |||
// }, | |||
foreach (var y in x["bones"].ArrayItems()) | |||
if (x.ContainsKey("bones")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
おなじく
@@ -82,6 +82,7 @@ struct MorphAccessor | |||
var uv = AddAccessor<Vector2>(mesh.VertexBuffer.TexCoords); | |||
var weights = AddAccessor<Vector4>(mesh.VertexBuffer.Weights); | |||
var joints = AddAccessor<UShort4>(mesh.VertexBuffer.Joints); | |||
var color = AddAccessor<Vector4>(mesh.VertexBuffer.Colors); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
頂点カラー考慮
@@ -99,13 +100,17 @@ struct MorphAccessor | |||
gltfPrim.indices = AddAccessor(subIndices); | |||
gltfPrim.attributes.POSITION = position.Value; | |||
gltfPrim.attributes.NORMAL = normal.Value; | |||
gltfPrim.attributes.TANGENT = -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
明示的に代入しないと何かが起こっていた?
default
の 0
になってたりしてたのかな?
uint?
とか null 許容型をとるのが C# らしくはありそうだが…境界領域だから optional は -1
としたほうが取り回しは楽か
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migration する前の変更する前の値が残ってました
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index(というかint, float, bool)を使うところは全部 Nullable の方がよいのだけど、作ったときはその知見が無かった。