From cdbc3f2eb0dd16b62a1021e88b6e6f2ca05dfb38 Mon Sep 17 00:00:00 2001 From: treeform Date: Sat, 15 Aug 2026 12:04:10 -0700 Subject: [PATCH] Flag reader placeholder maps instead of guessing them by size writeGLB skipped any material map that was 1x1 or smaller, on the theory that such images are the 1x1 fills the reader substitutes for empty texture slots. That guess is wrong for a genuinely 1x1 texture the caller supplied, which was silently dropped instead of written -- breaking the external and KTX2 image-mode tests, and CI on master since e33ca75. The reader is the only thing that creates those fills, so let it say so: each Material texture slot gains a `*Placeholder` flag, set where the fill is made, and the writer consults the flag rather than the dimensions. The flag defaults to false, so hand-built materials keep writing their textures no matter how small they are. Adds a test that a material read with no textures round-trips through writeGLB without gaining any, which e33ca75 changed behavior for but never covered. Co-Authored-By: Claude Opus 5 --- src/gltf/common.nim | 9 +++++++ src/gltf/reader.nim | 10 ++++++++ src/gltf/writer.nim | 24 +++++++++++-------- tests/tests.nim | 58 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/src/gltf/common.nim b/src/gltf/common.nim index ded4678..a05378d 100644 --- a/src/gltf/common.nim +++ b/src/gltf/common.nim @@ -179,6 +179,10 @@ type channels*: seq[AnimationChannel] Material* = ref object + ## Texture slots the source file left empty are filled with a 1x1 + ## constant image so renderers always have something to sample. Those + ## fills are marked with the matching `*Placeholder` flag, since a 1x1 + ## image is otherwise indistinguishable from a real one that small. name*: string baseColor*: Image baseColorKtx2*: string @@ -186,6 +190,7 @@ type baseColorTransform*: TextureTransform baseColorSampler*: TextureSampler baseColorFactor*: Color + baseColorPlaceholder*: bool metallicRoughness*: Image metallicRoughnessKtx2*: string metallicRoughnessName*: string @@ -193,6 +198,7 @@ type metallicRoughnessSampler*: TextureSampler metallicFactor*: float32 roughnessFactor*: float32 + metallicRoughnessPlaceholder*: bool normal*: Image normalKtx2*: string normalName*: string @@ -200,18 +206,21 @@ type normalSampler*: TextureSampler hasNormalTexture*: bool normalScale*: float32 + normalPlaceholder*: bool occlusion*: Image occlusionKtx2*: string occlusionName*: string occlusionTransform*: TextureTransform occlusionSampler*: TextureSampler occlusionStrength*: float32 + occlusionPlaceholder*: bool emissive*: Image emissiveKtx2*: string emissiveName*: string emissiveTransform*: TextureTransform emissiveSampler*: TextureSampler emissiveFactor*: Color + emissivePlaceholder*: bool alphaMode*: AlphaMode alphaCutoff*: float32 diff --git a/src/gltf/reader.nim b/src/gltf/reader.nim index ddfa32e..3aa407b 100644 --- a/src/gltf/reader.nim +++ b/src/gltf/reader.nim @@ -1193,6 +1193,7 @@ proc defaultRuntimeMaterial(): Material = result.baseColor = newImage(1, 1) result.baseColor.fill(rgbx(255, 255, 255, 255)) + result.baseColorPlaceholder = true result.baseColorFactor = color(1, 1, 1, 1) result.baseColorTransform = TextureTransform( texCoord: 0, @@ -1203,6 +1204,7 @@ proc defaultRuntimeMaterial(): Material = result.metallicRoughness = newImage(1, 1) result.metallicRoughness.fill(rgbx(255, 255, 255, 255)) + result.metallicRoughnessPlaceholder = true result.metallicFactor = 1.0 result.roughnessFactor = 1.0 result.metallicRoughnessTransform = TextureTransform( @@ -1214,6 +1216,7 @@ proc defaultRuntimeMaterial(): Material = result.normal = newImage(1, 1) result.normal.fill(rgbx(128, 128, 255, 255)) + result.normalPlaceholder = true result.hasNormalTexture = false result.normalScale = 1.0 result.normalTransform = TextureTransform( @@ -1225,6 +1228,7 @@ proc defaultRuntimeMaterial(): Material = result.occlusion = newImage(1, 1) result.occlusion.fill(rgbx(255, 255, 255, 255)) + result.occlusionPlaceholder = true result.occlusionStrength = 1.0 result.occlusionTransform = TextureTransform( texCoord: 0, @@ -1235,6 +1239,7 @@ proc defaultRuntimeMaterial(): Material = result.emissive = newImage(1, 1) result.emissive.fill(rgbx(255, 255, 255, 255)) + result.emissivePlaceholder = true result.emissiveFactor = color(0, 0, 0, 1) result.emissiveTransform = TextureTransform( texCoord: 0, @@ -1335,6 +1340,7 @@ proc loadPrimitive( else: result.material.baseColor = newImage(1, 1) result.material.baseColor.fill(rgbx(255, 255, 255, 255)) + result.material.baseColorPlaceholder = true result.material.baseColorTransform = TextureTransform( texCoord: pbr.baseColorTexture.texCoord, offset: pbr.baseColorTexture.offset, @@ -1353,6 +1359,7 @@ proc loadPrimitive( else: result.material.metallicRoughness = newImage(1, 1) result.material.metallicRoughness.fill(rgbx(255, 255, 255, 255)) + result.material.metallicRoughnessPlaceholder = true result.material.metallicRoughnessTransform = TextureTransform( texCoord: pbr.metallicRoughnessTexture.texCoord, offset: pbr.metallicRoughnessTexture.offset, @@ -1374,6 +1381,7 @@ proc loadPrimitive( else: result.material.normal = newImage(1, 1) result.material.normal.fill(rgbx(128, 128, 255, 255)) + result.material.normalPlaceholder = true result.material.hasNormalTexture = false result.material.normalScale = 1.0 result.material.normalTransform = TextureTransform( @@ -1393,6 +1401,7 @@ proc loadPrimitive( else: result.material.occlusion = newImage(1, 1) result.material.occlusion.fill(rgbx(255, 255, 255, 255)) + result.material.occlusionPlaceholder = true result.material.occlusionTransform = TextureTransform( texCoord: material.occlusionTexture.texCoord, offset: material.occlusionTexture.offset, @@ -1411,6 +1420,7 @@ proc loadPrimitive( else: result.material.emissive = newImage(1, 1) result.material.emissive.fill(rgbx(255, 255, 255, 255)) + result.material.emissivePlaceholder = true result.material.emissiveTransform = TextureTransform( texCoord: material.emissiveTexture.texCoord, offset: material.emissiveTexture.offset, diff --git a/src/gltf/writer.nim b/src/gltf/writer.nim index debeb40..f1c582a 100644 --- a/src/gltf/writer.nim +++ b/src/gltf/writer.nim @@ -293,11 +293,12 @@ proc writeGLB*( textureIds[key] = idx idx - proc isPlaceholder(img: Image): bool = - ## The reader substitutes 1x1 fill images for missing material maps; - ## writing those back out would add meaningless textures (the factor - ## values already carry the constant). - img == nil or (img.width <= 1 and img.height <= 1) + proc isPlaceholder(img: Image, placeholder: bool): bool = + ## The reader substitutes 1x1 fill images for missing material maps and + ## flags them; writing those back out would add meaningless textures (the + ## factor values already carry the constant). The flag is what decides: + ## a 1x1 image the caller supplied is a real texture and must be written. + img == nil or placeholder proc materialIndex(mat: Material): int = ## Returns the output material index for a material. @@ -321,13 +322,15 @@ proc writeGLB*( # Images embedded in a .glb often carry no name; fall back to the # semantic so unnamed textures still round-trip instead of vanishing. - if not isPlaceholder(mat.baseColor): + if not isPlaceholder(mat.baseColor, mat.baseColorPlaceholder): let name = if mat.baseColorName.len > 0: mat.baseColorName else: "baseColor" let texIdx = textureIndex(mat.baseColor, name, tsColor) pbr["baseColorTexture"] = %*{"index": texIdx} - if not isPlaceholder(mat.metallicRoughness): + if not isPlaceholder( + mat.metallicRoughness, mat.metallicRoughnessPlaceholder + ): let name = if mat.metallicRoughnessName.len > 0: mat.metallicRoughnessName @@ -339,7 +342,8 @@ proc writeGLB*( matNode["pbrMetallicRoughness"] = pbr matNode["doubleSided"] = newJBool(mat.doubleSided) - if not isPlaceholder(mat.normal) and mat.hasNormalTexture: + if not isPlaceholder(mat.normal, mat.normalPlaceholder) and + mat.hasNormalTexture: let name = if mat.normalName.len > 0: mat.normalName else: "normal" let texIdx = textureIndex(mat.normal, name, tsNormal) matNode["normalTexture"] = %*{ @@ -347,7 +351,7 @@ proc writeGLB*( "scale": mat.normalScale } - if not isPlaceholder(mat.occlusion): + if not isPlaceholder(mat.occlusion, mat.occlusionPlaceholder): let name = if mat.occlusionName.len > 0: mat.occlusionName else: "occlusion" let texIdx = textureIndex(mat.occlusion, name, tsData) matNode["occlusionTexture"] = %*{ @@ -355,7 +359,7 @@ proc writeGLB*( "strength": mat.occlusionStrength } - if not isPlaceholder(mat.emissive): + if not isPlaceholder(mat.emissive, mat.emissivePlaceholder): let name = if mat.emissiveName.len > 0: mat.emissiveName else: "emissive" let texIdx = textureIndex(mat.emissive, name, tsColor) matNode["emissiveTexture"] = %*{"index": texIdx} diff --git a/tests/tests.nim b/tests/tests.nim index c89e477..28aaeea 100644 --- a/tests/tests.nim +++ b/tests/tests.nim @@ -368,6 +368,64 @@ doAssert embeddedModel.root.nodes.len == 1 let embeddedPrimitive = embeddedModel.root.nodes[0].mesh.primitives[0] doAssert embeddedPrimitive.material.baseColorName == "named_diffuse.png" +echo "Testing placeholder maps are not written back." +# The reader fills empty texture slots with 1x1 constant images; writing +# those out would add meaningless textures the source file never had. +let + placeholderDir = joinPath(tmpDir, "out_placeholder") + untexturedGltfPath = joinPath(placeholderDir, "untextured.gltf") + untexturedBufferPath = joinPath(placeholderDir, "untextured.bin") + placeholderPath = joinPath(placeholderDir, "placeholder.glb") +createDir(placeholderDir) +writeBytes( + untexturedBufferPath, + @[ + 0x00'u8, 0x00, 0x00, 0x00, + 0x00'u8, 0x00, 0x00, 0x00, + 0x00'u8, 0x00, 0x00, 0x00 + ] +) +writeFile( + untexturedGltfPath, + $(%*{ + "asset": {"version": "2.0"}, + "buffers": [{"byteLength": 12, "uri": "untextured.bin"}], + "bufferViews": [{"buffer": 0, "byteOffset": 0, "byteLength": 12}], + "accessors": [ + {"bufferView": 0, "componentType": 5126, "count": 1, "type": "VEC3"} + ], + "materials": [{"pbrMetallicRoughness": {}}], + "meshes": [ + {"primitives": [{"attributes": {"POSITION": 0}, "material": 0}]} + ], + "nodes": [{"name": "Untextured", "mesh": 0}], + "scenes": [{"nodes": [0]}], + "scene": 0 + }) +) + +let untexturedModel = readGltfFile(untexturedGltfPath) +let untexturedMat = + untexturedModel.root["Untextured"].mesh.primitives[0].material +doAssert untexturedMat.baseColorPlaceholder +doAssert untexturedMat.metallicRoughnessPlaceholder +doAssert untexturedMat.normalPlaceholder +doAssert untexturedMat.occlusionPlaceholder +doAssert untexturedMat.emissivePlaceholder + +writeGLB(untexturedModel.root, placeholderPath, iwmExternal) +doAssert fileExists(placeholderPath) +# A written texture would land next to the .glb as a sidecar image. +for kind, path in walkDir(placeholderDir): + doAssert path.extractFilename() in [ + "untextured.gltf", "untextured.bin", "placeholder.glb" + ], "placeholder written as a texture: " & path +let placeholderModel = readGltfFile(placeholderPath) +let placeholderMat = + placeholderModel.root["Untextured"].mesh.primitives[0].material +doAssert placeholderMat.baseColorPlaceholder +doAssert placeholderMat.emissivePlaceholder + echo "Testing EXT_texture_webp source selection." let webpOutDir = joinPath(tmpDir, "out_webp")