MaterialChunk doesn't expose the Unflattener in its API

this makes the API more robust, because now it's not possible
to mess up the call to getShader(), by construction.

filament/src/MaterialParser.cpp

@@ -143,6 +143,9 @@ bool MaterialParser::parse() noexcept {
         if (!DictionaryReader::unflatten(cc, mImpl->mDictionaryTag, mImpl->mBlobDictionary)) {
             return false;
         }
+        if (!mImpl->mMaterialChunk.readIndex(mImpl->mMaterialTag)) {
+            return false;
+        }
     }
     return true;
 }
@@ -288,7 +291,7 @@ bool MaterialParser::getRequiredAttributes(AttributeBitset* value) const noexcep
 bool MaterialParser::getShader(ShaderBuilder& shader,
         ShaderModel shaderModel, uint8_t variant, ShaderType stage) noexcept {
     return mImpl->mMaterialChunk.getShader(shader,
-            mImpl->mMaterialTag, mImpl->mBlobDictionary, (uint8_t)shaderModel, variant, stage);
+            mImpl->mBlobDictionary, (uint8_t)shaderModel, variant, stage);
 }
 
 // ------------------------------------------------------------------------------------------------

libs/filaflat/include/filaflat/MaterialChunk.h

@@ -20,38 +20,42 @@
 
 #include <filament/MaterialChunkType.h>
 
-#include <filaflat/BlobDictionary.h>
-#include <filaflat/ShaderBuilder.h>
 #include <filaflat/Unflattener.h>
 
 #include <tsl/robin_map.h>
 
 namespace filaflat {
 
+class BlobDictionary;
 class ChunkContainer;
+class ShaderBuilder;
 
 class MaterialChunk {
 public:
     explicit MaterialChunk(ChunkContainer const& container);
 
+    // call this once after container.parse() has been called
+    bool readIndex(filamat::ChunkType materialTag);
+
+    // call this as many times as needed
     bool getShader(ShaderBuilder& shaderBuilder,
-            filamat::ChunkType materialTag, BlobDictionary const& dictionary,
+            BlobDictionary const& dictionary,
             uint8_t shaderModel, uint8_t variant, uint8_t stage);
 
 private:
     ChunkContainer const& mContainer;
+    filamat::ChunkType mMaterialTag = filamat::ChunkType::Unknown;
+    Unflattener mUnflattener{nullptr, nullptr};
+    const uint8_t* mBase = nullptr;
+    tsl::robin_map<uint32_t, uint32_t> mOffsets;
 
-    bool getTextShader(
-            Unflattener unflattener, BlobDictionary const& dictionary, ShaderBuilder& shaderBuilder,
+    bool getTextShader(Unflattener unflattener,
+            BlobDictionary const& dictionary, ShaderBuilder& shaderBuilder,
             uint8_t shaderModel, uint8_t variant, uint8_t stage);
 
     bool getSpirvShader(
-            Unflattener unflattener, BlobDictionary const& dictionary, ShaderBuilder& shaderBuilder,
+            BlobDictionary const& dictionary, ShaderBuilder& shaderBuilder,
             uint8_t shaderModel, uint8_t variant, uint8_t stage);
-
-    bool readIndex(Unflattener& unflattener);
-    const uint8_t* mBase = nullptr;
-    tsl::robin_map<uint32_t, uint32_t> mOffsets;
 };
 
 } // namespace filamat

libs/filaflat/src/MaterialChunk.cpp

@@ -15,7 +15,9 @@
  */
 
 #include <filaflat/MaterialChunk.h>
+#include <filaflat/BlobDictionary.h>
 #include <filaflat/ChunkContainer.h>
+#include <filaflat/ShaderBuilder.h>
 
 #include <utils/Log.h>
 
@@ -29,7 +31,19 @@ MaterialChunk::MaterialChunk(ChunkContainer const& container)
         : mContainer(container) {
 }
 
-bool MaterialChunk::readIndex(Unflattener& unflattener) {
+bool MaterialChunk::readIndex(filamat::ChunkType materialTag) {
+
+    if (mBase != nullptr) {
+        // readIndex() should be called only once.
+        return true;
+    }
+
+    Unflattener unflattener(
+            mContainer.getChunkStart(materialTag),
+            mContainer.getChunkEnd(materialTag));
+
+    mUnflattener = unflattener;
+    mMaterialTag = materialTag;
     mBase = unflattener.getCursor();
 
     // Read how many shaders we have in the chunk.
@@ -68,15 +82,13 @@ bool MaterialChunk::readIndex(Unflattener& unflattener) {
 }
 
 bool MaterialChunk::getTextShader(Unflattener unflattener, BlobDictionary const& dictionary,
-        ShaderBuilder& shader, uint8_t shaderModel, uint8_t variant, uint8_t ps) {
-
-    shader.reset();
-    if (mBase == nullptr ) {
-        if (!readIndex(unflattener)) {
-            return false;
-        }
+        ShaderBuilder& shaderBuilder, uint8_t shaderModel, uint8_t variant, uint8_t ps) {
+    if (mBase == nullptr) {
+        return false;
     }
 
+    shaderBuilder.reset();
+
     // Jump and read
     uint32_t key = makeKey(shaderModel, variant, ps);
     auto pos = mOffsets.find(key);
@@ -98,39 +110,39 @@ bool MaterialChunk::getTextShader(Unflattener unflattener, BlobDictionary const&
     }
 
     // Add an extra char for the null terminator.
-    shader.announce(shaderSize + 1);
+    shaderBuilder.announce(shaderSize + 1);
 
     // Read how many lines there are.
-    uint32_t numLines = 0;
-    if (!unflattener.read(&numLines)){
+    uint32_t lineCount = 0;
+    if (!unflattener.read(&lineCount)){
         return false;
     }
 
     // Read all lines.
-    for(int32_t i = 0 ; i < numLines; i++) {
+    for(int32_t i = 0 ; i < lineCount; i++) {
         uint16_t lineIndex;
         if (!unflattener.read(&lineIndex)) {
             return false;
         }
         const char* string = dictionary.getString(lineIndex);
-        shader.append(string, strlen(string));
-        shader.append("\n", 1);
+        shaderBuilder.append(string, strlen(string));
+        shaderBuilder.append("\n", 1);
     }
 
     // Write the terminating null character.
-    shader.append("", 1);
+    shaderBuilder.append("", 1);
 
     return true;
 }
 
 
-bool MaterialChunk::getSpirvShader(Unflattener unflattener, BlobDictionary const& dictionary,
-        ShaderBuilder& builder, uint8_t shaderModel, uint8_t variant, uint8_t stage) {
-    if (mBase == nullptr ) {
-        if (!readIndex(unflattener)) {
-            return false;
-        }
+bool MaterialChunk::getSpirvShader(BlobDictionary const& dictionary,
+        ShaderBuilder& shaderBuilder, uint8_t shaderModel, uint8_t variant, uint8_t stage) {
+
+    if (mBase == nullptr) {
+        return false;
     }
+
     uint32_t key = makeKey(shaderModel, variant, stage);
     auto pos = mOffsets.find(key);
     if (pos == mOffsets.end()) {
@@ -140,24 +152,21 @@ bool MaterialChunk::getSpirvShader(Unflattener unflattener, BlobDictionary const
     size_t index = pos->second;
     size_t shaderSize;
     const char* shaderContent = dictionary.getBlob(index, &shaderSize);
-    builder.reset();
-    builder.announce(shaderSize);
-    builder.append(shaderContent, shaderSize);
+
+    shaderBuilder.reset();
+    shaderBuilder.announce(shaderSize);
+    shaderBuilder.append(shaderContent, shaderSize);
     return true;
 }
 
-bool MaterialChunk::getShader(ShaderBuilder& shaderBuilder, filamat::ChunkType materialTag,
+bool MaterialChunk::getShader(ShaderBuilder& shaderBuilder,
         BlobDictionary const& dictionary, uint8_t shaderModel, uint8_t variant, uint8_t stage) {
-    Unflattener unflattener(
-            mContainer.getChunkStart(materialTag),
-            mContainer.getChunkEnd(materialTag));
-
-    switch (materialTag) {
+    switch (mMaterialTag) {
         case filamat::ChunkType::MaterialGlsl:
         case filamat::ChunkType::MaterialMetal:
-            return getTextShader(unflattener, dictionary, shaderBuilder, shaderModel, variant, stage);
+            return getTextShader(mUnflattener, dictionary, shaderBuilder, shaderModel, variant, stage);
         case filamat::ChunkType::MaterialSpirv:
-            return getSpirvShader(unflattener, dictionary, shaderBuilder, shaderModel, variant, stage);
+            return getSpirvShader(dictionary, shaderBuilder, shaderModel, variant, stage);
         default:
             return false;
     }

tools/matinfo/src/main.cpp

@@ -16,10 +16,11 @@
 
 #include <getopt/getopt.h>
 
+#include <filaflat/BlobDictionary.h>
 #include <filaflat/ChunkContainer.h>
+#include <filaflat/DictionaryReader.h>
 #include <filaflat/MaterialChunk.h>
 #include <filaflat/ShaderBuilder.h>
-#include <filaflat/DictionaryReader.h>
 #include <filaflat/Unflattener.h>
 
 #include <filament/MaterialChunkType.h>
@@ -50,8 +51,8 @@ static const int alignment = 24;
 class MaterialParser {
 public:
     MaterialParser(Backend backend, const void* data, size_t size)
-            : mBackend(backend), mChunkContainer(data, size), mMaterialChunk(mChunkContainer) {
-        switch (mBackend) {
+            : mChunkContainer(data, size), mMaterialChunk(mChunkContainer) {
+        switch (backend) {
             case Backend::OPENGL:
                 mMaterialTag = ChunkType::MaterialGlsl;
                 mDictionaryTag = ChunkType::DictionaryGlsl;
@@ -70,7 +71,10 @@ class MaterialParser {
     }
 
     bool parse() noexcept {
-        return mChunkContainer.parse();
+        if (mChunkContainer.parse()) {
+            return mMaterialChunk.readIndex(mMaterialTag);
+        }
+        return false;
     }
 
     bool isShadingMaterial() const noexcept {
@@ -100,13 +104,12 @@ class MaterialParser {
         }
 
         return mMaterialChunk.getShader(shader,
-                mMaterialTag, blobDictionary, (uint8_t)shaderModel, variant, stage);
+                blobDictionary, (uint8_t)shaderModel, variant, stage);
     }
 
 private:
     ChunkContainer mChunkContainer;
     MaterialChunk mMaterialChunk;
-    Backend mBackend;
     ChunkType mMaterialTag = ChunkType::Unknown;
     ChunkType mDictionaryTag = ChunkType::Unknown;
 };