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

Additional PBR textures, metallic/roughness texture + tangents #12

Merged
merged 3 commits into from
May 1, 2024

Conversation

mrehayden1
Copy link
Contributor

No description provided.

@mrehayden1
Copy link
Contributor Author

The tests in test/Text.GLTF.LoaderSpec.hs are still failing. Could use your help with these as I don't have access to the original blender project / whatever you used to generate the .glb/.gltf files.

@sgillespie
Copy link
Owner

I don't have access to the original blender project / whatever you used to generate the .glb/.gltf files.

Sorry, haven't had a chance to look yet, but these are basically just the blender default starter project with a cube. You could import the gltf files into blender and look at the differences.

@sgillespie
Copy link
Owner

This seems to make the tests pass:

diff --git i/test/Text/GLTF/LoaderSpec.hs w/test/Text/GLTF/LoaderSpec.hs
index 1aef6a8..1ee83b7 100644
--- i/test/Text/GLTF/LoaderSpec.hs
+++ w/test/Text/GLTF/LoaderSpec.hs
@@ -84,7 +84,10 @@ expectedMaterials =
         materialAlphaMode = Opaque,
         materialDoubleSided = True,
         materialEmissiveFactor = V3 0.0 0.0 0.0,
+        materialEmissiveTexture = Nothing,
         materialName = Just "Material",
+        materialNormalTexture = Nothing,
+        materialOcclusionTexture = Nothing,
         materialPbrMetallicRoughness =
           Just
             $ PbrMetallicRoughness
@@ -96,7 +99,8 @@ expectedMaterials =
                         textureTexCoord = 0
                       },
                 pbrMetallicFactor = 0.0,
-                pbrRoughnessFactor = 0.5
+                pbrRoughnessFactor = 0.5,
+                pbrMetallicRoughnessTexture = Nothing
               }
       }
   ]
@@ -198,6 +202,7 @@ expectedMeshes =
                     V3 0.0 (-1.0) (-0.0),
                     V3 0.0 0.0 1.0
                   ],
+                meshPrimitiveTangents = [],
                 meshPrimitiveTexCoords =
                   [ V2 0.625 0.5,
                     V2 0.625 0.5,

I'm still considering how far I want to take these test files generated from blender. On the one hand it's nice to verify we can actually parse an exported project, but on the other I'm not sure how much more this will scale.

@mrehayden1
Copy link
Contributor Author

Automated tests are great and all, but I find the cost of maintaining them to be quite high.

Is there possibly a way we could use quickcheck?

I'm quite happy to e2e test the features while they're relevant to me otherwise, maybe you could release pre-release versions for testing bleeding edge features with?

@sgillespie
Copy link
Owner

Is there possibly a way we could use quickcheck?

I would love to use property tests, but I haven't been able to wrap my head around how I could do this with https://hackage.haskell.org/package/gltf-codec.

@sgillespie
Copy link
Owner

Automated tests are great and all, but I find the cost of maintaining them to be quite high.

As I said before, I'm not sure I like the Blender tests test/Text/GLTF/LoaderSpec.hs. I'm inclined to keep them as simple as possible, and rely on the unit tests (test/Text/GLTF/Loader/Internal/*.hs) for anything else.

@sgillespie sgillespie merged commit 13f7ab9 into sgillespie:main May 1, 2024
1 check failed
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