gfx: ISF/CSF shader port-routing + codegen coherence (PR #2050)

model/renderer port drift for storage/image inputs and synthesized-int uniform shift across visitors, renderer port-advance and GLSL codegen (filter#2,3,11, avnd#6,7); ShaderSource hash/== (filter#4); save-without-document guard (filter#8); CSF oversized-workgroup continue + per-pass barrier (rhi#10,15).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: jcelerier

src/plugins/score-plugin-gfx/3rdparty/libisf/src/isf.cpp

@@ -4041,11 +4041,41 @@ void parser::parse_raw_raster_pipeline()
       uniforms += "layout(std140, binding = 2) uniform material_t {\n";
       for(const isf::input& val : d.inputs)
       {
-        // Storage buffers / storage images / geometry inputs / UBOs declared
-        // separately after samplers — skip them here.
-        if(ossia::get_if<isf::storage_input>(&val.data)
-           || ossia::get_if<isf::csf_image_input>(&val.data)
-           || ossia::get_if<isf::geometry_input>(&val.data)
+        // Storage buffers / storage images / geometry inputs / UBOs are declared
+        // separately after samplers. BUT their synthesized host-side size ints
+        // (storage flex-array size, geometry $USER counts) ARE packed into this
+        // material blob, so they must be declared here too — otherwise every
+        // uniform after them reads shifted. Mirrors the CSF Params block.
+        if(auto* storage = ossia::get_if<isf::storage_input>(&val.data))
+        {
+          if(storage->access.find("write") != std::string::npos
+             && !storage->layout.empty()
+             && storage->layout.back().type.find("[]") != std::string::npos)
+          {
+            num_uniform++;
+            uniforms += "int " + val.name + "_size;\n";
+            globalvars += "int " + val.name + "_size = isf_material_uniforms."
+                          + val.name + "_size;\n";
+          }
+          continue;
+        }
+        if(auto* geo = ossia::get_if<isf::geometry_input>(&val.data))
+        {
+          auto emit_synth_int = [&](const std::string& nm) {
+            num_uniform++;
+            uniforms += "int " + nm + ";\n";
+            globalvars += "int " + nm + " = isf_material_uniforms." + nm + ";\n";
+          };
+          if(geo->vertex_count.find("$USER") != std::string::npos)
+            emit_synth_int(val.name + "_vertex_count");
+          if(geo->instance_count.find("$USER") != std::string::npos)
+            emit_synth_int(val.name + "_instance_count");
+          for(const auto& aux : geo->auxiliary)
+            if(aux.size.find("$USER") != std::string::npos)
+              emit_synth_int(val.name + "_" + aux.name + "_size");
+          continue;
+        }
+        if(ossia::get_if<isf::csf_image_input>(&val.data)
            || ossia::get_if<isf::uniform_input>(&val.data))
           continue;
 
@@ -5792,6 +5822,20 @@ void parser::parse_csf()
           }
         }
       }
+      else if(auto* storage = ossia::get_if<storage_input>(&inp.data))
+      {
+        // A writable storage buffer whose LAYOUT ends in a flexible-array
+        // member gets a synthesized host-side size int (see ISFVisitors /
+        // RenderedCSFNode). Declare it here so this std140 block matches the
+        // packed material blob; otherwise every uniform after it reads shifted.
+        if(storage->access.find("write") != std::string::npos
+           && !storage->layout.empty()
+           && storage->layout.back().type.find("[]") != std::string::npos)
+        {
+          k++;
+          material_block += "    int " + inp.name + "_size;\n";
+        }
+      }
     }
 
     material_block += "};\n\n";

src/plugins/score-plugin-gfx/Gfx/CSF/Process.cpp

@@ -485,15 +485,23 @@ void Model::setupCSF(const isf::descriptor& desc)
             QString::fromStdString(input.name), Id<Process::Port>(output_i++), &self);
         self.m_outlets.push_back(port);
 
-        auto size_inl = new Process::IntSpinBox{
-            1,
-            536870911,
-            1024,
-            QString::fromStdString(input.name) + " size",
-            Id<Process::Port>(input_i++),
-            &self};
-        self.m_inlets.push_back(size_inl);
-        self.controlAdded(size_inl->id());
+        // Only writable buffers whose layout ends in a flexible-array member
+        // get a synthesized "size" inlet — this MUST match the renderer
+        // (isf_input_port_count_vis / isf_input_port_vis) and the generated
+        // GLSL, or every later control routes to the wrong port.
+        if(!v.layout.empty()
+           && v.layout.back().type.find("[]") != std::string::npos)
+        {
+          auto size_inl = new Process::IntSpinBox{
+              1,
+              536870911,
+              1024,
+              QString::fromStdString(input.name) + " size",
+              Id<Process::Port>(input_i++),
+              &self};
+          self.m_inlets.push_back(size_inl);
+          self.controlAdded(size_inl->id());
+        }
       }
     }
 
@@ -651,9 +659,17 @@ Process::Descriptor ProcessFactory::descriptor(QString) const noexcept
 template <>
 void DataStreamReader::read(const Gfx::CSF::Model& proc)
 {
-  auto& ctx = score::IDocument::documentContext(proc);
-  m_stream << proc.m_compute
-           << score::relativizeFilePath(proc.m_scriptPath, ctx);
+  // documentContext() SCORE_ASSERTs when the model isn't in a document
+  // (e.g. saving a template / copy). Only relativize against the document
+  // when there's an actual script path to relativize — mirrors the
+  // JSON/load guards. The empty case writes an empty path verbatim.
+  QString relativeScriptPath;
+  if(!proc.m_scriptPath.isEmpty())
+  {
+    auto& ctx = score::IDocument::documentContext(proc);
+    relativeScriptPath = score::relativizeFilePath(proc.m_scriptPath, ctx);
+  }
+  m_stream << proc.m_compute << relativeScriptPath;
   readPorts(*this, proc.m_inlets, proc.m_outlets);
 
   insertDelimiter();

src/plugins/score-plugin-gfx/Gfx/Filter/Process.cpp

@@ -209,9 +209,17 @@ void DataStreamWriter::write(Gfx::ShaderSource& p)
 template <>
 void DataStreamReader::read(const Gfx::Filter::Model& proc)
 {
-  auto& ctx = score::IDocument::documentContext(proc);
-  m_stream << proc.m_program
-           << score::relativizeFilePath(proc.m_scriptPath, ctx);
+  // documentContext() SCORE_ASSERTs when the model isn't in a document
+  // (e.g. saving a template / copy). Only relativize against the document
+  // when there's an actual script path to relativize — mirrors the
+  // JSON/load guards. The empty case writes an empty path verbatim.
+  QString relativeScriptPath;
+  if(!proc.m_scriptPath.isEmpty())
+  {
+    auto& ctx = score::IDocument::documentContext(proc);
+    relativeScriptPath = score::relativizeFilePath(proc.m_scriptPath, ctx);
+  }
+  m_stream << proc.m_program << relativeScriptPath;
 
   readPorts(*this, proc.m_inlets, proc.m_outlets);
 

src/plugins/score-plugin-gfx/Gfx/Graph/ISFVisitors.hpp

@@ -199,7 +199,13 @@ struct isf_input_size_vis
   // CSF-specific input handlers
   void operator()(const isf::storage_input& in) noexcept
   {
-    if(in.access.contains("write"))
+    // Must match what isf_input_port_vis (ISFNode.cpp) actually writes into the
+    // blob — and the synthesized "size" int it creates: ONLY a writable buffer
+    // whose layout ends in a flexible-array member. Reserving for every write
+    // buffer over-allocated the UBO (harmless, but desynced from the port
+    // visitor and the generated GLSL Params/material_t block).
+    if(in.access.contains("write") && !in.layout.empty()
+       && in.layout.back().type.find("[]") != std::string::npos)
     {
       (*this)(isf::long_input{});
     }
@@ -215,11 +221,10 @@ struct isf_input_size_vis
 
   void operator()(const isf::csf_image_input& in) noexcept
   {
-    if(in.access.contains("write"))
-    {
-      (*this)(isf::point2d_input{});
-      (*this)(isf::long_input{});
-    }
+    // isf_input_port_vis does NOT write anything into the material blob for
+    // write csf_image inputs (its point2d/long synthesis is commented out), so
+    // reserve nothing here — keep the size visitor and the port visitor (and
+    // hence the generated uniform block) in agreement.
   }
 
   void operator()(const isf::geometry_input& in) noexcept

src/plugins/score-plugin-gfx/Gfx/Graph/RenderedCSFNode.cpp

@@ -2970,7 +2970,7 @@ void RenderedCSFNode::buildComputeSrbBindings(
   for(const auto& input : n.m_descriptor.inputs)
   {
     // Storage buffers
-    if(ossia::get_if<isf::storage_input>(&input.data))
+    if(auto* storage_in = ossia::get_if<isf::storage_input>(&input.data))
     {
       // Find the corresponding storage buffer
       auto it = std::find_if(m_storageBuffers.begin(), m_storageBuffers.end(),
@@ -3026,6 +3026,20 @@ void RenderedCSFNode::buildComputeSrbBindings(
                      << QString::fromStdString(input.name);
         bindingIndex++;
       }
+
+      // Write-access buffers whose layout ends in a flexible-array member get a
+      // synthesized "size" INPUT port on the model (setupCSF / isf_input_port_-
+      // vis). The read_only branch advanced input_port_index for its own inlet,
+      // but the write branches above only touched output_port_index — so this
+      // sizing inlet was never skipped and every later storage input resolved
+      // the wrong port (its upstream buffer silently never bound). The geometry
+      // branch already does the equivalent for its $USER ports. Advance here
+      // under the SAME flex-array condition used everywhere else.
+      if(storage_in->access.contains("write") && !storage_in->layout.empty()
+         && storage_in->layout.back().type.find("[]") != std::string::npos)
+      {
+        input_port_index++;
+      }
     }
     // Regular textures (sampled)
     else if(ossia::get_if<isf::texture_input>(&input.data))
@@ -4653,11 +4667,14 @@ void RenderedCSFNode::runInitialPasses(
 
       if(totalWorkgroups > maxWorkgroups * maxWorkgroups * maxWorkgroups)
       {
-        // Workgroup count overflow: skip this pass. We haven't yet
+        // Workgroup count overflow: skip THIS pass only. We haven't yet
         // opened a compute pass at this point (the begin/end for this
         // dispatch is now hoisted *after* the size calculation), so
-        // there is nothing to close — just bail to the next pass.
-        return;
+        // there is nothing to close — continue to the next pass. Using
+        // `return` here aborted every remaining pass and desynced the
+        // ping-pong buffer swaps; mirror the dispatch(0,0,0) guard below
+        // which already uses `continue`.
+        continue;
       }
       if(totalWorkgroups > maxWorkgroups * maxWorkgroups)
       {
@@ -4715,22 +4732,20 @@ void RenderedCSFNode::runInitialPasses(
           pass.processUBO, 0, sizeof(ProcessUBO), &n.standardUBO);
     }
 
-    // Begin compute pass with ExternalContent flag so we can insert
-    // native memory barriers between dispatches via beginExternal/endExternal.
-    commands.beginComputePass(res, QRhiCommandBuffer::BeginPassFlag::ExternalContent);
+    // Each CSF pass issues exactly ONE dispatch in its own begin/endComputePass.
+    // QRhi automatically inserts the compute→compute memory barrier between
+    // consecutive passes that touch the same SSBO/image, so the previous
+    // per-pass ExternalContent flag + native barrier was redundant here — and
+    // ExternalContent needlessly forced Vulkan secondary command buffers. The
+    // native-barrier path stays for the genuinely multi-dispatch scatter loop
+    // (above), which issues several dispatches inside a single pass.
+    commands.beginComputePass(res);
     res = nullptr;
 
     commands.setComputePipeline(pass.pipeline);
     commands.setShaderResources(pass.srb);
     commands.dispatch(dispatchX, dispatchY, dispatchZ);
 
-    // Insert a compute→compute memory barrier so that SSBO writes from
-    // this dispatch are visible to the next dispatch. QRhi does not
-    // insert these automatically between consecutive compute passes.
-    commands.beginExternal();
-    insertComputeBarrier(*renderer.state.rhi, commands);
-    commands.endExternal();
-
     commands.endComputePass();
   }