fix(shaders_lab): cycle check uses real reachability, not vector index
Previously the cycle validator rejected any link whose source had a vector index >= target's, which silently killed legitimate connections between nodes added in the wrong drop order. Switch to a DFS over source_ids: an edge from->to creates a cycle iff `from` already (transitively) depends on `to`. topo_sort runs after each topology change so the vector ends up in a consistent order regardless of how nodes were inserted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Binary file not shown.
@@ -4,6 +4,7 @@
|
||||
#include "imgui_node_editor.h"
|
||||
#include <algorithm>
|
||||
#include <cstdio>
|
||||
#include <functional>
|
||||
#include <queue>
|
||||
#include <string>
|
||||
#include <unordered_map>
|
||||
@@ -325,14 +326,38 @@ bool dag_node_editor(std::vector<DagStep>& pipeline) {
|
||||
int from_idx = find_by_uid(pipeline, from_uid);
|
||||
int to_idx = find_by_uid(pipeline, to_uid);
|
||||
|
||||
// Reject cycles: in topo-ordered list, from must come before to.
|
||||
// We also reject self-loops.
|
||||
// Real cycle check: would the new edge from->to introduce a path
|
||||
// from `from` back to itself? It does iff `from` already (transitively)
|
||||
// depends on `to`. We walk source_ids of `from` and reject if we
|
||||
// ever hit `to`. Vector index order is irrelevant — topo_sort runs
|
||||
// at end-of-frame and reorders the pipeline.
|
||||
bool cycle = false;
|
||||
if (from_uid == to_uid) {
|
||||
ed::RejectNewItem(ImVec4(1, 0, 0, 1));
|
||||
valid = false;
|
||||
} else if (from_idx >= to_idx) {
|
||||
// Would create a back-edge; reject
|
||||
ed::RejectNewItem(ImVec4(1, 0.5f, 0, 1));
|
||||
cycle = true;
|
||||
} else if (from_idx >= 0 && to_idx >= 0) {
|
||||
const std::string to_id = pipeline[static_cast<size_t>(to_idx)].id;
|
||||
std::function<bool(const std::string&)> depends_on_to;
|
||||
depends_on_to = [&](const std::string& node_id) -> bool {
|
||||
if (node_id == to_id) return true;
|
||||
int idx = -1;
|
||||
for (int i = 0; i < static_cast<int>(pipeline.size()); ++i) {
|
||||
if (pipeline[static_cast<size_t>(i)].id == node_id) { idx = i; break; }
|
||||
}
|
||||
if (idx < 0) return false;
|
||||
const DagStep& s = pipeline[static_cast<size_t>(idx)];
|
||||
const DagNodeDef* d = dag_find(s.name);
|
||||
int n = d ? d->num_inputs : 0;
|
||||
for (int k = 0; k < n; ++k) {
|
||||
const std::string& sid = s.source_ids[static_cast<size_t>(k)];
|
||||
if (!sid.empty() && depends_on_to(sid)) return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
cycle = depends_on_to(pipeline[static_cast<size_t>(from_idx)].id);
|
||||
}
|
||||
|
||||
if (cycle) {
|
||||
ed::RejectNewItem(ImVec4(1, 0.3f, 0.3f, 1));
|
||||
valid = false;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user