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:
@@ -4,6 +4,7 @@
|
|||||||
#include "imgui_node_editor.h"
|
#include "imgui_node_editor.h"
|
||||||
#include <algorithm>
|
#include <algorithm>
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
|
#include <functional>
|
||||||
#include <queue>
|
#include <queue>
|
||||||
#include <string>
|
#include <string>
|
||||||
#include <unordered_map>
|
#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 from_idx = find_by_uid(pipeline, from_uid);
|
||||||
int to_idx = find_by_uid(pipeline, to_uid);
|
int to_idx = find_by_uid(pipeline, to_uid);
|
||||||
|
|
||||||
// Reject cycles: in topo-ordered list, from must come before to.
|
// Real cycle check: would the new edge from->to introduce a path
|
||||||
// We also reject self-loops.
|
// 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) {
|
if (from_uid == to_uid) {
|
||||||
ed::RejectNewItem(ImVec4(1, 0, 0, 1));
|
cycle = true;
|
||||||
valid = false;
|
} else if (from_idx >= 0 && to_idx >= 0) {
|
||||||
} else if (from_idx >= to_idx) {
|
const std::string to_id = pipeline[static_cast<size_t>(to_idx)].id;
|
||||||
// Would create a back-edge; reject
|
std::function<bool(const std::string&)> depends_on_to;
|
||||||
ed::RejectNewItem(ImVec4(1, 0.5f, 0, 1));
|
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;
|
valid = false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user