diff --git a/bindings/pycvc/CMakeLists.txt b/bindings/pycvc/CMakeLists.txt index 09329a5c..17f5d406 100644 --- a/bindings/pycvc/CMakeLists.txt +++ b/bindings/pycvc/CMakeLists.txt @@ -213,7 +213,8 @@ if(CVC_BUILD_PYCVC_CORE) pycvc_dataprep:test_pycvc_dataprep.py pycvc_volslice:test_pycvc_volslice.py pycvc_nav_train:test_pycvc_nav_train.py - pycvc_integration:test_pycvc_integration.py) + pycvc_integration:test_pycvc_integration.py + pycvc_proxy_hooks:test_pycvc_proxy_hooks.py) string(REPLACE ":" ";" _pair "${_t}") list(GET _pair 0 _name) list(GET _pair 1 _script) @@ -332,45 +333,67 @@ if(CVC_BUILD_PYCVC_GL) install(DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/pymod_gl/" DESTINATION "${_PYCVC_SITEDIR}/pycvc_gl" COMPONENT pycvc_gl FILES_MATCHING PATTERN "*.py") + # Mirror that INSTALLED package layout in the build tree (pkg/pycvc_gl/: the proxy as + # __init__.py, the extension inside, pymod_gl/*.py as submodules) and run the pycvc_gl + # tests against it, so they import pycvc_gl exactly as a consumer does. The flat + # pycvc_gl.py beside _pycvc_gl.so is a plain module, not a package, so + # `pycvc_gl.camera` et al. could never resolve from the build tree. + set(_PYCVC_GL_PKGROOT "${CMAKE_CURRENT_BINARY_DIR}/pkg") + set(_PYCVC_GL_PKG "${_PYCVC_GL_PKGROOT}/pycvc_gl") + file(GLOB _PYCVC_GL_PYMODS CONFIGURE_DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/pymod_gl/*.py") + add_custom_command( + OUTPUT "${_PYCVC_GL_PKGROOT}/pycvc_gl.staged" + COMMAND ${CMAKE_COMMAND} -E make_directory "${_PYCVC_GL_PKG}" + COMMAND ${CMAKE_COMMAND} -E copy_if_different "$" "${_PYCVC_GL_PKG}/" + COMMAND ${CMAKE_COMMAND} -E copy_if_different "${CMAKE_CURRENT_BINARY_DIR}/pycvc_gl.py" + "${_PYCVC_GL_PKG}/__init__.py" + COMMAND ${CMAKE_COMMAND} -E copy_if_different ${_PYCVC_GL_PYMODS} "${_PYCVC_GL_PKG}/" + COMMAND ${CMAKE_COMMAND} -E touch "${_PYCVC_GL_PKGROOT}/pycvc_gl.staged" + DEPENDS pycvc_gl ${_PYCVC_GL_PYMODS} + COMMENT "Staging the pycvc_gl package layout for the build-tree tests" + VERBATIM) + add_custom_target(pycvc_gl_pkg ALL DEPENDS "${_PYCVC_GL_PKGROOT}/pycvc_gl.staged") + # The package root first (so `pycvc_gl` is the package), then the binary dir (pycvc). + set(_PYCVC_GL_TEST_PYTHONPATH "${_PYCVC_GL_PKGROOT}:${CMAKE_CURRENT_BINARY_DIR}") add_test(NAME pycvc_gl_smoke COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_gl.py) set_tests_properties(pycvc_gl_smoke PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # AriRuntime: load demo.ari + run the frame loop from Python. Skips (exit 0) without a GL context. add_test(NAME pycvc_ari COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_ari.py) set_tests_properties(pycvc_ari PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # VTK Python bridge test — skips gracefully unless the vtk-python wrappers # (vtkmodules) are importable; runs the vtkProp<->vtkActor round trip when they are. add_test(NAME pycvc_vtk COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_vtk.py) set_tests_properties(pycvc_vtk PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # Phase-5 zero-copy texture: node.set_texture + image.numpy() live edit + # texture_modified (the strong VTK-inspection check self-skips without vtkmodules). add_test(NAME pycvc_texture COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_texture.py) set_tests_properties(pycvc_texture PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # CameraController: navigation + full cvc::state config (both directions), # construct from an injected app OR a SceneRenderer, canonical viewer path. add_test(NAME pycvc_gl_camera COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_gl_camera.py) set_tests_properties(pycvc_gl_camera PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # world_units scene surface: GraphicsNode dimensions through the transform # chain, the built-in grid/axis nodes, lights, VolRenNode + volren value types. add_test(NAME pycvc_gl_world COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_gl_world.py) set_tests_properties(pycvc_gl_world PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # Dear ImGui overlay + state-bound ui panels: binding presence + an offscreen # overlay round-trip (self-skips without offscreen GL). add_test(NAME pycvc_gl_imgui COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_gl_imgui.py) set_tests_properties(pycvc_gl_imgui PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # ViewportManager / Viewport: picture-in-picture from Python — N viewports over # separate scenes + a mirror, the input router (viewportAt, focus-follows-click # into cvc::state, per-viewport camera), region tuples, frameRGB bytes @@ -378,12 +401,12 @@ if(CVC_BUILD_PYCVC_GL) add_test(NAME pycvc_gl_viewport COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_gl_viewport.py) set_tests_properties(pycvc_gl_viewport PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") # ChaseCamera parity: the native (C++ Track) chase camera returns the same pose # as the pure-Python ChaseCamera on one position stream — the Python side of # "the cameras work the same" (its C++ side is cvcgl_track_parity). add_test(NAME pycvc_gl_chase_parity COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_SOURCE_DIR}/test_pycvc_gl_chase_parity.py) set_tests_properties(pycvc_gl_chase_parity PROPERTIES ENVIRONMENT - "PYTHONPATH=${CMAKE_CURRENT_BINARY_DIR};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") + "PYTHONPATH=${_PYCVC_GL_TEST_PYTHONPATH};LD_LIBRARY_PATH=${_PYCVC_GL_TEST_LIBPATH}${_PYCVC_TEST_LIBPATH}:$ENV{LD_LIBRARY_PATH}") endif() # CVC_BUILD_PYCVC_GL diff --git a/bindings/pycvc/pycvc_ari.i b/bindings/pycvc/pycvc_ari.i index 40fff366..147abc65 100644 --- a/bindings/pycvc/pycvc_ari.i +++ b/bindings/pycvc/pycvc_ari.i @@ -44,10 +44,11 @@ // Python — the Python surface loads .ari files instead. %ignore cvc::gl::ariadne::AriRuntime::set_root; -// Keep the window/camera/overlay the AriRuntime borrows alive for its whole life. +// Keep the window/camera/overlay the AriRuntime borrows alive for its whole life. The ctor has a +// single signature, so its proxy takes NAMED parameters (no `args`; see the %pythonappend note in +// pycvc_gl.i). %pythonappend cvc::gl::ariadne::AriRuntime::AriRuntime %{ - if len(args) >= 3: - self._pycvc_view, self._pycvc_cam, self._pycvc_overlay = args[0], args[1], args[2] + self._pycvc_view, self._pycvc_cam, self._pycvc_overlay = view, cam, ui %} %include "cvc/gl/ariadne/AriRuntime.h" diff --git a/bindings/pycvc/pycvc_gl.i b/bindings/pycvc/pycvc_gl.i index 6dfa6ea0..d1a31eb0 100644 --- a/bindings/pycvc/pycvc_gl.i +++ b/bindings/pycvc/pycvc_gl.i @@ -258,8 +258,16 @@ except Exception: # pragma: no cover -- VTK python bindings are optional // A Python-CONSTRUCTED node (a director subclass built as MyNode(app, path, // name)) must keep its app alive too — its ~SceneNode touches the app's state -// tree by raw reference. args[0] is the app the node ctor takes. (Nodes obtained -// from a SceneGraph get this via the SceneGraph appends below instead.) +// tree by raw reference. (Nodes obtained from a SceneGraph get this via the +// SceneGraph appends below instead.) +// +// NB on %pythonappend: SWIG emits `def f(self, *args)` ONLY for an overloaded +// function; a single-signature one gets NAMED parameters (`def f(self, ctx, +// statePath, name)`), where `args` is undefined and the append raises NameError +// on every call. So an overloaded ctor reads args[0], a single-signature one +// names its parameter — and adding/removing an overload flips which applies. +// test_pycvc_proxy_hooks.py fails the build's tests on any proxy that references +// an undefined name, so a flip cannot slip through silently. %pythonappend cvc::gl::GraphicsNode::GraphicsNode %{ if args: self._pycvc_app = args[0] %} @@ -270,7 +278,7 @@ except Exception: # pragma: no cover -- VTK python bindings are optional if args: self._pycvc_app = args[0] %} %pythonappend cvc::gl::VolRenNode::VolRenNode %{ - if args: self._pycvc_app = args[0] + self._pycvc_app = ctx # single signature -> named-parameter proxy %} %pythonappend cvc::gl::LightNode::LightNode %{ if args: self._pycvc_app = args[0] @@ -282,7 +290,7 @@ except Exception: # pragma: no cover -- VTK python bindings are optional if args: self._pycvc_app = args[0] %} %pythonappend cvc::gl::VolSliceNode::VolSliceNode %{ - if args: self._pycvc_app = args[0] + self._pycvc_app = ctx # single signature -> named-parameter proxy %} // ── SceneNode (abstract base): trim VTK / threading internals ─────────────── @@ -978,6 +986,13 @@ def _typed_node(sg, name): // pickWorld has a double[3] OUT param SWIG can't express; re-exposed as pick_world // below (returns an (x,y,z) tuple or None). %ignore cvc::gl::SceneRenderer::pickWorld; +// Keep the borrowed SceneGraph alive: ~SceneRenderer detaches from it (close() -> +// ~ViewportManager -> SceneGraph::setRenderer), so a scene released FIRST — e.g. a +// function returning drops its `sg` local before `view` — was a use-after-free +// segfault at teardown. (Default args make this an overloaded, *args proxy.) +%pythonappend cvc::gl::SceneRenderer::SceneRenderer %{ + if args: self._pycvc_scene = args[0] +%} %include "cvc/gl/SceneRenderer.h" %extend cvc::gl::SceneRenderer { @@ -1109,9 +1124,9 @@ def _typed_node(sg, name): %pythonappend cvc::gl::ViewportManager::activeViewport %{ if val is not None: val._pycvc_keepalive = self %} -// addSceneViewport is a named-parameter proxy (no *args), so reference the -// `scene` argument by name — `args` is undefined here (only the *args ctor -// wrappers get it), which raised NameError the moment a viewport was added. +// addSceneViewport is a named-parameter proxy (single signature, no *args), so +// reference the `scene` argument by name — `args` is undefined here, which raised +// NameError the moment a viewport was added. (See the %pythonappend note above.) %pythonappend cvc::gl::ViewportManager::addSceneViewport %{ if val is not None: val._pycvc_keepalive = self diff --git a/bindings/pycvc/pycvc_imgui.i b/bindings/pycvc/pycvc_imgui.i index 48e2457b..9052f9fa 100644 --- a/bindings/pycvc/pycvc_imgui.i +++ b/bindings/pycvc/pycvc_imgui.i @@ -29,12 +29,15 @@ // the HUD to the shared state-driven controller, the same seam in C++ and Python. %ignore cvc::gl::ImGuiOverlay::imguiContext; // opaque ImGuiContext* // Keep the injected viewer alive: ~ImGuiOverlay detaches its window observers. +// Both are single-signature, i.e. NAMED-parameter proxies: reference the parameter +// by name (`args` does not exist there; see the %pythonappend note in pycvc_gl.i). %pythonappend cvc::gl::ImGuiOverlay::ImGuiOverlay %{ - if args: self._pycvc_keepalive = args[0] + self._pycvc_keepalive = viewer %} -// Keep the attached camera alive: the overlay holds it by raw pointer. +// Keep the attached camera alive: the overlay holds it by raw pointer (None detaches +// and releases it). %pythonappend cvc::gl::ImGuiOverlay::attachCamera %{ - if args: self._pycvc_camera = args[0] + self._pycvc_camera = cam %} %include "cvc/gl/ImGuiOverlay.h" diff --git a/bindings/pycvc/test_pycvc_gl_imgui.py b/bindings/pycvc/test_pycvc_gl_imgui.py index 8d028407..a4395f3d 100644 --- a/bindings/pycvc/test_pycvc_gl_imgui.py +++ b/bindings/pycvc/test_pycvc_gl_imgui.py @@ -124,7 +124,14 @@ def check(what, ok): ov.setDrawCallback(lambda: None) # a Python draw callback (reuses the callable typemap) check("draw callback accepted", True) ov.setUiScale(1.5) - check("uiScale round-trips", abs(ov.uiScale() - 1.5) < 1e-6) + if ov.enabled(): + check("uiScale round-trips", abs(ov.uiScale() - 1.5) < 1e-6) + else: + # Inert overlay (libcvc built without CVC_ENABLE_IMGUI, or setup failed): every + # method is a documented no-op and uiScale() stays 1.0, so there is nothing to + # round-trip. (This check never ran before: ImGuiOverlay(view) raised NameError in + # its keepalive hook, which the except below reported as a SKIP.) + print(" skip: uiScale round-trip (overlay inert: enabled() == False)") # exercise the bool controls; their inert-mode (CVC_ENABLE_IMGUI off) value is # not asserted, only that the getters marshal a bool. ov.setVisible(True) diff --git a/bindings/pycvc/test_pycvc_proxy_hooks.py b/bindings/pycvc/test_pycvc_proxy_hooks.py new file mode 100644 index 00000000..4e475ae4 --- /dev/null +++ b/bindings/pycvc/test_pycvc_proxy_hooks.py @@ -0,0 +1,93 @@ +"""Guard: no SWIG proxy function references an undefined name. + +A %pythonappend / %pythonprepend body is pasted verbatim into the generated proxy, +whose signature SWIG picks per function: `def f(self, *args)` when the C++ function +is OVERLOADED, NAMED parameters (`def f(self, viewer)`) when it has a single +signature. A hook written for one shape breaks on the other -- `args[0]` in a +named-parameter proxy (or `viewer` in an *args one) is a NameError on EVERY call, +e.g. ImGuiOverlay(view) / AriRuntime(view, cam, ui) could not be constructed at all. +Adding or removing a C++ overload silently flips the shape, so check the generated +modules statically instead of hoping each hook is exercised by some test. + +Resolution is deliberately simple: a name is defined if it is a parameter or bound +anywhere inside the function (nested lambdas/comprehensions included), a global of +the imported module, or a builtin. +""" + +import ast +import builtins +import importlib + +fails = 0 + + +def _bound_names(fn): + names = set() + for node in ast.walk(fn): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.Lambda)): + a = node.args + for arg in a.posonlyargs + a.args + a.kwonlyargs: + names.add(arg.arg) + if a.vararg: + names.add(a.vararg.arg) + if a.kwarg: + names.add(a.kwarg.arg) + if not isinstance(node, ast.Lambda): + names.add(node.name) + elif isinstance(node, ast.Name) and isinstance(node.ctx, (ast.Store, ast.Del)): + names.add(node.id) + elif isinstance(node, (ast.Import, ast.ImportFrom)): + for alias in node.names: + names.add((alias.asname or alias.name).split(".")[0]) + elif isinstance(node, ast.ExceptHandler) and node.name: + names.add(node.name) + elif isinstance(node, (ast.Global, ast.Nonlocal)): + names.update(node.names) + return names + + +def _functions(tree): + """Yield (qualname, FunctionDef) for module-level functions and class methods.""" + for node in tree.body: + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + yield node.name, node + elif isinstance(node, ast.ClassDef): + for item in node.body: + if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)): + yield "%s.%s" % (node.name, item.name), item + + +def check_module(modname): + global fails + try: + mod = importlib.import_module(modname) + except ImportError as exc: + print(" skip: %s not importable (%s)" % (modname, exc)) + return + path = mod.__file__ + with open(path, encoding="utf-8") as f: + tree = ast.parse(f.read(), filename=path) + known = set(vars(mod)) | set(dir(builtins)) + bad = {} # (qualname, name) -> first line, one report per offending function+name + for qual, fn in _functions(tree): + bound = _bound_names(fn) + for node in ast.walk(fn): + if isinstance(node, ast.Name) and isinstance(node.ctx, ast.Load): + if node.id not in bound and node.id not in known: + bad.setdefault((qual, node.id), node.lineno) + bad = ["%s:%d %s() uses undefined name %r" % (path, line, qual, name) + for (qual, name), line in sorted(bad.items(), key=lambda kv: kv[1])] + if bad: + fails += len(bad) + for b in bad: + print(" [FAIL] " + b) + else: + print(" ok: %s -- every proxy function's names resolve" % modname) + + +if __name__ == "__main__": + check_module("pycvc") + check_module("pycvc_gl") + if fails: + raise SystemExit("%d undefined-name reference(s) in SWIG proxies" % fails) + print("PASS") diff --git a/src/cvcGL/CMakeLists.txt b/src/cvcGL/CMakeLists.txt index 7816eb08..94731736 100644 --- a/src/cvcGL/CMakeLists.txt +++ b/src/cvcGL/CMakeLists.txt @@ -467,6 +467,13 @@ add_executable(cvcgl_stage_caster_truth test/cvcgl_stage_caster_truth.cpp) target_link_libraries(cvcgl_stage_caster_truth PRIVATE cvcGL) add_test(NAME cvcgl_stage_caster_truth COMMAND cvcgl_stage_caster_truth) +# Multi-field StageLighting setters (setStage/setKey/setWash/applyPreset) must keep +# EVERY field: the rig's own change callback re-reads all fields from state, so +# writing them one at a time with it live kept only the first. Headless. +add_executable(cvcgl_stage_setters test/cvcgl_stage_setters.cpp) +target_link_libraries(cvcgl_stage_setters PRIVATE cvcGL) +add_test(NAME cvcgl_stage_setters COMMAND cvcgl_stage_setters) + # ViewportManager composites N Viewports (each its own vtkRenderer + camera + # scene) as SetViewport/SetLayer regions in ONE vtkRenderWindow (one GL context — # mandatory under WASM). Pins the picture-in-picture path the SceneRenderer test diff --git a/src/cvcGL/StageLighting.cpp b/src/cvcGL/StageLighting.cpp index 55bc9258..02286e38 100644 --- a/src/cvcGL/StageLighting.cpp +++ b/src/cvcGL/StageLighting.cpp @@ -169,10 +169,16 @@ void StageLighting::setStage(double cx, double cy, double cz, double radius) { s.cy = cy; s.cz = cz; s.radius = std::max(1e-3, radius); - getState("stage_x").value(s.cx); - getState("stage_y").value(s.cy); - getState("stage_z").value(s.cz); - getState("stage_radius").value(s.radius); + { + // Mirror to state WITHOUT our own change callback: it re-reads every field from + // state, so the first write would pull the not-yet-written fields' OLD values back + // into m_impl and only stage_x would stick. Bound UI still sees every write. + cvc::state_init_scope quiet(*this); + getState("stage_x").value(s.cx); + getState("stage_y").value(s.cy); + getState("stage_z").value(s.cz); + getState("stage_radius").value(s.radius); + } apply(); } @@ -255,7 +261,12 @@ void StageLighting::applyPreset(Preset p) { s.warmth = 0.0; break; } - seedState(); // push the preset out through state so bound UI follows + { + // Push the preset out through state so bound UI follows -- quietly, for the same + // reason as setStage: otherwise only the first changed field of the preset sticks. + cvc::state_init_scope quiet(*this); + seedState(); + } apply(); } @@ -266,10 +277,13 @@ void StageLighting::setKey(double intensity, double azimuthDeg, double elevation s.keyAz = azimuthDeg; s.keyEl = elevationDeg; s.keyCone = coneDeg; - getState("key_intensity").value(s.keyI); - getState("key_azimuth").value(s.keyAz); - getState("key_elevation").value(s.keyEl); - getState("key_cone").value(s.keyCone); + { + cvc::state_init_scope quiet(*this); // see setStage + getState("key_intensity").value(s.keyI); + getState("key_azimuth").value(s.keyAz); + getState("key_elevation").value(s.keyEl); + getState("key_cone").value(s.keyCone); + } apply(); } @@ -290,9 +304,12 @@ void StageLighting::setWash(double intensity, int count, double heightScale) { s.washI = intensity; s.washCount = std::max(0, count); s.washHeight = std::max(0.2, heightScale); - getState("wash_intensity").value(s.washI); - getState("wash_count").value(s.washCount); - getState("wash_height").value(s.washHeight); + { + cvc::state_init_scope quiet(*this); // see setStage + getState("wash_intensity").value(s.washI); + getState("wash_count").value(s.washCount); + getState("wash_height").value(s.washHeight); + } apply(); } diff --git a/src/cvcGL/test/cvcgl_stage_setters.cpp b/src/cvcGL/test/cvcgl_stage_setters.cpp new file mode 100644 index 00000000..c4319e7e --- /dev/null +++ b/src/cvcGL/test/cvcgl_stage_setters.cpp @@ -0,0 +1,117 @@ +// Every field of a multi-field StageLighting setter must stick. +// +// The rig mirrors its settings into cvc::state, and its own change callback +// re-reads EVERY field from state (so a UI edit to one key flows back in). The +// multi-field setters used to write their fields one at a time with that +// callback live: the first write re-read the rest from state -- still holding the +// OLD values -- and the remaining writes then stored those old values right back. +// setStage(1, 2, 3, 10) left the stage at (1, 0, 0, ), setWash kept +// only the intensity, and applyPreset() kept only the first changed field of the +// preset. Nothing here needs a GL context: the rig is headless on a SceneGraph. +#include +#include +#include +#include +#include +#include +#include + +using cvc::gl::SceneGraph; +using cvc::gl::StageLighting; + +static int fails = 0; +static void check(bool ok, const std::string &w) { + std::printf(" %s %s\n", ok ? "PASS" : "FAIL", w.c_str()); + if (!ok) + ++fails; +} + +// A passive probe rooted at the rig's path (see cvcgl_state_binding for why the +// path is derived independently and why the probe is unthreaded). +class Peer : public cvc::state_object { +public: + Peer(cvc::app &c, const std::string &p) : cvc::state_object(c, p) { + this->setInstanceThreading(false); + } + double num(const std::string &k) { + const std::string v = getState(k).value(); + return v.empty() ? std::nan("") : std::stod(v); + } + template void wr(const std::string &k, T v) { getState(k).value(v); } +}; + +static bool near(double a, double b) { return std::fabs(a - b) < 1e-9; } + +static void expect_state(Peer &L, const char *key, double want) { + const double got = L.num(key); + check(near(got, want), + std::string(key) + " = " + std::to_string(got) + " (want " + std::to_string(want) + ")"); +} + +static void expect_stage(StageLighting &rig, double x, double y, double z, double r, + const std::string &what) { + double cx, cy, cz, cr; + rig.stage(cx, cy, cz, cr); + check(near(cx, x) && near(cy, y) && near(cz, z) && near(cr, r), + what + ": stage() = (" + std::to_string(cx) + ", " + std::to_string(cy) + ", " + + std::to_string(cz) + ", " + std::to_string(cr) + ")"); +} + +int main() { + cvc::app app; + app.properties("system.log_verbosity", "0"); + SceneGraph sg(app, "stg"); + StageLighting rig(sg); + Peer L(app, StageLighting::sceneStatePath(sg.getStatePrefix())); + + std::printf("== setStage ==\n"); + // Every coordinate differs from the default (0, 0, 0, 10), so a field that + // falls back to its old value cannot pass by coincidence. + rig.setStage(1.0, 2.0, 3.0, 7.0); + expect_stage(rig, 1.0, 2.0, 3.0, 7.0, "object"); + expect_state(L, "stage_x", 1.0); + expect_state(L, "stage_y", 2.0); + expect_state(L, "stage_z", 3.0); + expect_state(L, "stage_radius", 7.0); + + std::printf("== frameBounds (routes through setStage) ==\n"); + rig.frameBounds(-4.0, -2.0, 0.0, 4.0, 2.0, 10.0); + // centre (0, 0), z = minZ + 0.15 * height, radius = half the footprint diagonal + expect_stage(rig, 0.0, 0.0, 1.5, 0.5 * std::sqrt(8.0 * 8.0 + 4.0 * 4.0), "frameBounds"); + + std::printf("== setKey ==\n"); + rig.setKey(1.2, 45.0, 30.0, 35.0); + expect_state(L, "key_intensity", 1.2); + expect_state(L, "key_azimuth", 45.0); + expect_state(L, "key_elevation", 30.0); + expect_state(L, "key_cone", 35.0); + + std::printf("== setWash ==\n"); + rig.setWash(0.7, 5, 2.5); + expect_state(L, "wash_intensity", 0.7); + expect_state(L, "wash_count", 5.0); + expect_state(L, "wash_height", 2.5); + + std::printf("== applyPreset(Dramatic) ==\n"); + rig.applyPreset(StageLighting::Preset::Dramatic); + expect_state(L, "key_intensity", 1.35); + expect_state(L, "key_elevation", 30.0); + expect_state(L, "key_cone", 22.0); + expect_state(L, "fill_intensity", 0.08); + expect_state(L, "back_intensity", 0.85); + expect_state(L, "wash_intensity", 0.05); + expect_state(L, "wash_count", 2.0); + expect_state(L, "ambient", 0.08); + expect_state(L, "warm_key", 0.55); + + std::printf("== state -> object still flows in ==\n"); + // The setters silence the rig's OWN callback while mirroring; an outside edit + // (a bound UI slider) must still reach the object. + L.wr("stage_radius", 12.5); + double cx, cy, cz, cr; + rig.stage(cx, cy, cz, cr); + check(near(cr, 12.5), "external stage_radius edit reaches the rig: " + std::to_string(cr)); + + std::printf(fails ? "FAILED (%d)\n" : "OK\n", fails); + return fails ? 1 : 0; +}