From fc1aed67ebfe04923a419a2e3086ee8520514d47 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 07:02:35 +0000 Subject: [PATCH] Add $vpr/$vpt/$vpd viewport special variables (v3.5 deferred item) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Picks up the first of the two items deferred at the end of the v3.5 pass. Interpreter gains a setViewport() setter (mirroring the $preview/$t VarRef-fallback pattern) with OpenSCAD-matching defaults so headless evaluation still returns sane numbers instead of undef. MeshBuilder's requestBuild() now takes a ViewportState snapshot, threaded through a pre-populated Interpreter into CsgEvaluator's full evaluate() overload (the same pattern already used for fileTable). Application derives that snapshot from the render camera's pitch/yaw/target/distance at each requestBuild() call site. Verified via a standalone build of the full non-GPU test suite (glm/ spdlog/Catch2 from apt, no vcpkg/Vulkan SDK needed for that target) — 2877 assertions across 533 test cases pass, including new coverage for the defaults, setViewport(), and user-assignment shadowing. MeshBuilder.cpp was independently syntax-checked against a stubbed manifold/VMA — this caught a real GCC quirk (a nested aggregate used as a default argument of its own enclosing class hits "default member initializer required before the end of its enclosing class"), fixed by keeping ViewportState at namespace scope. Application.cpp's own diff is mechanical (7 call-site argument additions plus one small accessor) and could not be compiled in this sandbox (full GLFW/ImGui/Vulkan stack unavailable) but was reviewed by hand against Camera's verified public API. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01MEXUhKnGAnmaLJ4R8PEg7r --- docs/roadmap.md | 12 ++++++--- src/app/Application.cpp | 27 +++++++++++++++----- src/app/Application.h | 8 ++++++ src/app/MeshBuilder.cpp | 20 ++++++++++++--- src/app/MeshBuilder.h | 24 +++++++++++++++--- src/lang/Interpreter.cpp | 8 ++++++ src/lang/Interpreter.h | 21 +++++++++++++++ tests/test_interpreter.cpp | 52 ++++++++++++++++++++++++++++++++++++++ 8 files changed, 155 insertions(+), 17 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index 6fc43d8..ed0f34b 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -84,12 +84,18 @@ cover. Fixed: - [x] `linear_extrude(slices=...)` — was parsed but silently ignored; twist division count now honors it over the `$fn` default - [x] `children([vector])` / `children([range])` — only a single plain-number index worked before - [x] `echo(name=value)` now formats as `name = value`, matching OpenSCAD +- [x] `$vpr`/`$vpt`/`$vpd` viewport special variables — `MeshBuilder`'s + `requestBuild()` now takes a `ViewportState` snapshot (derived from + `Application::currentViewport()`, reading the camera's pitch/yaw/target/ + distance) and plumbs it into a pre-populated `Interpreter` before + evaluation, mirroring how `fileTable` is already threaded through the + `evaluate(result, interp)` overload. Headless/test evaluation (a + default-constructed `Interpreter`) falls back to OpenSCAD's own defaults + (`$vpr=[55,0,25]`, `$vpt=[0,0,0]`, `$vpd=140`) instead of `undef`. No + roll in this camera model, so `$vpr[1]` is always 0. **Explicitly deferred, not done in this pass** (tracked here so they aren't lost, not because they don't matter): -- `$vpr`/`$vpt`/`$vpd` viewport special variables — would need the render - layer's camera state plumbed into the interpreter, a cross-subsystem wire - that doesn't exist today; rarely load-bearing in real `.scad` files. - `cube`/`sphere`/`translate`/etc. are reserved lexer keywords rather than ordinary identifiers, so (unlike real OpenSCAD) a script can't use one of those names as a variable. Real-world impact is low, but it's a genuine diff --git a/src/app/Application.cpp b/src/app/Application.cpp index 853bdf0..bf3f4ca 100644 --- a/src/app/Application.cpp +++ b/src/app/Application.cpp @@ -133,7 +133,7 @@ void Application::run() { m_watcher = std::make_unique( m_state.scadPath, [this](const auto& p) { onFileChanged(p); }); - m_meshBuilder.requestBuild(m_state.scadPath); + m_meshBuilder.requestBuild(m_state.scadPath, currentViewport()); } } @@ -151,7 +151,7 @@ void Application::run() { m_camera.orbitYaw(dt * 0.5f); // ~28.6 deg/s if (m_state.meshDirty.exchange(false) && !m_state.scadPath.empty()) - m_meshBuilder.requestBuild(m_state.scadPath); + m_meshBuilder.requestBuild(m_state.scadPath, currentViewport()); if (auto result = m_meshBuilder.poll()) { m_diagPanel.setDiagnostics(result->diags); @@ -314,6 +314,19 @@ void Application::fitToView() { m_camera.fitToBounds(m_meshBoundsMin, m_meshBoundsMax); } +ViewportState Application::currentViewport() const { + ViewportState vp; + vp.vpr[0] = glm::degrees(m_camera.pitch()); + vp.vpr[1] = 0.0; // no roll in this camera model + vp.vpr[2] = glm::degrees(m_camera.yaw()); + glm::vec3 t = m_camera.target(); + vp.vpt[0] = t.x; + vp.vpt[1] = t.y; + vp.vpt[2] = t.z; + vp.vpd = m_camera.distance(); + return vp; +} + void Application::openFile() { auto path = openFileDialog(); if (path.empty()) return; @@ -334,7 +347,7 @@ void Application::openFile() { m_diagPanel.setScadPath(path); m_watcher = std::make_unique( path, [this](const auto& p) { onFileChanged(p); }); - m_meshBuilder.requestBuild(path); + m_meshBuilder.requestBuild(path, currentViewport()); } void Application::loadStlFile(const std::filesystem::path& path) { @@ -410,7 +423,7 @@ void Application::drawMenuBar() { openFile(); ImGui::Separator(); if (ImGui::MenuItem("Reload", "R")) - m_meshBuilder.requestBuild(m_state.scadPath); + m_meshBuilder.requestBuild(m_state.scadPath, currentViewport()); ImGui::Separator(); bool hasMesh = !m_meshVerts.empty(); if (!hasMesh) ImGui::BeginDisabled(); @@ -503,7 +516,7 @@ void Application::drawPrefsPopup() { if (m_config.warnOverlappingRoots != prev) { m_meshBuilder.setWarnOverlappingRoots(m_config.warnOverlappingRoots); if (!m_state.scadPath.empty()) - m_meshBuilder.requestBuild(m_state.scadPath); + m_meshBuilder.requestBuild(m_state.scadPath, currentViewport()); } ImGui::Spacing(); @@ -576,7 +589,7 @@ void Application::drawImGui() { formatCount(m_stlVertCount).c_str()); } else { if (ImGui::Button("Reload")) - m_meshBuilder.requestBuild(m_state.scadPath); + m_meshBuilder.requestBuild(m_state.scadPath, currentViewport()); ImGui::Separator(); @@ -584,7 +597,7 @@ void Application::drawImGui() { ImGui::Checkbox("Manifold sphere", &m_useManifoldSphere); if (m_useManifoldSphere != prev) { m_meshBuilder.setUseManifoldSphere(m_useManifoldSphere); - m_meshBuilder.requestBuild(m_state.scadPath); + m_meshBuilder.requestBuild(m_state.scadPath, currentViewport()); } ImGui::Separator(); diff --git a/src/app/Application.h b/src/app/Application.h index 3a7ee3c..9de3366 100644 --- a/src/app/Application.h +++ b/src/app/Application.h @@ -51,6 +51,14 @@ class Application { void exportStl(); void loadStlFile(const std::filesystem::path& path); + // Snapshot of m_camera for $vpr/$vpt/$vpd, passed to each requestBuild() + // call so a build reflects the camera as of when the build was queued + // (matching OpenSCAD: these vars update on render, not on every mouse + // drag). No roll in this camera model, so vpr[1] is always 0; vpr[0]/ + // vpr[2] map from pitch/yaw the same way OpenSCAD's own vpr[0] + // (elevation, 0=horizon/90=top-down) and vpr[2] (azimuth) behave. + ViewportState currentViewport() const; + // GLFW callbacks (static → forward to instance) static void onScroll(GLFWwindow* w, double x, double y); static void onMouseButton(GLFWwindow* w, int button, int action, int mods); diff --git a/src/app/MeshBuilder.cpp b/src/app/MeshBuilder.cpp index 76f9ec9..2785842 100644 --- a/src/app/MeshBuilder.cpp +++ b/src/app/MeshBuilder.cpp @@ -3,6 +3,7 @@ #include "csg/CsgEvaluator.h" #include "csg/MeshCache.h" #include "csg/MeshEvaluator.h" +#include "lang/Interpreter.h" #include "lang/SourceLoader.h" #include @@ -68,13 +69,14 @@ MeshBuilder::~MeshBuilder() { m_thread.join(); } -void MeshBuilder::requestBuild(std::filesystem::path path) { +void MeshBuilder::requestBuild(std::filesystem::path path, ViewportState viewport) { int gen = ++m_currentGen; { std::lock_guard lk(m_workMutex); m_hasWork = true; m_workPath = std::move(path); m_workGen = gen; + m_workViewport = viewport; } m_workCv.notify_one(); } @@ -97,6 +99,7 @@ void MeshBuilder::workerLoop() { while (true) { std::filesystem::path path; int gen = 0; + ViewportState viewport; { std::unique_lock lk(m_workMutex); m_workCv.wait(lk, [this] { return m_stop || m_hasWork; }); @@ -105,12 +108,13 @@ void MeshBuilder::workerLoop() { m_hasWork = false; path = m_workPath; gen = m_workGen; + viewport = m_workViewport; } - buildOne(std::move(path), gen); + buildOne(std::move(path), gen, viewport); } } -void MeshBuilder::buildOne(std::filesystem::path path, int gen) { +void MeshBuilder::buildOne(std::filesystem::path path, int gen, ViewportState viewport) { using Clock = std::chrono::steady_clock; auto t0 = Clock::now(); auto elapsedMs = [&] { @@ -165,7 +169,15 @@ void MeshBuilder::buildOne(std::filesystem::path path, int gen) { csgEval.baseDir = path.parent_path(); // for import()'s relative paths csgEval.fileTable = &loaded.files; // per-file diagnostics + relative-path resolution across include/use - auto scene = csgEval.evaluate(ast); + + // Full evaluate() overload (rather than the convenience one) so $vpr/ + // $vpt/$vpd reflect the camera snapshot passed to requestBuild(). + lang::Interpreter interp; + interp.setViewport(viewport.vpr[0], viewport.vpr[1], viewport.vpr[2], viewport.vpt[0], + viewport.vpt[1], viewport.vpt[2], viewport.vpd); + interp.loadAssignments(ast); + interp.loadFunctions(ast); + auto scene = csgEval.evaluate(ast, interp); // Forward echo() output as Info diagnostics for (const auto& msg : scene.echoMessages) { diff --git a/src/app/MeshBuilder.h b/src/app/MeshBuilder.h index d7cc977..f5f0876 100644 --- a/src/app/MeshBuilder.h +++ b/src/app/MeshBuilder.h @@ -13,6 +13,21 @@ namespace chisel::app { +// Snapshot of render-camera state to plumb into $vpr/$vpt/$vpd for a build. +// Defaults match OpenSCAD's own defaults (no camera() statement) so a caller +// with no live camera (tests, CLI use) can pass ViewportState{} and still +// get sane values instead of undef — see Interpreter::setViewport. Kept at +// namespace scope rather than nested in MeshBuilder: a nested aggregate used +// as a default argument of its own enclosing class's member function hits a +// "default member initializer required before the end of its enclosing +// class" error under GCC, since default-member-initializer resolution and +// default-function-argument resolution both defer to end-of-class. +struct ViewportState { + double vpr[3] = {55.0, 0.0, 25.0}; // [x,y,z] rotation degrees + double vpt[3] = {0.0, 0.0, 0.0}; // [x,y,z] translation (orbit target) + double vpd = 140.0; // viewport distance +}; + enum class BuildPhase : int { Idle, Parsing, @@ -59,8 +74,10 @@ class MeshBuilder { ~MeshBuilder(); // stops and joins the worker thread // Queue a new build. If a build is already running, its result will be - // discarded when poll() is next called. - void requestBuild(std::filesystem::path path); + // discarded when poll() is next called. `viewport` is snapshotted at + // request time and used to set $vpr/$vpt/$vpd for this build only — + // callers that don't care (or have no camera) can omit it. + void requestBuild(std::filesystem::path path, ViewportState viewport = {}); void setUseManifoldSphere(bool v) noexcept { m_useManifoldSphere.store(v); } void setWarnOverlappingRoots(bool v) noexcept { m_warnOverlappingRoots.store(v); } @@ -80,7 +97,7 @@ class MeshBuilder { private: void workerLoop(); - void buildOne(std::filesystem::path path, int gen); + void buildOne(std::filesystem::path path, int gen, ViewportState viewport); // Owned by and only ever touched from the worker thread (workerLoop()/ // buildOne() below) — never accessed from the main thread, so it needs @@ -100,6 +117,7 @@ class MeshBuilder { bool m_hasWork = false; std::filesystem::path m_workPath; int m_workGen = 0; + ViewportState m_workViewport; // Incremented by requestBuild(); read by poll() to detect stale results. std::atomic m_currentGen{0}; diff --git a/src/lang/Interpreter.cpp b/src/lang/Interpreter.cpp index d249915..da09016 100644 --- a/src/lang/Interpreter.cpp +++ b/src/lang/Interpreter.cpp @@ -90,6 +90,14 @@ Value Interpreter::evaluate(const ExprNode& expr) { if (node.name == "$parent_modules") return Value::fromNumber(m_moduleNameStack.empty() ? 0.0 : static_cast(m_moduleNameStack.size() - 1)); + if (node.name == "$vpr") + return Value::fromVec({Value::fromNumber(m_vpr[0]), Value::fromNumber(m_vpr[1]), + Value::fromNumber(m_vpr[2])}); + if (node.name == "$vpt") + return Value::fromVec({Value::fromNumber(m_vpt[0]), Value::fromNumber(m_vpt[1]), + Value::fromNumber(m_vpt[2])}); + if (node.name == "$vpd") + return Value::fromNumber(m_vpd); return Value::undef(); } diff --git a/src/lang/Interpreter.h b/src/lang/Interpreter.h index 8a21fbb..bc97e19 100644 --- a/src/lang/Interpreter.h +++ b/src/lang/Interpreter.h @@ -83,11 +83,32 @@ class Interpreter { void pushModuleName(std::string name) { m_moduleNameStack.push_back(std::move(name)); } void popModuleName() { m_moduleNameStack.pop_back(); } + // Sets $vpr/$vpt/$vpd (viewport rotation/translation/distance), backing + // those special variables' VarRef fallback below. Left at their + // OpenSCAD-matching defaults (see member initializers) unless the caller + // has real render-camera state to plumb in (MeshBuilder does this, + // deriving vpr from Camera's yaw/pitch — see MeshBuilder::buildOne); + // headless evaluation (tests, CLI use) just keeps the defaults. + void setViewport(double vprX, double vprY, double vprZ, + double vptX, double vptY, double vptZ, + double vpd) { + m_vpr = {vprX, vprY, vprZ}; + m_vpt = {vptX, vptY, vptZ}; + m_vpd = vpd; + } + private: std::unordered_map m_env; std::unordered_map m_funcDefs; std::vector m_moduleNameStack; + // $vpr/$vpt/$vpd defaults match OpenSCAD's own defaults for a file with + // no camera() statement and no GUI camera attached, so headless + // evaluation reads sane numbers rather than undef. + std::array m_vpr{55.0, 0.0, 25.0}; + std::array m_vpt{0.0, 0.0, 0.0}; + double m_vpd = 140.0; + // Guards against unbounded recursion in user-defined functions (e.g. a // missing base case) blowing the native call stack. Silent cap, no // diagnostic, matching the existing `for`-loop kMaxIter convention. diff --git a/tests/test_interpreter.cpp b/tests/test_interpreter.cpp index 96ccd8f..9475226 100644 --- a/tests/test_interpreter.cpp +++ b/tests/test_interpreter.cpp @@ -776,6 +776,58 @@ TEST_CASE("Interp:$preview and $t have sane defaults", "[interp][v35]") { REQUIRE(evalNum("$t") == Approx(0.0)); } +// --------------------------------------------------------------------------- +// v3.5 (deferred) — $vpr / $vpt / $vpd +// --------------------------------------------------------------------------- +TEST_CASE("Interp:$vpr/$vpt/$vpd default to OpenSCAD's own defaults", "[interp][v35]") { + Value vpr = evalVal("$vpr"); + REQUIRE(vpr.isVector()); + REQUIRE(vpr.asVec().size() == 3); + REQUIRE(vpr.asVec()[0].asNumber() == Approx(55.0)); + REQUIRE(vpr.asVec()[1].asNumber() == Approx(0.0)); + REQUIRE(vpr.asVec()[2].asNumber() == Approx(25.0)); + + Value vpt = evalVal("$vpt"); + REQUIRE(vpt.isVector()); + REQUIRE(vpt.asVec().size() == 3); + REQUIRE(vpt.asVec()[0].asNumber() == Approx(0.0)); + REQUIRE(vpt.asVec()[1].asNumber() == Approx(0.0)); + REQUIRE(vpt.asVec()[2].asNumber() == Approx(0.0)); + + REQUIRE(evalNum("$vpd") == Approx(140.0)); +} + +TEST_CASE("Interp:setViewport overrides $vpr/$vpt/$vpd", "[interp][v35]") { + Lexer lexer("_a = $vpr; _b = $vpt; _c = $vpd;"); + auto tokens = lexer.tokenize(); + Parser parser(std::move(tokens)); + auto result = parser.parse(); + REQUIRE(result.assignments.size() == 3); + + Interpreter interp; + interp.setViewport(10.0, 20.0, 30.0, 1.0, 2.0, 3.0, 99.0); + + Value vpr = interp.evaluate(*result.assignments[0].value); + REQUIRE(vpr.asVec()[0].asNumber() == Approx(10.0)); + REQUIRE(vpr.asVec()[1].asNumber() == Approx(20.0)); + REQUIRE(vpr.asVec()[2].asNumber() == Approx(30.0)); + + Value vpt = interp.evaluate(*result.assignments[1].value); + REQUIRE(vpt.asVec()[0].asNumber() == Approx(1.0)); + REQUIRE(vpt.asVec()[1].asNumber() == Approx(2.0)); + REQUIRE(vpt.asVec()[2].asNumber() == Approx(3.0)); + + Value vpd = interp.evaluate(*result.assignments[2].value); + REQUIRE(vpd.asNumber() == Approx(99.0)); +} + +TEST_CASE("Interp:$vpr/$vpt/$vpd can be shadowed by a user assignment", "[interp][v35]") { + // Matches OpenSCAD: these are defaults, not protected keywords. + Interpreter interp = loadEnv("$vpd = 7; $vpt = [1,1,1];"); + REQUIRE(interp.getVar("$vpd").asNumber() == Approx(7.0)); + REQUIRE(interp.getVar("$vpt").asVec()[0].asNumber() == Approx(1.0)); +} + // --------------------------------------------------------------------------- // v3.5 — is_undef / is_bool / is_num / is_string / is_list / is_function // ---------------------------------------------------------------------------