Update glTF.CPP to 1.5.19 #34
Conversation
Update driectxtex to 2018.6.1.2
| @@ -131,13 +131,13 @@ | |||
| </ItemGroup> | |||
| <ItemGroup> | |||
| <PackageReference Include="Microsoft.NETCore.UniversalWindowsPlatform"> | |||
| <Version>6.0.7</Version> | |||
| <Version>6.2.0-Preview1-26502-02</Version> | |||
There was a problem hiding this comment.
Any reason why we're using a preview version here?
There was a problem hiding this comment.
I just pushed the update button in NuGet; there is no reason I needed this package updated.
| @@ -333,12 +346,13 @@ namespace | |||
|
|
|||
| for (auto &primitive : mesh.primitives) | |||
| { | |||
| AddIndexOffset(primitive.positionsAccessorId, accessorOffset); | |||
| AddIndexOffset(primitive.normalsAccessorId, accessorOffset); | |||
| AddIndexOffset(primitive.indicesAccessorId, accessorOffset); | |||
There was a problem hiding this comment.
Is this correct? or does it have to be changed as well?
There was a problem hiding this comment.
Indices is still its own field where empty string implies not present in the json. So no it doesn't need to change.
|
It would be great to get someone from glTF.CPP to double-check this one @chriche-ms @agrittmsft |
glTF-Toolkit/src/GLTFLODUtils.cpp
Outdated
| @@ -68,7 +78,7 @@ namespace | |||
| } | |||
|
|
|||
| template <typename T> | |||
| std::string SerializeExtensionMSFTLod(const T&, const std::vector<std::string>& lods, const GLTFDocument& gltfDocument) | |||
| std::string SerializeExtensionMSFTLod(const T&, const std::vector<std::string>& lods, const Document& Document) | |||
| localMaterial.specularGlossiness.diffuseFactor == globalMaterial.specularGlossiness.diffuseFactor && | ||
| localMaterial.specularGlossiness.glossinessFactor == globalMaterial.specularGlossiness.glossinessFactor && | ||
| localMaterial.specularGlossiness.specularFactor == globalMaterial.specularGlossiness.specularFactor; | ||
| localMaterial.HasExtension<KHR::Materials::PBRSpecularGlossiness>() == globalMaterial.HasExtension<KHR::Materials::PBRSpecularGlossiness>() && |
There was a problem hiding this comment.
What's the reason for not using PBRSpecularGlossiness.IsEqual(other)?
There was a problem hiding this comment.
Because the textureIds will/could be different in this comparison.
There was a problem hiding this comment.
Ok, in that case can we reduce the number of GetExtension calls to 2? One for localMaterial and one for globalMaterial.
There was a problem hiding this comment.
That would make this expression a lot more complex (looking).; I'll leave the optimization up to the compiler.
|
Thanks for fixing all these breaking changes from our glTF SDK updates @najadojo! |
| inline void AddIndexOffset(MeshPrimitive& primitive, const char* attributeName, size_t offset) | ||
| { | ||
| // an empty id string indicates that the id is not inuse and therefore should not be updated | ||
| if (primitive.HasAttribute(attributeName)) |
There was a problem hiding this comment.
I'm not sure if it's available in the SDK version you have but MeshPrimitive::TryGetAttributeAccessorId may be better than HasAttribute followed by a find (if available)
| AddIndexOffset(material.specularGlossiness.specularGlossinessTextureId, texturesOffset); | ||
| if (material.HasExtension<KHR::Materials::PBRSpecularGlossiness>()) | ||
| { | ||
| AddIndexOffset(material.GetExtension<KHR::Materials::PBRSpecularGlossiness>().diffuseTexture.textureId, texturesOffset); |
There was a problem hiding this comment.
Have you considered storing the result of GetExtension in a local variable rather than calling the function repeatedly?
|
Superceded by #36 |
Update driectxtex to 2018.6.1.2
No functional changes.