Harden scene loading: load each file once, fail loudly on malformed DSON

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

2 files changed, 103 insertions, 9 deletions

diff --git a/generat3d/scene.py b/generat3d/scene.py
index 49cc4c9..08ad4ee 100644
--- a/generat3d/scene.py
+++ b/generat3d/scene.py
@@ -77,12 +77,20 @@ def _lookup(library, doc, url, section):
     raise Error(f"{url}: no {section} entry {ident!r}")
 
 
+def _number(url, v):
+    if isinstance(v, bool) or not isinstance(v, (int, float)):
+        raise Error(f"{url}: dial value is not a number")
+    return v
+
+
 class _Loader:
     def __init__(self, path, library):
         self.path = Path(path)
         self.library = library
         self.doc = read_dson(self.path)
         self.graph = Graph()
+        self.loaded = set()
+        self.hd = 0
 
     def run(self):
         nodes = self.doc.get("scene", {}).get("nodes", [])
@@ -97,9 +105,9 @@ class _Loader:
                 posed = posed or any(v for ch in ("rotation", "translation")
                                      for v in _channel(n, ch))
             elif kind != "figure":
-                warn(f"skipped scene node {n.get('name', n['id'])!r} ({kind or 'no type'})")
+                warn(f"skipped scene node {n.get('name') or n.get('id')!r} ({kind or 'no type'})")
             elif "conform_target" in n:
-                warn(f"skipped follower {n.get('name', n['id'])!r} (not handled in v1)")
+                warn(f"skipped follower {n.get('name') or n.get('id')!r} (not handled in v1)")
             else:
                 figs.append((n, fpath, asset))
         if not figs:
@@ -112,6 +120,8 @@ class _Loader:
         posed = self._dials() or posed
         if posed:
             warn("pose in scene ignored: the .dbz holds rest data")
+        if self.hd:
+            warn(f"{self.hd} HD morphs: detail ignored (v1)")
         return Scene(self.path, self.figure, self.graph)
 
     def _figure(self, inst, fpath, asset):
@@ -171,25 +181,39 @@ class _Loader:
             morph = mod.get("morph")
             if morph:
                 if "hd_url" in morph:
-                    warn(f"{path}#{mod['id']}: HD morph detail ignored")
+                    self.hd += 1
                 d = morph.get("deltas", {}).get("values", [])
                 if morph.get("vertex_count", nverts) != nverts:
                     warn(f"{path}#{mod['id']}: vertex count mismatch, morph skipped")
                 elif d:
                     arr = np.array(d, dtype=float)
+                    if (arr.ndim != 2 or arr.shape[1] != 4
+                            or (arr[:, 0] != np.floor(arr[:, 0])).any()
+                            or (arr[:, 0] < 0).any() or (arr[:, 0] >= nverts).any()):
+                        raise Error(f"{path}#{mod['id']}: bad morph deltas")
                     self.figure.morphs[k] = (arr[:, 0].astype(int), arr[:, 1:4])
             for f in mod.get("formulas", []):
                 self._formula(f, f"{path}#{mod['id']}")
 
+    def _load_file(self, path):
+        if path.lower() in self.loaded:
+            return
+        self.loaded.add(path.lower())
+        doc = self.library.load(path)
+        try:
+            self._modifiers(path, doc)
+        except (KeyError, TypeError, ValueError, IndexError, AttributeError) as e:
+            raise Error(f"{path}: malformed morph data: {e!r}") from None
+
     def _morphs(self, fpath):
         morph_dir = posixpath.join(posixpath.dirname(fpath), "Morphs")
         for path in self.library.walk(morph_dir):
             try:
-                doc = self.library.load(path)
+                self.library.load(path)
             except Error as e:
                 warn(f"skipped unreadable morph file: {e}")
                 continue
-            self._modifiers(path, doc)
+            self._load_file(path)
 
     def _dials(self):
         """Apply .duf dial values. Returns True if the scene animates a pose channel."""
@@ -199,18 +223,18 @@ class _Loader:
             if "url" not in m:
                 continue
             k = key(self.figure.id, parse_url(m["url"])[2], "value")
-            v = m.get("channel", {}).get("current_value", 0.0)
+            v = _number(m["url"], m.get("channel", {}).get("current_value", 0.0))
             if k not in self.graph.props:
                 path = parse_url(m["url"])[1]
                 if path and self.library.find(path):
-                    self._modifiers(path, self.library.load(path))
+                    self._load_file(path)
             if k in self.graph.props:
                 values[k] = v
             elif v:
                 missing.append(m["url"])
         for a in scene.get("animations", []):
             channel = parse_url(a["url"])[3]
-            v = a.get("keys", [[0, 0.0]])[0][1]
+            v = _number(a["url"], a.get("keys", [[0, 0.0]])[0][1])
             if channel.split("/")[0] in POSE_REST:
                 posed = posed or bool(v)
             elif channel == "value":
@@ -228,4 +252,7 @@ class _Loader:
 
 def load(path, library):
     """Load a .duf scene and everything its figure needs."""
-    return _Loader(path, library).run()
+    try:
+        return _Loader(path, library).run()
+    except (KeyError, TypeError, IndexError, ValueError, AttributeError) as e:
+        raise Error(f"{path}: malformed scene data: {e!r}") from None
diff --git a/tests/test_scene.py b/tests/test_scene.py
index 0e52216..13b80bb 100644
--- a/tests/test_scene.py
+++ b/tests/test_scene.py
@@ -123,3 +123,70 @@ class SceneTest(unittest.TestCase):
                                       "rotation": [{"id": "x", "current_value": 30}]}]
         _, err = load_quiet(self.duf({"nodes": nodes}))
         self.assertIn("pose", err)
+
+    def root(self, rel, doc):
+        """Extra library root holding one hand-written file."""
+        root = self.dir / "extra"
+        p = root / rel
+        p.parent.mkdir(parents=True, exist_ok=True)
+        p.write_text(json.dumps(doc))
+        return root
+
+    def morph_doc(self, *mods):
+        return {"modifier_library": list(mods)}
+
+    def mod(self, ident, morph=None):
+        m = {"id": ident, "parent": "/data/Test/Fig/Fig.dsf#Fig",
+             "channel": {"id": "value", "value": 0}}
+        if morph:
+            m["morph"] = morph
+        return m
+
+    MORPHS = "data/Test/Fig/Morphs/Test/"
+
+    def test_file_loaded_once(self):
+        scene, _ = load_quiet(self.duf({"nodes": self.base_nodes(), "animations": [
+            {"url": "Fig:/data/Test/Fig/Morphs/Test/Tall.dsf#Tall?value", "keys": [[0, 1]]}],
+            "modifiers": [{"id": "Typo", "url": "/data/Test/Fig/Morphs/Test/Tall.dsf#Typo",
+                           "parent": "#Fig", "channel": {"id": "value", "current_value": 0}}]}))
+        self.assertEqual(len(scene.graph.formulas["hip:hip?center_point/y"]), 1)
+        self.assertEqual(scene.graph.evaluate()["hip:hip?center_point/y"], 7)
+
+    def test_malformed_morph_file_is_fatal(self):
+        root = self.root(self.MORPHS + "NoId.dsf", {"modifier_library": [{"channel": {"value": 0}}]})
+        with self.assertRaisesRegex(Error, "NoId.dsf: malformed morph data"):
+            load_quiet(DATA / "scene.duf", [root])
+
+    def test_bad_deltas_fatal(self):
+        root = self.root(self.MORPHS + "BadDelta.dsf", self.morph_doc(
+            self.mod("BadDelta", {"vertex_count": 3, "deltas": {"values": [[7, 1, 1, 1]]}})))
+        with self.assertRaisesRegex(Error, "bad morph deltas"):
+            load_quiet(DATA / "scene.duf", [root])
+
+    def test_malformed_scene_data(self):
+        for scene in ({"nodes": self.base_nodes(), "animations": [
+                {"url": "Fig:#Tall?value", "keys": []}]}, {"nodes": None}):
+            with self.assertRaisesRegex(Error, "malformed scene data"):
+                load_quiet(self.duf(scene))
+
+    def test_dial_not_a_number(self):
+        with self.assertRaisesRegex(Error, "dial value is not a number"):
+            load_quiet(self.duf({"nodes": self.base_nodes(), "modifiers": [
+                {"id": "Tall", "url": "/data/Test/Fig/Morphs/Test/Tall.dsf#Tall",
+                 "parent": "#Fig", "channel": {"id": "value", "current_value": "a"}}]}))
+
+    def test_hd_warning_once(self):
+        hd = {"vertex_count": 3, "hd_url": "x", "deltas": {"values": [[0, 1, 0, 0]]}}
+        root = self.root(self.MORPHS + "Hd.dsf", self.morph_doc(
+            self.mod("Hd1", hd), self.mod("Hd2", hd)))
+        _, err = load_quiet(DATA / "scene.duf", [root])
+        self.assertEqual(err.count("2 HD morphs"), 1)
+
+    def test_on_demand_morph_outside_morphs_dir(self):
+        root = self.root("data/Test/Extra/Bulk.dsf", self.morph_doc(
+            self.mod("Bulk", {"vertex_count": 3, "deltas": {"values": [[2, 0, 0, 1]]}})))
+        scene, _ = load_quiet(self.duf({"nodes": self.base_nodes(), "modifiers": [
+            {"id": "Bulk", "url": "/data/Test/Extra/Bulk.dsf#Bulk", "parent": "#Fig",
+             "channel": {"id": "value", "current_value": 1}}]}), [root])
+        self.assertIn("Fig:Bulk?value", scene.figure.morphs)
+        self.assertEqual(scene.graph.props["Fig:Bulk?value"].value, 1)