From 359a89b5687952782437f2ea25f92f4574b4226f Mon Sep 17 00:00:00 2001
From: Patrick Walton <pcwalton@mimiga.net>
Date: Sun, 18 Feb 2024 01:07:29 -0800
Subject: [PATCH] Fix noncomformant behavior by not requiring the `source`
 field to be present in textures.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It's perfectly legal for `source` to not be present. Typically, this
will happen when an extension such as `KHR_texture_basisu` provides the
actual source.

The glTF spec § 5.29 states unambiguously that `source` is an optional
field [1]. The validator backs this up (see the `blank_texture.gltf`
test case, which validates). Furthermore, the `KHR_texture_basisu`
extension provides the following example in "Using without a fallback"
[2]:

```json
"textures": [
{
    "extensions": {
	"KHR_texture_basisu": {
	    "source": 0
	}
    }
}
],
```

This doesn't contain a `source` field. Parsing this today with the
`gltf` crate causes a panic in `serde`.

I ran into this with the `gltf-transform` [3] tool, which follows the
standard when compressing textures and therefore produces valid glTF
files that the `gltf` crate panics when attempting to load.

[1]: https://registry.khronos.org/glTF/specs/2.0/glTF-2.0.html#reference-texture

[2]: https://github.com/KhronosGroup/glTF/blob/main/extensions/2.0/Khronos/KHR_texture_basisu/README.md#using-without-a-fallback

[3]: https://github.com/donmccurdy/glTF-Transform
---
 gltf-json/src/texture.rs     |  3 ++-
 src/texture.rs               | 12 ++++++------
 tests/blank_texture.gltf     |  8 ++++++++
 tests/import_sanity_check.rs | 13 ++++++++++++-
 4 files changed, 28 insertions(+), 8 deletions(-)
 create mode 100644 tests/blank_texture.gltf

diff --git a/gltf-json/src/texture.rs b/gltf-json/src/texture.rs
index 837253c5..cf03e2e0 100644
--- a/gltf-json/src/texture.rs
+++ b/gltf-json/src/texture.rs
@@ -179,7 +179,8 @@ pub struct Texture {
     pub sampler: Option<Index<Sampler>>,
 
     /// The index of the image used by this texture.
-    pub source: Index<image::Image>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub source: Option<Index<image::Image>>,
 
     /// Extension specific data.
     #[serde(default, skip_serializing_if = "Option::is_none")]
diff --git a/src/texture.rs b/src/texture.rs
index 3aab4e07..b98a0e60 100644
--- a/src/texture.rs
+++ b/src/texture.rs
@@ -157,12 +157,12 @@ impl<'a> Texture<'a> {
             .unwrap_or_else(|| Sampler::default(self.document))
     }
 
-    /// Returns the image used by this texture.
-    pub fn source(&self) -> image::Image<'a> {
-        self.document
-            .images()
-            .nth(self.json.source.value())
-            .unwrap()
+    /// Returns the image used by this texture, if any.
+    pub fn source(&self) -> Option<image::Image<'a>> {
+        self.json
+            .source
+            .as_ref()
+            .map(|index| self.document.images().nth(index.value()).unwrap())
     }
 
     /// Returns extension data unknown to this crate version.
diff --git a/tests/blank_texture.gltf b/tests/blank_texture.gltf
new file mode 100644
index 00000000..adf8194a
--- /dev/null
+++ b/tests/blank_texture.gltf
@@ -0,0 +1,8 @@
+{
+  "asset": {
+    "version": "2.0"
+  },
+  "textures": [
+    {}
+  ]
+}
\ No newline at end of file
diff --git a/tests/import_sanity_check.rs b/tests/import_sanity_check.rs
index 7714fa75..ea372e26 100644
--- a/tests/import_sanity_check.rs
+++ b/tests/import_sanity_check.rs
@@ -60,7 +60,8 @@ fn run() -> Result<(), Box<dyn StdError>> {
         }
     }
 
-    sparse_accessor_without_buffer_view_test()
+    sparse_accessor_without_buffer_view_test()?;
+    blank_texture_test()
 }
 
 /// Test a file with a sparse accessor with no buffer view
@@ -79,6 +80,16 @@ fn sparse_accessor_without_buffer_view_test() -> Result<(), Box<dyn StdError>> {
     Ok(())
 }
 
+/// Test a file with a texture with neither sampler nor source
+fn blank_texture_test() -> Result<(), Box<dyn StdError>> {
+    let gltf_path = path::Path::new("tests/blank_texture.gltf");
+    print!("{:?}: ", gltf_path);
+    let result = gltf::import(gltf_path)?;
+    sanity_check(&result.0, &result.1, &result.2);
+    println!("ok");
+    Ok(())
+}
+
 #[test]
 fn import_sanity_check() {
     assert!(run().is_ok());