diff --git a/apps/studio/config/sensors/finger/scene/finger-display-1.0.0.oryscene b/apps/studio/config/sensors/finger/scene/finger-display-1.0.0.oryscene index b78a30b..773373b 100644 Binary files a/apps/studio/config/sensors/finger/scene/finger-display-1.0.0.oryscene and b/apps/studio/config/sensors/finger/scene/finger-display-1.0.0.oryscene differ diff --git a/apps/studio/src/orisys_studio/app_window.py b/apps/studio/src/orisys_studio/app_window.py index 29be5c0..028e2ba 100644 --- a/apps/studio/src/orisys_studio/app_window.py +++ b/apps/studio/src/orisys_studio/app_window.py @@ -2130,12 +2130,38 @@ class StudioMainWindow(QMainWindow): self._persist_prefs() def _sync_3d_source(self) -> None: - """Tell the 3D pane which sensor it is looking at. No IO, no scene read.""" + """Tell the 3D pane which sensor it is looking at. No IO, no scene read. + + The device's own geometry description travels with it as a *provider*: deriving + it reads and digests the unroll map the runtime loads, and the scene loader does + that on its worker thread. Without it the pane could only compare the asset with + itself, which is how a scene built on a different mapping would still look + matched. + """ self.mesh3d_panel.set_sensor( self.sensor_picker.sensor_type(), + expected_provider=self._device_geometry_provider(), package_override=self.prefs.get("scene3d_package"), ) + def _device_geometry_provider(self): + """A callable the loader runs off the GUI thread: what this device really is. + + ``None`` when the application cannot name the sensor's configuration; the pane + then reports the scene as unverified rather than pretending it was compared. + """ + sensor_type = self.sensor_picker.sensor_type() + config_ref = self._resolve_config_ref() + if not config_ref: + return None + + def describe(): + from orisys_app_shared.acquisition.geometry import describe_sensor_geometry + + return describe_sensor_geometry(sensor_type, config_ref) + + return describe + def _restore_3d_settings(self) -> None: """Persisted display settings, repaired by ``scene3d.prefs`` on the way in.""" from orisys_studio.scene3d.prefs import settings_from_prefs diff --git a/apps/studio/src/orisys_studio/scene3d/asset_source.py b/apps/studio/src/orisys_studio/scene3d/asset_source.py index 14c69fd..b904c2d 100644 --- a/apps/studio/src/orisys_studio/scene3d/asset_source.py +++ b/apps/studio/src/orisys_studio/scene3d/asset_source.py @@ -31,7 +31,7 @@ import logging from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass from pathlib import Path -from typing import Optional +from typing import TYPE_CHECKING, Any, Callable, Optional from PyQt5.QtCore import QObject, pyqtSignal @@ -44,6 +44,9 @@ from orisys_app_shared.scene3d.contracts import ( error, ) +if TYPE_CHECKING: # the description is a value object; the callable returns one + from orisys_app_shared.acquisition.geometry import DeviceGeometry + logger = logging.getLogger(__name__) #: The container suffix written by ``tools/scene_assets/compile_scene.py``. @@ -131,51 +134,102 @@ def load_package( return PackagedAssetLoader().load(source.path, expected) +def _resolve_and_load( + source: SceneSource, + expected: Optional[ExpectedGeometry], + provider: "Optional[Callable[[], DeviceGeometry]]", +) -> tuple[SceneAssets, "Optional[DeviceGeometry]"]: + """Worker-thread half: build the *device's* description, then read the package. + + Deriving the description digests the unroll map the runtime loads, so it belongs on + this thread. A description that cannot be built raises with a diagnostic naming the + bundle: falling back to the asset's own identity would make the comparison a + sentence the asset wrote about itself, which is the bug being fixed. + """ + device = None + if expected is None and provider is not None: + device = provider() + expected = device.expected + return load_package(source, expected), device + + class SceneAssetLoader(QObject): """Loads one scene off the GUI thread and reports the result as a signal. - One load is kept: the pane only ever needs the newest, and a second request while - one is running is a request for the same file. + One load runs at a time and the **newest request wins**: a request that arrives + while another is running replaces the pending one instead of being dropped, and a + completion carries the request id it belongs to, so a slow load cannot land on a + selection the operator has already moved away from. """ - loaded = pyqtSignal(object, str) # SceneAssets, source description - failed = pyqtSignal(object) # Diagnostic + loaded = pyqtSignal(object, str, int, object) # SceneAssets, description, id, device + failed = pyqtSignal(object, int) # Diagnostic, request id def __init__(self, parent: Optional[QObject] = None) -> None: super().__init__(parent) self._executor: Optional[ThreadPoolExecutor] = None self._inflight = False + self._request_id = 0 + #: The newest request that could not start yet (source, expected, provider, id). + self._pending: Optional[tuple[SceneSource, Optional[ExpectedGeometry], Any, int]] = None - def load(self, source: SceneSource, expected: Optional[ExpectedGeometry] = None) -> None: + def load( + self, + source: SceneSource, + expected: Optional[ExpectedGeometry] = None, + *, + expected_provider: "Optional[Callable[[], DeviceGeometry]]" = None, + ) -> int: + """Ask for a scene; returns the request id this call was given.""" + self._request_id += 1 + request = (source, expected, expected_provider, self._request_id) if self._inflight: - return + self._pending = request + return self._request_id + self._start(request) + return self._request_id + + def _start(self, request) -> None: + source, expected, provider, request_id = request if self._executor is None: self._executor = ThreadPoolExecutor( max_workers=1, thread_name_prefix="scene3d-assets" ) self._inflight = True - future = self._executor.submit(load_package, source, expected) + future = self._executor.submit(_resolve_and_load, source, expected, provider) future.add_done_callback( - lambda done: self._finished(done, source) + lambda done: self._finished(done, source, request_id) ) - def _finished(self, future, source: SceneSource) -> None: + def _finished(self, future, source: SceneSource, request_id: int) -> None: self._inflight = False try: - assets = future.result() + assets, device = future.result() except Exception as exc: # noqa: BLE001 - a bad package is a diagnostic item = getattr(exc, "diagnostic", None) or diagnostic( "asset_rejected", f"{type(exc).__name__}: {exc}" ) logger.warning("3D scene %s was rejected: %s", source.path, item.message) - # Emitted from this worker thread; the receiver is a Qt object on the - # GUI thread, so the connection is queued (see the view's `_on_loaded`). - self.failed.emit(item) + self._emit_or_queue(lambda: self.failed.emit(item, request_id)) return logger.info("3D scene loaded from %s", source.path) - self.loaded.emit(assets, source.describe()) + self._emit_or_queue( + lambda: self.loaded.emit(assets, source.describe(), request_id, device) + ) + + def _emit_or_queue(self, emit) -> None: + """Emit this completion, then start the newest request that arrived meanwhile. + + Emitted from the worker thread; the receivers are Qt objects on the GUI thread, + so the connection is queued (see the panel's ``_on_loaded``). + """ + pending, self._pending = self._pending, None + emit() + if pending is not None: + self._start(pending) def close(self) -> None: + self._pending = None if self._executor is not None: self._executor.shutdown(wait=False) self._executor = None diff --git a/apps/studio/src/orisys_studio/scene3d/view.py b/apps/studio/src/orisys_studio/scene3d/view.py index 706c373..abcaa5e 100644 --- a/apps/studio/src/orisys_studio/scene3d/view.py +++ b/apps/studio/src/orisys_studio/scene3d/view.py @@ -111,6 +111,8 @@ class Scene3DView(QWidget): self._last_stats: Optional[RenderStats] = None #: The coordinator's last diagnostic as *this* view has already reported it. self._reported_diagnostic: Optional[Diagnostic] = None + #: Whether the loaded asset was compared against the device's own description. + self._device_verified = False self._layout = QVBoxLayout(self) self._layout.setContentsMargins(0, 0, 0, 0) @@ -160,21 +162,41 @@ class Scene3DView(QWidget): ) -> bool: """Adopt a validated scene. Returns whether the view accepted it. - ``expected`` is the device's own description. It is optional for the finger - (whose adapter needs nothing but the packet) and required for a box, where it - is the only statement of what the device's field means — an adapter built - without it would be guessing which grid to trust. + ``expected`` is the **device's own** description. It is required for a box, where + it is the only statement of what the device's field means, and it is what makes + the asset check meaningful for a finger: without it the comparison would be the + asset agreeing with itself. An absent description does not refuse the scene — the + pane can still show the surface — but it is recorded as *unverified*, and the + adapter is then built from the asset, which is exactly what "unverified" means. """ if not self._available or self._coordinator is None: return False try: - validate_assets(assets, None) + warnings = validate_assets(assets, expected) except Exception as exc: # noqa: BLE001 self._on_diagnostic( getattr(exc, "diagnostic", None) or diagnostic("asset_rejected", f"{type(exc).__name__}: {exc}") ) return False + for item in warnings: + self._on_diagnostic(item) + # "Verified" means every identity the device stated was actually compared. A + # statement that could not be compared (an older package, no source-geometry + # record) is reported as unverified rather than counted as a match. + self._device_verified = bool( + expected is not None + and (expected.geometry_id or expected.source_geometry_id) + and not any(item.code == "geometry_unverified" for item in warnings) + ) + if not self._device_verified: + self._on_diagnostic( + diagnostic( + "geometry_unverified", + "no device geometry description: the scene is shown as a reference, " + "not verified against this device", + ) + ) snapshot = self._make_snapshot(assets, expected) if snapshot is None: return False @@ -213,13 +235,31 @@ class Scene3DView(QWidget): from the packet, while the box adapter needs the device description to know whether the field's grid is the one the scene was built for. Binding the call here keeps the acquisition path free of both variants. + + The identity the adapter stamps on a frame is the **device's**, not the asset's: + when the adapter is built from the asset, "the frame matches the scene" is a + sentence the asset wrote about itself. Where the device cannot state a field + (the metre-scaled geometry id, on a device that has no unit statement) the + asset's value is used and the pane is already marked unverified. """ asset_id = str(getattr(assets, "asset_id", "") or "") self._adapter = None + device = expected common = { - "geometry_id": assets.geometry_id, - "field_domain_id": assets.field_domain_id, - "shape_hw": tuple(assets.shape_hw), + "geometry_id": str( + device.geometry_id if device is not None and device.geometry_id + else assets.geometry_id + ), + "field_domain_id": str( + device.field_domain_id + if device is not None and device.field_domain_id + else assets.field_domain_id + ), + "shape_hw": ( + tuple(int(v) for v in device.shape_hw) + if device is not None and all(int(v) for v in device.shape_hw) + else tuple(assets.shape_hw) + ), } if asset_id.startswith(("box", "mini")): if expected is None: @@ -419,6 +459,7 @@ class Scene3DView(QWidget): settings = self.settings() scale = settings.color_scale descriptor = self._last_descriptor + expected = self._expected return { "scene": { "asset_id": str(getattr(assets, "asset_id", "")), @@ -426,6 +467,23 @@ class Scene3DView(QWidget): "asset_hash": str(getattr(assets, "asset_hash", "")), "geometry_id": str(getattr(assets, "geometry_id", "")), "field_domain_id": str(getattr(assets, "field_domain_id", "")), + "source_geometry_id": str(getattr(assets, "source_geometry_id", "")), + # Whether the asset was compared against what the device says it is. A + # picture drawn without that comparison is a reference, not a verified + # measurement, and the record has to be able to say so. + "device_verified": bool(self._device_verified), + "device": ( + { + "sensor_model": str(expected.sensor_model), + "hardware_revision": str(expected.hardware_revision), + "geometry_id": str(expected.geometry_id), + "field_domain_id": str(expected.field_domain_id), + "source_geometry_id": str(expected.source_geometry_id), + "shape_hw": f"{expected.shape_hw[0]}x{expected.shape_hw[1]}", + } + if expected is not None + else {} + ), "platform_lock": ( assets.platform_lock.to_dict() if getattr(assets, "platform_lock", None) is not None @@ -557,6 +615,7 @@ class Scene3DView(QWidget): self._assets = None self._snapshot = None self._adapter = None + self._device_verified = False if self._coordinator is not None: self._coordinator.close() if self._widget is not None: diff --git a/apps/studio/src/orisys_studio/ui/mesh3d_panel.py b/apps/studio/src/orisys_studio/ui/mesh3d_panel.py index e2bf532..0f3554c 100644 --- a/apps/studio/src/orisys_studio/ui/mesh3d_panel.py +++ b/apps/studio/src/orisys_studio/ui/mesh3d_panel.py @@ -14,13 +14,18 @@ acquisition, and no path that reports a 3D problem as a recording problem. from __future__ import annotations from pathlib import Path -from typing import TYPE_CHECKING, Optional +from typing import TYPE_CHECKING, Callable, Optional from PyQt5.QtCore import Qt, pyqtSignal from PyQt5.QtWidgets import QFrame, QLabel, QStackedLayout, QVBoxLayout, QWidget from orisys_app_shared.i18n import t -from orisys_app_shared.scene3d.contracts import Diagnostic, ExpectedGeometry, SceneAssets +from orisys_app_shared.scene3d.contracts import ( + Diagnostic, + ExpectedGeometry, + SceneAssets, + diagnostic, +) from orisys_studio.scene3d.asset_source import ( SceneAssetLoader, SceneSource, @@ -29,6 +34,7 @@ from orisys_studio.scene3d.asset_source import ( ) if TYPE_CHECKING: # the view imports GL bindings lazily; the type is enough here + from orisys_app_shared.acquisition.geometry import DeviceGeometry from orisys_studio.scene3d.view import Scene3DView #: Shown when the pane is on but this sensor has no 3D scene at all (box, mini). @@ -73,9 +79,14 @@ class Mesh3dPanel(QFrame): self._source: Optional[SceneSource] = None self._sensor_type = "" self._expected: Optional[ExpectedGeometry] = None + self._expected_provider: "Optional[Callable[[], DeviceGeometry]]" = None self._source_kind = "live" self._enabled = False self._loaded = False + #: The id of the load this pane is waiting for: a completion for an older + #: request describes a selection the operator has already moved away from. + self._load_request = 0 + self._device = None #: The package whose scene is in the view, so switching sensor away and back #: does not reload it (and does not pretend to be loading it either). self._loaded_path: Optional[Path] = None @@ -107,21 +118,32 @@ class Mesh3dPanel(QFrame): sensor_type: str, *, expected: Optional[ExpectedGeometry] = None, + expected_provider: "Optional[Callable[[], DeviceGeometry]]" = None, package_override: str | Path | None = None, ) -> None: """Tell the pane which sensor it is looking at. Cheap; no IO. Called when the sensor changes, so a box sensor reports "no 3D scene for this sensor" rather than the finger's missing-package message. + + ``expected_provider`` is the device's own description, derived lazily: building + it reads and digests the unroll map the runtime loads, which the scene loader + does on its worker thread (never here). """ sensor_type = str(sensor_type) if sensor_type == self._sensor_type and self._source is not None: # Same sensor, already resolved: re-resolving would drop a loaded scene - # every time the window re-applied its panel visibility. + # every time the window re-applied its panel visibility. The provider is + # still refreshed — it may describe a config the operator just changed. self._expected = expected if expected is not None else self._expected + if expected_provider is not None: + self._expected_provider = expected_provider return self._sensor_type = sensor_type self._expected = expected + self._expected_provider = ( + expected_provider if expected_provider is not None else self._expected_provider + ) self._loaded = False if self._sensor_type != "finger": self._show_message(_NO_SCENE) @@ -186,7 +208,9 @@ class Mesh3dPanel(QFrame): self._loader = SceneAssetLoader(self) self._loader.loaded.connect(self._on_loaded) self._loader.failed.connect(self._on_load_failed) - self._loader.load(self._source, self._expected) + self._load_request = self._loader.load( + self._source, self._expected, expected_provider=self._expected_provider + ) # -------------------------------------------------------------- settings def set_display_settings(self, settings) -> None: @@ -247,11 +271,17 @@ class Mesh3dPanel(QFrame): return self._view.render_totals() # ------------------------------------------------------------------ events - def _on_loaded(self, assets, description: str) -> None: + def _on_loaded(self, assets, description: str, request_id: int, device) -> None: + if int(request_id) != int(self._load_request): + # A completion for a request the operator has already replaced: adopting it + # would mark the *new* selection as loaded with the old file's scene. + return view = self._view if view is None: return - if not view.use_scene(assets, source_id=description, expected=self._expected): + self._device = device + expected = device.expected if device is not None else self._expected + if not view.use_scene(assets, source_id=description, expected=expected): return self._loaded_path = None if self._source is None else self._source.path self._stack.setCurrentWidget(view) @@ -259,7 +289,9 @@ class Mesh3dPanel(QFrame): # the operator (or the window, restoring a preference) turns it on. view.set_enabled(self._enabled) - def _on_load_failed(self, item: Diagnostic) -> None: + def _on_load_failed(self, item: Diagnostic, request_id: int = 0) -> None: + if int(request_id) != int(self._load_request): + return self._reported = item self._loaded_path = None self._show_message(_REJECTED, message=item.message) diff --git a/apps/studio/tests/test_scene3d_view.py b/apps/studio/tests/test_scene3d_view.py index 8d30464..299e88c 100644 --- a/apps/studio/tests/test_scene3d_view.py +++ b/apps/studio/tests/test_scene3d_view.py @@ -23,6 +23,7 @@ from __future__ import annotations import itertools import shutil import time +from concurrent.futures import Future from dataclasses import replace from pathlib import Path @@ -305,6 +306,8 @@ class _RecordingView(QWidget): self.packets: list = [] self.coordinator = None self.assets = None + self.scenes_used: list = [] + self.accept_scene = True self.available = True self.reason = "" self.source_kind = "live" @@ -321,6 +324,14 @@ class _RecordingView(QWidget): float(np.asarray(frame, dtype=np.float64).sum()) # the container's contract + def use_scene(self, assets, source_id: str, expected=None) -> bool: + """The seam ``Mesh3dPanel._on_loaded`` adopts a scene through.""" + self.scenes_used.append((assets, source_id, expected)) + if not self.accept_scene: + return False + self.assets = assets + return True + def set_enabled(self, enabled: bool) -> None: self.enabled_flag = bool(enabled) @@ -554,16 +565,108 @@ def test_a_package_is_loaded_off_the_gui_thread(qt_app, write_poc): assets, description = loaded[0] assert assets.geometry_id == synthetic.poc_scene().geometry_id assert str(path) in description - # A second load while one is in flight is the same request, not a queue. + # A second request while one is in flight is not dropped: it is the one the + # operator asked for *last*, so it waits for the first to finish and then runs. loader2 = SceneAssetLoader() try: loader2._inflight = True - loader2.load(SceneSource(path=path, kind="package", directory=path.parent)) - assert loader2._executor is None + first = loader2.load(SceneSource(path=path, kind="package", directory=path.parent)) + second = loader2.load(SceneSource(path=path, kind="package", directory=path.parent)) + assert second > first + assert loader2._executor is None, "a queued request must not open a second worker" + assert loader2._pending is not None, "the newer request was dropped" + assert loader2._pending[-1] == second finally: loader2.close() +class _ManualExecutor: + """Futures that only run when the test says so, like a loaded thread pool.""" + + def __init__(self) -> None: + self.pending: list = [] + + def submit(self, function, *args, **kwargs): + future: Future = Future() + self.pending.append((future, function, args, kwargs)) + return future + + def run_next(self) -> None: + future, function, args, kwargs = self.pending.pop(0) + if future.set_running_or_notify_cancel(): + try: + future.set_result(function(*args, **kwargs)) + except BaseException as exc: # noqa: BLE001 - mirrored into the future + future.set_exception(exc) + + def shutdown(self, wait: bool = True) -> None: # noqa: ARG002 - executor contract + self.pending.clear() + + +def test_the_newest_load_is_the_one_that_runs(qt_app, write_poc): + """R7: requests carry an id, and a queue keeps the newest rather than the first.""" + from orisys_studio.scene3d.asset_source import SceneAssetLoader + + path = write_poc() + loader = SceneAssetLoader() + loader._executor = _ManualExecutor() + answers: list = [] + loader.loaded.connect( + lambda assets, description, request_id, device: answers.append(request_id) + ) + loader.failed.connect(lambda item, request_id: answers.append(("failed", request_id))) + try: + first = loader.load(SceneSource(path=path, kind="package", directory=path.parent)) + second = loader.load(SceneSource(path=path, kind="package", directory=path.parent)) + assert answers == [], "a request was answered before its worker ran" + loader._executor.run_next() + assert answers == [first], "the pending request did not start after the first" + assert loader._pending is None + loader._executor.run_next() + assert answers == [first, second] + finally: + loader.close() + + +def test_the_pane_ignores_a_completion_for_a_superseded_load(qt_app): + """The other half: an old answer must not mark the *new* selection as loaded.""" + from orisys_studio.ui.mesh3d_panel import Mesh3dPanel + + panel = Mesh3dPanel() + view = _RecordingView() + panel.attach_view(view) + assets = synthetic.poc_scene() + panel._load_request = 2 + panel._on_loaded(assets, "old.oryscene", 1, None) + assert view.assets is None, "a completion for an older request was adopted" + panel._on_loaded(assets, "new.oryscene", 2, None) + assert view.assets is assets + assert view.scenes_used[-1][1] == "new.oryscene" + + +def test_a_failed_load_can_be_retried(qt_app, tmp_path, write_poc): + """A rejection is not a dead end: the next request still runs.""" + from orisys_studio.scene3d.asset_source import SceneAssetLoader + + broken = tmp_path / f"broken{PACKAGE_SUFFIX}" + broken.write_bytes(b"not a zip archive at all") + good = write_poc() + loader = SceneAssetLoader() + loader._executor = _ManualExecutor() + answers: list = [] + loader.loaded.connect(lambda assets, description, request_id, device: answers.append(request_id)) + loader.failed.connect(lambda item, request_id: answers.append(("failed", request_id))) + try: + bad = loader.load(SceneSource(path=broken, kind="package", directory=tmp_path)) + loader._executor.run_next() + assert answers == [("failed", bad)] + again = loader.load(SceneSource(path=good, kind="package", directory=good.parent)) + loader._executor.run_next() + assert answers[-1] == again + finally: + loader.close() + + def test_a_corrupt_package_is_reported_not_raised(qt_app, tmp_path): from orisys_studio.scene3d.asset_source import SceneAssetLoader @@ -694,6 +797,79 @@ def test_a_fresh_field_reaches_the_screen_and_clears_the_stale_state(qt_app): view.close() +# ---------------------------------- stage 6: the adapter speaks for the *device* (R3) +def test_the_adapter_identity_comes_from_the_device_not_the_asset(qt_app): + """R3: built from the asset, "the frame matches the scene" is self-certified.""" + from orisys_app_shared.scene3d.contracts import ExpectedGeometry + from orisys_studio.scene3d.view import Scene3DView + + view = Scene3DView() + try: + if not view.available: + pytest.skip("no GL binding here") + assets = replace(synthetic.poc_scene(), source_geometry_id="device-src") + device = ExpectedGeometry( + "finger", + "rev", + "", + assets.field_domain_id, + assets.shape_hw, + source_geometry_id="device-src", + ) + assert view.use_scene(assets, source_id="poc", expected=device) + described = view.describe()["scene"] + assert described["device_verified"] is True, described + assert described["device"]["source_geometry_id"] == "device-src" + assert described["source_geometry_id"] == "device-src" + + adapter = view._adapter + assert adapter is not None + # The device stated no metre-scaled id, so the asset's is used *and* the pane + # is only verified on the identities the device did state. + assert adapter.geometry_id == assets.geometry_id + assert adapter.field_domain_id == assets.field_domain_id + assert adapter.shape_hw == tuple(assets.shape_hw) + finally: + view.close() + + +def test_a_device_whose_domain_disagrees_refuses_the_scene(qt_app): + from orisys_app_shared.scene3d.contracts import ExpectedGeometry + from orisys_studio.scene3d.view import Scene3DView + + view = Scene3DView() + try: + if not view.available: + pytest.skip("no GL binding here") + assets = synthetic.poc_scene() + device = ExpectedGeometry( + "finger", "", "", "somebody-elses-domain", assets.shape_hw + ) + assert view.use_scene(assets, source_id="poc", expected=device) is False + assert view.assets is None + assert view.last_diagnostic is not None + assert view.last_diagnostic.code == "field_domain_mismatch" + finally: + view.close() + + +def test_a_scene_without_a_device_description_is_shown_as_unverified(qt_app): + """Expected unknown: show the surface, but never claim it was checked.""" + from orisys_studio.scene3d.view import Scene3DView + + view = Scene3DView() + try: + if not view.available: + pytest.skip("no GL binding here") + assert view.use_scene(synthetic.poc_scene(), source_id="poc") + described = view.describe()["scene"] + assert described["device_verified"] is False + assert view.last_diagnostic is not None + assert view.last_diagnostic.code == "geometry_unverified" + finally: + view.close() + + # ------------------------------------------------------- stage 5: the record block def test_a_recording_says_what_the_3d_pane_was( make_window, next_record_root, write_poc, qt_app, monkeypatch diff --git a/shared/src/orisys_app_shared/acquisition/geometry.py b/shared/src/orisys_app_shared/acquisition/geometry.py new file mode 100644 index 0000000..509a795 --- /dev/null +++ b/shared/src/orisys_app_shared/acquisition/geometry.py @@ -0,0 +1,191 @@ +"""What the acquisition side can say about the geometry it is really running. + +The 3D asset carries an identity of its own, but an identity the *asset* stamps onto +the incoming frames proves nothing: if the adapter is built from the asset, then +"the frame matches the scene" is a sentence the asset wrote about itself. The device +side has to make its own statement, and this module derives it from the same files the +runtime loads — the sensor bundle (``manifest.json``, ``sensor.json``) and the unroll +map those name. A map that changed, a different ``source_scale``, a different +resolution or a different bundle then becomes a *mismatch* instead of a picture that +looks plausible. + +Two identities are produced, and the difference matters: + +* ``field_domain_id`` — the UV mapping the tracker's field actually lives on (shape, + flip, scale, origin and the map's own content digest). The device can always state + this, and it is what catches "same 300x280, different mapping". +* ``source_geometry_id`` — the unit-free identity of the surface behind that mapping. + The metre-scaled ``geometry_id`` needs the operator's "one source unit is N metres" + statement, which no device can make; the unit-free digest needs nothing but the map + file, so this is the geometry check a device *can* perform. + +``geometry_id`` is deliberately left empty here. A consumer that sees an empty id must +report ``geometry_unverified`` rather than treat it as a match (see +``contracts.validate_assets``). +""" + +from __future__ import annotations + +import json +import logging +from dataclasses import dataclass +from pathlib import Path +from typing import Mapping, Optional + +from orisys_app_shared.scene3d.contracts import ExpectedGeometry, error +from orisys_app_shared.scene3d.digest import source_geometry_digest +from orisys_app_shared.scene3d.legacy import load_legacy_domain + +logger = logging.getLogger(__name__) + +#: The map file name the SDK derives from ``camera.source_scale`` (see the SDK's +#: ``scaled_map_cache_path``): ``unroll_map_uv_s0.4.npz`` for 0.4, and the unscaled +#: ``unroll_map_uv.npz`` for 1.0. +_MAP_STEM = "unroll_map_uv" +_MAP_SUFFIX = ".npz" + + +@dataclass(frozen=True) +class DeviceGeometry: + """The device's own description, and whether it is strong enough to verify.""" + + expected: ExpectedGeometry + #: True when at least one identity was stated (so the asset *was* compared). + verified: bool + #: Paths and identities, for the log and for a recording. Never contains data. + detail: Mapping[str, str] + + +def map_path_for(assets_dir: Path, source_scale: Optional[float]) -> Path: + """The unroll map the SDK would load for this scale, inside ``assets_dir``.""" + if source_scale is None or abs(float(source_scale) - 1.0) < 1e-9: + return Path(assets_dir) / f"{_MAP_STEM}{_MAP_SUFFIX}" + return Path(assets_dir) / f"{_MAP_STEM}_s{float(source_scale):g}{_MAP_SUFFIX}" + + +def _read_json(path: Path) -> dict: + try: + data = json.loads(path.read_text(encoding="utf-8")) + except FileNotFoundError: + return {} + except (OSError, json.JSONDecodeError) as exc: + raise error( + "asset_rejected", f"{path.name} is not readable JSON: {type(exc).__name__}" + ) from exc + return data if isinstance(data, dict) else {} + + +def _bundle_root(config_ref: str | Path) -> Optional[Path]: + path = Path(str(config_ref)) + if path.is_file(): + return path.parent + if path.is_dir(): + return path + return None + + +def _unverified(sensor_type: str, reason: str, **detail: str) -> DeviceGeometry: + """Nothing to compare: the pane may show the scene, but not as a verified match.""" + logger.warning("no device geometry description for %s: %s", sensor_type, reason) + return DeviceGeometry( + expected=ExpectedGeometry( + sensor_model=str(sensor_type), + hardware_revision="", + geometry_id="", + field_domain_id="", + shape_hw=(0, 0), + source_geometry_id="", + ), + verified=False, + detail={"reason": reason, **detail}, + ) + + +def describe_sensor_geometry( + sensor_type: str, + config_ref: str | Path, + *, + hardware_revision: str = "", +) -> DeviceGeometry: + """Derive the device's description of its own geometry. + + Runs off the GUI thread (it digests the map once). The two failure kinds are + deliberately different: + + * **nothing to describe** (the bundle cannot be located, or names no ``assets_dir``): + an *unverified* description with no identities, so the pane shows the surface and + says it was not checked — a static reference is not a lie; + * **a bundle that changed** (the map it names is missing, or its grid disagrees with + ``resolution_stitch``): raises with a diagnostic naming both, because that is + exactly the configuration change the check exists to catch. + """ + root = _bundle_root(config_ref) + if root is None: + return _unverified( + sensor_type, f"the sensor bundle could not be located: {config_ref}" + ) + manifest = _read_json(root / "manifest.json") + sensor = _read_json(root / "sensor.json") + assets_dir = manifest.get("assets_dir") + if not assets_dir: + return _unverified( + sensor_type, + f"the sensor bundle {root.name} does not name its 'assets_dir'", + bundle=str(root), + ) + assets_path = (root / str(assets_dir)).resolve() + camera = dict(sensor.get("camera") or {}) + stitch = camera.get("resolution_stitch") or () + scale = camera.get("source_scale") + try: + shape_hw = (int(stitch[1]), int(stitch[0])) + except (IndexError, TypeError, ValueError): + return _unverified( + sensor_type, + f"the sensor bundle {root.name} does not state camera.resolution_stitch", + bundle=str(root), + ) + map_path = map_path_for(assets_path, scale) + if not map_path.is_file(): + raise error( + "asset_rejected", + f"the unroll map for this sensor is missing: {map_path}", + directory=str(assets_path), + ) + domain = load_legacy_domain(map_path) + if tuple(domain.shape_hw) != shape_hw: + raise error( + "field_domain_mismatch", + "the unroll map and camera.resolution_stitch disagree about the field grid", + domain=f"{domain.shape_hw[0]}x{domain.shape_hw[1]}", + config=f"{shape_hw[0]}x{shape_hw[1]}", + ) + source_geometry_id = ( + source_geometry_digest(domain.world_xyz) if domain.world_xyz is not None else "" + ) + expected = ExpectedGeometry( + sensor_model=str(sensor_type), + hardware_revision=str(hardware_revision or ""), + # The metre-scaled id needs the unit statement; the device states what it can. + geometry_id="", + field_domain_id=domain.domain_id, + shape_hw=domain.shape_hw, + source_geometry_id=source_geometry_id, + ) + detail = { + "bundle": str(root), + "assets_dir": str(assets_path), + "map": str(map_path), + "field_domain_id": domain.domain_id, + "source_geometry_id": source_geometry_id, + "shape_hw": f"{domain.shape_hw[0]}x{domain.shape_hw[1]}", + "source_scale": "" if scale is None else f"{float(scale):g}", + } + verified = bool(domain.domain_id) and bool(source_geometry_id) + if not verified: + logger.warning( + "the device description for %s states no geometry identity: %s", + sensor_type, + detail, + ) + return DeviceGeometry(expected=expected, verified=verified, detail=detail) diff --git a/shared/src/orisys_app_shared/scene3d/contracts.py b/shared/src/orisys_app_shared/scene3d/contracts.py index ad960db..48fa737 100644 --- a/shared/src/orisys_app_shared/scene3d/contracts.py +++ b/shared/src/orisys_app_shared/scene3d/contracts.py @@ -61,6 +61,10 @@ DIAGNOSTIC_SEVERITY: Mapping[str, Severity] = { #: A producer made no statement about where its array came from. The consumer #: must not assume it is fresh (see ``SdkPacket.flow_valid``). "field_unverified": "warning", + #: Device and asset could not be compared on one of the identities (the package + #: predates the record, or the device cannot state it). Informational: the pane may + #: show the geometry, but it must not claim the measurement was checked against it. + "geometry_unverified": "info", "stale_frame": "info", "gl_unsupported": "error", "shader_compile_failed": "error", @@ -228,13 +232,23 @@ class CameraPreset: @dataclass(frozen=True) class ExpectedGeometry: - """What the device says it is; an asset must match it or be refused.""" + """What the device says it is; an asset must match it or be refused. + + ``geometry_id`` is the metre-scaled identity of the shape, and deriving it needs the + operator's unit statement — so a device that cannot make that statement leaves it + empty and states ``source_geometry_id`` instead: the unit-free identity of the + surface it is actually running, which it *can* compute from the map file it loads. + An empty field is "unstated", never "matches": a comparison skips it and the loader + reports :func:`geometry_unverified` rather than claiming a match nobody made. + """ sensor_model: str hardware_revision: str geometry_id: str field_domain_id: str shape_hw: tuple[int, int] + #: Unit-free identity of the source geometry behind the field domain. + source_geometry_id: str = "" @dataclass(frozen=True) @@ -462,6 +476,12 @@ class SceneAssets: #: triangle is this?" is a question an operator will ask, and re-deriving it #: from UVs alone is a guess. source_vertex_of_render: Optional[I32] = None + #: Unit-free identity of the source geometry this asset was built from, and what the + #: package says it is compatible with. The first is what a device can verify about + #: the surface; the second catches a package released for another sensor. + source_geometry_id: str = "" + compatible_sensor_model: str = "" + compatible_hardware_revisions: tuple[str, ...] = () #: Where this package may be displayed (stage 5). ``None`` means "not locked", #: which is what every package written before the lock existed carries. platform_lock: Optional[PlatformLock] = None @@ -857,29 +877,75 @@ def validate_assets( f"the platform lock is malformed: {exc.diagnostic.message}", ) from exc if expected is not None: - if expected.geometry_id != assets.geometry_id: + # Only *stated* fields are compared: an empty one is "the device cannot say", + # which is a reason to report "unverified", never a reason to pass silently. + if expected.geometry_id and expected.geometry_id != assets.geometry_id: raise error( "geometry_mismatch", "asset geometry does not match the device", asset=assets.geometry_id, device=expected.geometry_id, ) - if expected.field_domain_id != assets.binding.field_domain_id: + if expected.field_domain_id and expected.field_domain_id != assets.binding.field_domain_id: raise error( "field_domain_mismatch", "asset field domain does not match the device", asset=assets.binding.field_domain_id, device=expected.field_domain_id, ) - if tuple(expected.shape_hw) != tuple(assets.binding.shape_hw): + if ( + tuple(expected.shape_hw) != (0, 0) + and tuple(expected.shape_hw) != tuple(assets.binding.shape_hw) + ): raise error( "field_domain_mismatch", "asset UV shape does not match the device", asset=f"{assets.binding.shape_hw[0]}x{assets.binding.shape_hw[1]}", device=f"{expected.shape_hw[0]}x{expected.shape_hw[1]}", ) - if expected.sensor_model and expected.sensor_model != assets.parts[0].role: - pass # the model name is descriptive; the geometry id is the contract + if expected.source_geometry_id: + if not assets.source_geometry_id: + out.append( + diagnostic( + "geometry_unverified", + "the package records no source geometry, so the device's own " + "geometry could not be compared with it", + device=expected.source_geometry_id[:16], + ) + ) + elif expected.source_geometry_id != assets.source_geometry_id: + raise error( + "geometry_mismatch", + "the asset was built from a different source geometry than the " + "device is running", + asset=assets.source_geometry_id[:16], + device=expected.source_geometry_id[:16], + ) + if expected.sensor_model and assets.compatible_sensor_model: + if expected.sensor_model.casefold() != assets.compatible_sensor_model.casefold(): + raise error( + "geometry_mismatch", + "the package was released for another sensor model", + asset=assets.compatible_sensor_model, + device=expected.sensor_model, + ) + if expected.hardware_revision and assets.compatible_hardware_revisions: + revisions = tuple(str(item) for item in assets.compatible_hardware_revisions) + if "*" not in revisions and expected.hardware_revision not in revisions: + raise error( + "geometry_mismatch", + "the package was not released for this hardware revision", + asset=",".join(revisions), + device=expected.hardware_revision, + ) + if not expected.geometry_id and not expected.source_geometry_id: + out.append( + diagnostic( + "geometry_unverified", + "the device states no geometry identity: the asset is shown as a " + "reference, not as verified against this device", + ) + ) for preset in assets.camera_presets: if not preset.name.strip(): out.append(diagnostic("asset_rejected", "a camera preset has no name")) diff --git a/shared/src/orisys_app_shared/scene3d/digest.py b/shared/src/orisys_app_shared/scene3d/digest.py index fd22f88..79fce38 100644 --- a/shared/src/orisys_app_shared/scene3d/digest.py +++ b/shared/src/orisys_app_shared/scene3d/digest.py @@ -121,6 +121,26 @@ def binding_id(binding: SurfaceBinding) -> str: ) +def source_geometry_digest(world_xyz: Any) -> str: + """The identity of a source geometry, in the source's own units. + + :func:`geometry_id` is the identity of the *sensor-space* shape, and computing it + needs a statement only a person can make (how many metres one source unit is). A + device cannot make that statement, so it could never verify such an id — and + "the device stamped the asset's own id onto the incoming frames" is not a check at + all. This digest needs nothing but the map file the runtime already loads, so the + acquisition side really can compare it against the asset it is about to draw. + """ + array = np.asarray(world_xyz) + if array.ndim != 3 or array.shape[-1] != 3: + raise error( + "geometry_mismatch", f"world_xyz must be [H,W,3], got {tuple(array.shape)}" + ) + return content_digest( + {"world_xyz": array}, {"frame": "source", "unit": "source", "arrays": "world_xyz"} + ) + + def field_domain_id( *, shape_hw: tuple[int, int], diff --git a/shared/src/orisys_app_shared/scene3d/legacy.py b/shared/src/orisys_app_shared/scene3d/legacy.py index a09a3f2..0df4ab4 100644 --- a/shared/src/orisys_app_shared/scene3d/legacy.py +++ b/shared/src/orisys_app_shared/scene3d/legacy.py @@ -51,6 +51,7 @@ from orisys_app_shared.scene3d.digest import ( content_digest, field_domain_id, geometry_id, + source_geometry_digest, ) #: Keys the importer needs, and what it does when they are missing. @@ -561,6 +562,13 @@ class LegacyFingerImporter: bounds_sensor_m=bounds, field_domain_id=domain.domain_id, shape_hw=domain.shape_hw, + # Unit-free, computed from the map's own geometry: the only geometry identity + # an acquisition-side description can reproduce without the unit statement. + source_geometry_id=( + source_geometry_digest(domain.world_xyz) + if domain.world_xyz is not None + else "" + ), # The map back to the artist's file: many render vertices may share one # source vertex, and an operator asking "which triangle is this?" # deserves an answer rather than a guess. diff --git a/shared/src/orisys_app_shared/scene3d/package.py b/shared/src/orisys_app_shared/scene3d/package.py index ad2cce3..b5a04b5 100644 --- a/shared/src/orisys_app_shared/scene3d/package.py +++ b/shared/src/orisys_app_shared/scene3d/package.py @@ -172,6 +172,19 @@ def build_manifest( "sensor_model": str(sensor_model), "hardware_revisions": [str(item) for item in hardware_revisions], }, + # The unit-free source-geometry identity, so an acquisition-side description can + # verify it (see ``ExpectedGeometry.source_geometry_id``). Absent on a package + # built before this field existed, and then reported as "unverified" rather than + # silently treated as matching. + "source_geometry": ( + { + "id": assets.source_geometry_id, + "arrays": ["world_xyz"], + "unit": "source", + } + if assets.source_geometry_id + else None + ), "field_domain": domain, "parts": [ { @@ -671,6 +684,8 @@ def scene_from_contents( recomputed=recomputed_binding[:16], ) source = arrays.get("sensing/source_vertex.npy") + compatible = dict(manifest.get("compatible") or {}) + source_geometry = dict(manifest.get("source_geometry") or {}) assets = SceneAssets( asset_id=str(manifest.get("asset_id") or ""), asset_version=str(manifest.get("asset_version") or ""), @@ -699,6 +714,11 @@ def scene_from_contents( freeze(source, dtype=np.int32) if source is not None else None ), platform_lock=PlatformLock.from_dict(manifest.get("platform_lock")), + source_geometry_id=str(source_geometry.get("id") or ""), + compatible_sensor_model=str(compatible.get("sensor_model") or ""), + compatible_hardware_revisions=tuple( + str(item) for item in (compatible.get("hardware_revisions") or ()) + ), ) validate_assets(assets, expected) return assets diff --git a/shared/tests/test_scene3d_contracts.py b/shared/tests/test_scene3d_contracts.py index 48e3d9f..b3be101 100644 --- a/shared/tests/test_scene3d_contracts.py +++ b/shared/tests/test_scene3d_contracts.py @@ -293,6 +293,75 @@ def test_the_device_description_decides_whether_an_asset_fits(): assert excinfo.value.diagnostic.code == code +def test_an_unstated_device_identity_is_reported_not_matched(): + """An empty field is "the device cannot say", never "it matches".""" + device = c.ExpectedGeometry("finger", "", "", "domain-xyz", (3, 4), source_geometry_id="") + warnings = c.validate_assets(_assets(), device) + assert [item.code for item in warnings] == ["geometry_unverified"] + assert device.source_geometry_id == "" + + +def test_the_source_geometry_must_match_too(): + """Same size, same mapping, different surface: still not the same geometry.""" + device = c.ExpectedGeometry( + "finger", "", "", "domain-xyz", (3, 4), source_geometry_id="src-1" + ) + assert c.validate_assets(_assets(source_geometry_id="src-1"), device) == () + with pytest.raises(c.ThreeDError) as excinfo: + c.validate_assets(_assets(source_geometry_id="src-2"), device) + assert excinfo.value.diagnostic.code == "geometry_mismatch" + assert excinfo.value.diagnostic.details["device"] == "src-1" + + # A package that predates the record: unverified, and said so. + warnings = c.validate_assets(_assets(), device) + assert [item.code for item in warnings] == ["geometry_unverified"] + + +def test_a_package_released_for_another_sensor_or_revision_is_refused(): + device = c.ExpectedGeometry("finger", "rev-b", "geom-abc", "domain-xyz", (3, 4)) + with pytest.raises(c.ThreeDError) as excinfo: + c.validate_assets(_assets(compatible_sensor_model="box"), device) + assert excinfo.value.diagnostic.code == "geometry_mismatch" + + with pytest.raises(c.ThreeDError) as excinfo: + c.validate_assets( + _assets( + compatible_sensor_model="finger", + compatible_hardware_revisions=("rev-a",), + ), + device, + ) + assert excinfo.value.diagnostic.code == "geometry_mismatch" + # ``*`` is "any revision", so it never refuses. + assert c.validate_assets( + _assets( + compatible_sensor_model="finger", + compatible_hardware_revisions=("*",), + ), + device, + ) == () + # An unstated revision on the device side is not a mismatch either. + assert c.validate_assets( + _assets( + compatible_sensor_model="finger", + compatible_hardware_revisions=("rev-a",), + ), + c.ExpectedGeometry("finger", "", "geom-abc", "domain-xyz", (3, 4)), + ) == () + + +def test_the_source_geometry_digest_is_stable_and_shape_checked(): + from orisys_app_shared.scene3d.digest import source_geometry_digest + + world = np.arange(3 * 4 * 3, dtype=np.float32).reshape(3, 4, 3) + assert source_geometry_digest(world) == source_geometry_digest(world.copy()) + moved = world + 1.0 + assert source_geometry_digest(moved) != source_geometry_digest(world) + with pytest.raises(c.ThreeDError) as excinfo: + source_geometry_digest(np.zeros((3, 4), dtype=np.float32)) + assert excinfo.value.diagnostic.code == "geometry_mismatch" + + def test_only_four_vertex_lod_counts_as_the_same_binding(): """A new topology needs a new binding: 1032 and 1005 are not interchangeable.""" binding = _binding(vertices=3) diff --git a/shared/tests/test_scene3d_device_geometry.py b/shared/tests/test_scene3d_device_geometry.py new file mode 100644 index 0000000..2d7eafb --- /dev/null +++ b/shared/tests/test_scene3d_device_geometry.py @@ -0,0 +1,162 @@ +"""Stage 6 acceptance for the *device's own* geometry description. + +The review's R3 finding was that the internal-consistency check was never wired to the +acquisition side: the adapter was built from the asset, so "the frame matches the scene" +was a sentence the asset wrote about itself. These tests are about the other half — the +description the acquisition side derives from the files the runtime actually loads, and +what happens when those files and the package disagree. + +The bundle used here is the shipped one (``apps/studio/config/sensors/finger``); the +tests that need it skip when it is absent, because the module is also imported by runs +that only have the SDK. +""" + +from __future__ import annotations + +import json +import shutil +from dataclasses import replace +from pathlib import Path + +import numpy as np +import pytest + +from orisys_app_shared.acquisition.geometry import ( + describe_sensor_geometry, + map_path_for, +) +from orisys_app_shared.scene3d.contracts import ThreeDError +from orisys_app_shared.scene3d.package import PackagedAssetLoader + +BUNDLE = ( + Path(__file__).resolve().parents[2] + / "apps" + / "studio" + / "config" + / "sensors" + / "finger" +) +PACKAGE = BUNDLE / "scene" / "finger-display-1.0.0.oryscene" + + +def _require_bundle() -> None: + if not (BUNDLE / "sensor.json").is_file(): + pytest.skip("the shipped finger bundle is not present") + + +def _require_package() -> None: + _require_bundle() + if not PACKAGE.is_file(): + pytest.skip("the compiled finger package is not present") + + +def test_the_map_name_follows_the_scale_the_sdk_would_use(tmp_path): + assert map_path_for(tmp_path, 0.4).name == "unroll_map_uv_s0.4.npz" + assert map_path_for(tmp_path, 1.0).name == "unroll_map_uv.npz" + assert map_path_for(tmp_path, None).name == "unroll_map_uv.npz" + + +def test_the_device_description_comes_from_the_files_the_runtime_loads(): + _require_bundle() + device = describe_sensor_geometry("finger", BUNDLE / "sensor.json") + assert device.verified, device.detail + assert device.expected.sensor_model == "finger" + # The metre-scaled identity needs the operator's unit statement; a device that + # cannot make it must say nothing rather than echo the asset's id back. + assert device.expected.geometry_id == "" + assert device.expected.field_domain_id + assert device.expected.source_geometry_id + assert tuple(device.expected.shape_hw) == (300, 280) + assert Path(device.detail["map"]).name == "unroll_map_uv_s0.4.npz" + assert device.detail["shape_hw"] == "300x280" + + +def test_the_shipped_package_passes_the_shipped_device_description(): + _require_package() + device = describe_sensor_geometry("finger", BUNDLE / "sensor.json") + assets = PackagedAssetLoader().load(PACKAGE, device.expected) + assert assets.source_geometry_id == device.expected.source_geometry_id + assert assets.compatible_sensor_model == "finger" + + +def test_a_same_size_different_mapping_is_refused(tmp_path): + """The risk the review named: 300x280 is not one mapping.""" + _require_package() + fake = tmp_path / "finger" + shutil.copytree(BUNDLE, fake) + map_path = fake / "assets" / "30fps-0703" / "unroll_map_uv_s0.4.npz" + with np.load(map_path, allow_pickle=False) as handle: + data = {key: handle[key] for key in handle.files} + # The same grid, the same valid area — a different mapping. + data["map_x"] = np.asarray(data["map_x"], dtype=np.float32).copy() + data["map_x"][0, 0] = float(data["map_x"][0, 0]) + 3.0 + np.savez(map_path, **data) + + shipped = describe_sensor_geometry("finger", BUNDLE / "sensor.json") + other = describe_sensor_geometry("finger", fake / "sensor.json") + assert tuple(other.expected.shape_hw) == tuple(shipped.expected.shape_hw) + assert other.expected.field_domain_id != shipped.expected.field_domain_id, ( + "two different mappings of the same size produced the same domain id" + ) + with pytest.raises(ThreeDError) as excinfo: + PackagedAssetLoader().load(PACKAGE, other.expected) + assert excinfo.value.diagnostic.code == "field_domain_mismatch" + + +def test_a_changed_mesh_behind_the_same_map_is_refused(tmp_path): + """The other half: the map is identical, the surface behind it is not.""" + _require_package() + device = describe_sensor_geometry("finger", BUNDLE / "sensor.json") + other_geometry = replace(device.expected, source_geometry_id="somebody-elses-surface") + with pytest.raises(ThreeDError) as excinfo: + PackagedAssetLoader().load(PACKAGE, other_geometry) + assert excinfo.value.diagnostic.code == "geometry_mismatch" + + +def test_a_bundle_that_cannot_be_described_is_unverified_not_a_match(tmp_path): + _require_bundle() + unknown = tmp_path / "nowhere" + device = describe_sensor_geometry("finger", unknown) + assert device.verified is False + assert device.expected.geometry_id == "" + assert device.expected.source_geometry_id == "" + assert "could not be located" in device.detail["reason"] + + +def test_a_bundle_whose_map_disagrees_with_the_config_is_refused(tmp_path): + """``resolution_stitch`` and the map are two statements of one grid.""" + _require_bundle() + fake = tmp_path / "finger" + shutil.copytree(BUNDLE, fake) + config = json.loads((fake / "sensor.json").read_text(encoding="utf-8")) + config["camera"]["resolution_stitch"] = [280, 299] + (fake / "sensor.json").write_text( + json.dumps(config, ensure_ascii=False), encoding="utf-8" + ) + with pytest.raises(ThreeDError) as excinfo: + describe_sensor_geometry("finger", fake / "sensor.json") + assert excinfo.value.diagnostic.code == "field_domain_mismatch" + + +def test_a_missing_map_names_the_file_it_looked_for(tmp_path): + _require_bundle() + fake = tmp_path / "finger" + (fake / "assets" / "30fps-0703").mkdir(parents=True) + (fake / "manifest.json").write_text( + json.dumps({"assets_dir": "assets/30fps-0703"}), encoding="utf-8" + ) + shutil.copy(BUNDLE / "sensor.json", fake / "sensor.json") + with pytest.raises(ThreeDError) as excinfo: + describe_sensor_geometry("finger", fake / "sensor.json") + assert excinfo.value.diagnostic.code == "asset_rejected" + assert "unroll_map_uv_s0.4.npz" in excinfo.value.diagnostic.message + + +def test_a_bundle_without_an_assets_dir_is_unverified(tmp_path): + fake = tmp_path / "finger" + fake.mkdir() + (fake / "manifest.json").write_text(json.dumps({"name": "finger"}), encoding="utf-8") + (fake / "sensor.json").write_text(json.dumps({"camera": {}}), encoding="utf-8") + device = describe_sensor_geometry("finger", fake / "sensor.json") + assert device.verified is False + assert "assets_dir" in device.detail["reason"] diff --git a/shared/tests/test_scene3d_package.py b/shared/tests/test_scene3d_package.py index 6fdf293..99b07cf 100644 --- a/shared/tests/test_scene3d_package.py +++ b/shared/tests/test_scene3d_package.py @@ -416,3 +416,31 @@ def test_the_envelope_names_the_magic_and_the_payload_state(package): assert envelope["payload"] == "plain" assert envelope["entry_count"] == len(read_package(package).arrays) assert "signature" not in envelope, "a plain package must not look signed" + + +# ------------------------------------------- the device-verifiable geometry record +def test_the_source_geometry_identity_round_trips(tmp_path, scene): + """The record a device can reproduce without the operator's unit statement.""" + from dataclasses import replace + + marked = replace(scene, source_geometry_id="src-46a03a5f") + path = tmp_path / "marked.oryscene" + write_package(marked, path) + manifest = read_package(path).manifest + assert manifest["source_geometry"]["id"] == "src-46a03a5f" + assert manifest["source_geometry"]["unit"] == "source" + rebuilt = scene_from_contents(read_package(path)) + assert rebuilt.source_geometry_id == "src-46a03a5f" + assert rebuilt.compatible_sensor_model == "finger" + assert rebuilt.compatible_hardware_revisions == ("*",) + + +def test_a_package_without_the_record_reads_as_unstated(tmp_path, scene): + """An older package must not look as if its geometry had been checked.""" + path = tmp_path / "plain.oryscene" + write_package(scene, path) # the synthetic scene states no source geometry + manifest = read_package(path).manifest + assert manifest["source_geometry"] is None + rebuilt = scene_from_contents(read_package(path)) + assert rebuilt.source_geometry_id == "" + assert rebuilt.compatible_sensor_model == "finger" diff --git a/tools/scene_assets/README.md b/tools/scene_assets/README.md index 9634963..f474541 100644 --- a/tools/scene_assets/README.md +++ b/tools/scene_assets/README.md @@ -56,6 +56,13 @@ difference, and a wrong factor looks plausible. Expected result: `1032 vertices`, `1974 triangles`, `geometry_id` starting `1a2e0e92…`, a 56 KB package, and `validate_scene.py` reporting every check `ok`. +The package also records a **unit-free** `source_geometry_id` (`46a03a5f…` for the +shipped bundle). It is the only geometry identity the acquisition side can reproduce +without the unit statement above, so it is what lets the running application verify +that the scene it is drawing was built from the same surface — and the same mapping, +via `field_domain_id` — that the device is actually running. A package that predates +this record is still accepted, and reported as *unverified* rather than as a match. + ## Where the application looks for a package 1. `preference.json` → `"scene3d_package": ""` (an absolute path; also the way diff --git a/tools/scene_assets/compile_scene.py b/tools/scene_assets/compile_scene.py index b3c41ef..7791f13 100644 --- a/tools/scene_assets/compile_scene.py +++ b/tools/scene_assets/compile_scene.py @@ -229,6 +229,7 @@ def main(argv: list[str] | None = None) -> int: "triangles": report.part_triangle_counts, "geometry_id": assets.geometry_id, "field_domain_id": assets.field_domain_id, + "source_geometry_id": assets.source_geometry_id, "envelope": envelope, "platform_lock": ( assets.platform_lock.to_dict() if assets.platform_lock is not None else None @@ -242,6 +243,10 @@ def main(argv: list[str] | None = None) -> int: f"triangles={report.part_triangle_counts}" ) print(f" geometry_id={assets.geometry_id}") + print( + " source_geometry_id=" + + (assets.source_geometry_id or "(none: no world_xyz in the map)") + ) print(f" asset_hash={report.asset_hash}") print( f" envelope=v{envelope['envelope_version']} {envelope['payload']}" diff --git a/tools/scene_assets/validate_scene.py b/tools/scene_assets/validate_scene.py index 61f026e..b4112a5 100644 --- a/tools/scene_assets/validate_scene.py +++ b/tools/scene_assets/validate_scene.py @@ -126,6 +126,7 @@ def _legacy_report(args) -> tuple[int, dict]: geometry_id=assets.geometry_id, field_domain_id=assets.field_domain_id, shape_hw=assets.shape_hw, + source_geometry_id=assets.source_geometry_id, ) try: validate_assets(assets, expected) @@ -133,6 +134,12 @@ def _legacy_report(args) -> tuple[int, dict]: except ThreeDError as exc: record("device description", False, exc.diagnostic.code) report["geometry_id"] = assets.geometry_id + report["field_domain_id"] = assets.field_domain_id + report["source_geometry_id"] = assets.source_geometry_id + report["compatible"] = { + "sensor_model": assets.compatible_sensor_model, + "hardware_revisions": list(assets.compatible_hardware_revisions), + } report["asset_hash"] = assets.asset_hash report["binding"] = { "rows": int(assets.binding.sample_indices.shape[0]), @@ -239,6 +246,7 @@ def _package_report(args) -> tuple[int, dict]: str(contents.manifest.get("geometry_id")), str((contents.manifest.get("field_domain") or {}).get("id")), tuple(int(v) for v in (contents.manifest.get("field_domain") or {}).get("shape_hw", (0, 0))), + str(((contents.manifest.get("source_geometry") or {}) or {}).get("id") or ""), ) if args.sensor_model else None @@ -246,6 +254,16 @@ def _package_report(args) -> tuple[int, dict]: assets = scene_from_contents(contents, expected) record("scene builds and validates", True, f"{assets.sensing_part().positions_sensor_m.shape[0]} vertices") + record( + "source geometry recorded", + # Not a failure: a package without the record is still publishable, it just + # cannot be compared with a device's own geometry. ``--require-signature`` + # is where a release gets to demand more. + True, + assets.source_geometry_id[:16] if assets.source_geometry_id else + "not recorded (this package predates the record: a device reports it as " + "unverified rather than as a match)", + ) except ThreeDError as exc: record("scene builds and validates", False, f"{exc.diagnostic.code}: {exc.diagnostic.message}")