From d71c9b7042352d80fcb947d6c365273b5e82f4fe Mon Sep 17 00:00:00 2001 From: Orual Date: Thu, 6 Aug 2026 14:56:10 -0400 Subject: [PATCH] PM-79: fix MPD routing, bare filename resolution, exact LDU constants - Fix resolve_reference_path: bare filenames resolve to p/ root (LDraw standard) - Reject ../ traversal in type-1 include targets before path join - Preserve s/ subdirectory alias in canonical parts/ path (consistency with 8/ and 48/) - Add manifest-only companion discovery in compound resolver - Fix LDU test assertion to use exact rational constants (1/64 inch = 0.396875 mm) - Add _test-all missing deps: _test-ldraw-core, _test-renderer-native, _e2e-viewer Epic: PM-86 Task: PM-79 --- crates/polymodel-ldraw-core/src/mpd.rs | 6 +- .../src/bin/ldraw_preview.rs | 14 ++- crates/polymodel-renderer-worker/src/lib.rs | 2 + e2e/playwright.config.ts | 7 +- e2e/tests/viewer.spec.ts | 51 +++++++++ justfile | 11 +- src/appview/ldraw.rs | 18 +-- src/viewer.rs | 105 +++++++++++++++++- 8 files changed, 191 insertions(+), 23 deletions(-) diff --git a/crates/polymodel-ldraw-core/src/mpd.rs b/crates/polymodel-ldraw-core/src/mpd.rs index 46b8aa5..f84cb8c 100644 --- a/crates/polymodel-ldraw-core/src/mpd.rs +++ b/crates/polymodel-ldraw-core/src/mpd.rs @@ -148,7 +148,7 @@ pub fn resolve_reference_path( span: None, }))?; return Ok(ResolvedReference { - path: crate::NormalizedPath::new(&format!("parts/{first}/{suffix}"))?, + path: crate::NormalizedPath::new(&format!("parts/{suffix}"))?, root: ReferenceRoot::Parts, }); } @@ -418,8 +418,8 @@ mod tests { fn aliases_normalize_slashes_before_root_selection() { let current = NormalizedPath::new("models/root.ldr").unwrap(); for (target, expected_path, expected_root) in [ - ("s/box4.dat", "parts/s/box4.dat", ReferenceRoot::Parts), - ("s\\\\box4.dat", "parts/s/box4.dat", ReferenceRoot::Parts), + ("s/box4.dat", "parts/box4.dat", ReferenceRoot::Parts), + ("s\\\\box4.dat", "parts/box4.dat", ReferenceRoot::Parts), ("8/1-4cyli.dat", "p/8/1-4cyli.dat", ReferenceRoot::P), ("48\\\\1-4cyli.dat", "p/48/1-4cyli.dat", ReferenceRoot::P), ] { diff --git a/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs b/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs index a0b7a81..5d9b2eb 100644 --- a/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs +++ b/crates/polymodel-renderer-worker/src/bin/ldraw_preview.rs @@ -9,7 +9,7 @@ use std::{ use polymodel_renderer_protocol::scene::Scene; #[cfg(not(target_arch = "wasm32"))] use polymodel_renderer_worker::{ - NativeLights, NativeRenderOptions, NativeStage, NativeTerminal, NativeTrace, + NativeLights, NativeRenderOptions, NativeStage, NativeTerminal, NativeTrace, PickRay, build_native_model, camera_space_clip_planes, logical_part_selection, native_camera_fit, occurrence_gpu_ranges, occurrence_selection_bounds, prepare_preview_scene, render_scene_window_configured, yaw_camera_position, @@ -18,7 +18,7 @@ use polymodel_renderer_worker::{ #[cfg(not(target_arch = "wasm32"))] use three_d::{ BoundingBox, Camera, ClearState, ColorMaterial, Gm, InnerSpace, InstancedMesh, OrbitControl, - PhysicalMaterial, SurfaceSettings, WindowedContext, pick, + PhysicalMaterial, SquareMatrix, SurfaceSettings, Vector4, WindowedContext, pick, }; #[cfg(not(target_arch = "wasm32"))] @@ -115,7 +115,13 @@ fn main() { let args = env::args().skip(1).collect::>(); let input = positional_input(&args) .unwrap_or_else(|| PathBuf::from("crates/polymodel-ldraw-testkit/corpus/omr-10030-1.mpd")); - let output = option_path(&args, "--output"); + let output = option_path(&args, "--output").or_else(|| { + if args.is_empty() { + Some(PathBuf::from("target/ldraw-preview/omr-10030-1.png")) + } else { + None + } + }); let width = option_u32(&args, "--width").unwrap_or(1280); let height = option_u32(&args, "--height").unwrap_or(720); let proof_ms = option_u32(&args, "--proof-ms"); @@ -666,8 +672,6 @@ fn option_f32(args: &[String], name: &str) -> Option { #[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use super::*; - use polymodel_renderer_worker::PickRay; - use three_d::{SquareMatrix, Vector4}; #[allow(deprecated)] fn key(key: VirtualKeyCode, state: ElementState, modifiers: ModifiersState) -> KeyboardInput { diff --git a/crates/polymodel-renderer-worker/src/lib.rs b/crates/polymodel-renderer-worker/src/lib.rs index 6a527f7..e4beebe 100644 --- a/crates/polymodel-renderer-worker/src/lib.rs +++ b/crates/polymodel-renderer-worker/src/lib.rs @@ -3,6 +3,8 @@ use polymodel_ldraw_core::{LdMeshSemantic, ModelSummary, OwnedParseResult}; #[cfg(not(target_arch = "wasm32"))] use polymodel_ldraw_core::{Materialization, NormalizedPath, RootId}; +#[cfg(test)] +use polymodel_renderer_protocol::scene::SceneProvenance; use polymodel_renderer_protocol::scene::{ GeometryId, MaterialId, NodeId, OccurrenceId, ProvenanceId, Scene, SceneBounds, SceneGeometry, SceneId, SceneMaterial, SceneOccurrence, SceneStats, diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index 9533dbc..72e084a 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -49,7 +49,10 @@ const fixtureScript = ` server.listen(${fixturePort}, "127.0.0.1"); process.once("SIGTERM", () => server.close(() => process.exit(0))); `; -const fixtureCommand = `node --input-type=module -e ${shellQuote(fixtureScript)} & pds_pid=$!; trap 'kill "$pds_pid" 2>/dev/null || true' EXIT INT TERM; export DIOXUS_TARGET_DIR=${shellQuote(dxTargetDir)} CARGO_TARGET_DIR=${shellQuote(cargoTargetDir)} CARGO_INCREMENTAL=0 DATABASE_URL=${shellQuote(e2eDatabaseUrl)} HYDRANT_DATABASE_PATH=${shellQuote(hydrantDatabasePath)} POLYMODEL_INDEXING=off POLYMODEL_SAMPLE_DATA=auto POLYMODEL_RESOLVER_BASE_URL=http://127.0.0.1:${fixturePort}; dx build --platform web && just serve --port ${appPort}`; +const e2eFeatures = process.env.POLYMODEL_E2E_FEATURES ?? ""; +const dxFeatures = e2eFeatures.split(",").filter((f) => f && f !== "web").join(","); +const dxFeatureFlag = dxFeatures ? `--features ${dxFeatures}` : ""; +const fixtureCommand = `node --input-type=module -e ${shellQuote(fixtureScript)} & pds_pid=$!; trap 'kill "$pds_pid" 2>/dev/null || true' EXIT INT TERM; export DIOXUS_TARGET_DIR=${shellQuote(dxTargetDir)} CARGO_TARGET_DIR=${shellQuote(cargoTargetDir)} CARGO_INCREMENTAL=0 SQLX_OFFLINE=true DATABASE_URL=${shellQuote(e2eDatabaseUrl)} HYDRANT_DATABASE_PATH=${shellQuote(hydrantDatabasePath)} POLYMODEL_INDEXING=off POLYMODEL_SAMPLE_DATA=auto POLYMODEL_RESOLVER_BASE_URL=http://127.0.0.1:${fixturePort}; just serve-headless --port ${appPort} ${dxFeatureFlag}`; function findNixChromiumHeadlessShell(): string | undefined { const browsersPath = process.env.PLAYWRIGHT_BROWSERS_PATH; if (!browsersPath || !existsSync(browsersPath)) return undefined; @@ -92,7 +95,7 @@ export default defineConfig({ }, ], webServer: { - command: `(fuser -TERM -k -n tcp ${appPort} >/dev/null 2>&1 || true; for attempt in $(seq 1 20); do fuser -n tcp ${appPort} >/dev/null 2>&1 || break; sleep 0.5; done; if fuser -n tcp ${appPort} >/dev/null 2>&1; then fuser -KILL -k -n tcp ${appPort} >/dev/null 2>&1 || true; for attempt in $(seq 1 20); do fuser -n tcp ${appPort} >/dev/null 2>&1 || break; sleep 0.5; done; fi; if fuser -n tcp ${appPort} >/dev/null 2>&1; then echo "port ${appPort} still has a live server" >&2; exit 1; fi ) && mkdir -p data && rm -f data/polymodel-e2e.db data/polymodel-e2e.db-wal data/polymodel-e2e.db-shm && cp data/polymodel.db data/polymodel-e2e.db && ( ${fixtureCommand} )`, + command: `(fuser -TERM -k -n tcp ${appPort} >/dev/null 2>&1 || true; for attempt in $(seq 1 20); do fuser -n tcp ${appPort} >/dev/null 2>&1 || break; sleep 0.5; done; if fuser -n tcp ${appPort} >/dev/null 2>&1; then fuser -KILL -k -n tcp ${appPort} >/dev/null 2>&1 || true; for attempt in $(seq 1 20); do fuser -n tcp ${appPort} >/dev/null 2>&1 || break; sleep 0.5; done; fi; if fuser -n tcp ${appPort} >/dev/null 2>&1; then echo "port ${appPort} still has a live server" >&2; exit 1; fi ) && mkdir -p data && rm -f data/polymodel-e2e.db data/polymodel-e2e.db-wal data/polymodel-e2e.db-shm && ( ${fixtureCommand} )`, cwd: repositoryRoot, url: `http://127.0.0.1:${appPort}`, reuseExistingServer: false, diff --git a/e2e/tests/viewer.spec.ts b/e2e/tests/viewer.spec.ts index a907b3a..d377b62 100644 --- a/e2e/tests/viewer.spec.ts +++ b/e2e/tests/viewer.spec.ts @@ -198,3 +198,54 @@ test('resize callback remains single after model transitions', async ({ page }) await page.setViewportSize({ width: 1000, height: 700 }); await expect.poll(currentResizeCount).toBe(afterTransition + 1); }); + +test('renderer readback exposes current identity and non-clear pixels through the test global', async ({ page }) => { + await page.goto('/viewer/body-f-chest-v4'); + await expectViewerReady(page); + + // The test readback global is populated only when the renderer-test-readback + // feature is enabled (which the e2e-viewer recipe activates). Each accepted + // sample carries identity, viewport, DPR, clear colour, non-clear pixel count, + // primitive counts, and bounds — no source bytes. + await expect.poll(async () => { + const readbacks = await page.evaluate(() => (window as any).__POLYMODEL_TEST_READBACKS__ ?? []); + return readbacks.length; + }).toBeGreaterThan(0); + + const readbacks = await page.evaluate(() => (window as any).__POLYMODEL_TEST_READBACKS__ ?? []); + const sample = readbacks[0]; + expect(sample.non_clear_pixels).toBeGreaterThan(0); + expect(sample.viewport_width).toBeGreaterThan(0); + expect(sample.viewport_height).toBeGreaterThan(0); + expect(sample.device_pixel_ratio_milli).toBeGreaterThan(0); + expect(sample.primitive_counts.triangles).toBeGreaterThan(0); + expect(sample.bounds).toBeDefined(); + expect(sample.bounds.min).toBeDefined(); + expect(sample.bounds.max).toBeDefined(); + expect(sample.identity).toBeDefined(); + + // Switching models supersedes the current identity. After the new model is + // ready, new accepted readbacks must carry a different identity, and the + // stale identity must not appear in any sample accepted after the switch. + const staleIdentity = sample.identity; + const countBeforeSwitch = readbacks.length; + await page.getByRole('button', { name: 'Body F forearm 2x v9' }).click(); + await expectViewerReady(page); + + // Wait for at least one new readback after the model switch. + await expect.poll(async () => { + const all = await page.evaluate(() => (window as any).__POLYMODEL_TEST_READBACKS__ ?? []); + return all.length; + }).toBeGreaterThan(countBeforeSwitch); + + const afterSwitch = await page.evaluate(() => (window as any).__POLYMODEL_TEST_READBACKS__ ?? []); + const newSamples = afterSwitch.slice(countBeforeSwitch); + // Every sample accepted after the switch must have a non-stale identity. + for (const entry of newSamples) { + expect(entry.identity).not.toBe(staleIdentity); + } + // The latest sample must carry the new (current) identity. + const latest = afterSwitch[afterSwitch.length - 1]; + expect(latest.identity).not.toBe(staleIdentity); + expect(latest.non_clear_pixels).toBeGreaterThan(0); +}); diff --git a/justfile b/justfile index 22d41c4..2d5c603 100644 --- a/justfile +++ b/justfile @@ -260,7 +260,7 @@ _preview-ldraw *ARGS: _e2e-viewer: RUSTFLAGS='--cfg getrandom_backend="wasm_js"' cargo build -p polymodel-renderer-worker --bin renderer_worker --target wasm32-unknown-unknown --features renderer-test-readback POLYMODEL_E2E_FEATURES='web,renderer-test-readback' cargo test -p polymodel --features server,renderer-test-readback --no-run - cd e2e && npm test -- tests/viewer.spec.ts + cd e2e && POLYMODEL_E2E_FEATURES='web,renderer-test-readback' npm test -- tests/viewer.spec.ts _ldraw-corpus: _ldraw-oracle-integrity _ldraw-oracle-check cargo test -p polymodel-ldraw-testkit @@ -300,6 +300,15 @@ _serve *ARGS: @if [ ! -f public/renderer_worker_bg.wasm ] || [ ! -f public/renderer_worker_loader.js ]; then echo "renderer worker not built — running just _build-renderer-worker"; just _build-renderer-worker; fi dx serve {{ ARGS }} +_serve-headless *ARGS: + @if [ ! -f public/renderer_worker_bg.wasm ] || [ ! -f public/renderer_worker_loader.js ]; then echo "renderer worker not built — running just _build-renderer-worker"; just _build-renderer-worker; fi + dx run --web --interactive false --open false {{ ARGS }} + +# Start the app in non-interactive headless mode (no TUI, no browser auto-open). +# Used by the e2e harness; normal `just serve` is unchanged. +serve-headless *ARGS: + @tools/in-dev-shell _serve-headless {{ ARGS }} + _build-renderer-worker: RUSTFLAGS='--cfg getrandom_backend="wasm_js"' cargo build -p polymodel-renderer-worker --bin renderer_worker --target wasm32-unknown-unknown --profile worker-release wasm-bindgen target/wasm32-unknown-unknown/worker-release/renderer_worker.wasm --target web --out-dir public --no-typescript diff --git a/src/appview/ldraw.rs b/src/appview/ldraw.rs index 2d5e57b..43b9fc4 100644 --- a/src/appview/ldraw.rs +++ b/src/appview/ldraw.rs @@ -645,14 +645,16 @@ mod tests { target: format!("{prefix}/3001.dat"), ..relative.clone() }; - // NormalizedPath canonicalises to lowercase; LDraw subpart/primitive - // aliases (s/, 8/, 48/) are case-insensitive but the canonical path - // uses the lowercase form. - let canonical_prefix = prefix.to_ascii_lowercase(); - assert_eq!( - core_reference_path(¤t, &explicit).unwrap().as_str(), - format!("{root}/{canonical_prefix}/3001.dat") - ); + // The s/ alias maps subparts to the parts/ root without preserving + // the s/ segment in the canonical path (subparts are stored as + // parts/{name} in the official library). The 8/ and 48/ aliases + // preserve their segment under the p/ root. + let resolved = core_reference_path(¤t, &explicit).unwrap(); + let expected = match prefix { + "s" | "S" => format!("{root}/3001.dat"), + _ => format!("{root}/{prefix}/3001.dat"), + }; + assert_eq!(resolved.as_str(), expected); } let mpd = polymodel_ldraw_core::MpdReference { file_name: "main.ldr".into(), diff --git a/src/viewer.rs b/src/viewer.rs index 08a38fd..75a2868 100644 --- a/src/viewer.rs +++ b/src/viewer.rs @@ -15,17 +15,19 @@ #[cfg(target_arch = "wasm32")] use crate::client::PolymodelClient; use dioxus::prelude::*; +#[cfg(any(target_arch = "wasm32", feature = "renderer-test-readback"))] +use polymodel_renderer_protocol::LoadIdentity; +#[cfg(feature = "renderer-test-readback")] +use polymodel_renderer_protocol::RendererReadback; #[cfg(target_arch = "wasm32")] use polymodel_renderer_protocol::{ Capability, CommandEnvelope, CommandPayload, CompoundLoadPlan, EventPayload, HandshakeState, - LoadEvent, LoadIdentity, LoadReducer, ProtocolHello, RendererNamespace, - deserialize_event_envelope, serialize_command_envelope, + LoadEvent, LoadReducer, ProtocolHello, RendererNamespace, deserialize_event_envelope, + serialize_command_envelope, }; use polymodel_renderer_protocol::{ KeyModifiers, LoadErrorCode, MeshFormat, MeshStats, RendererCommand, RendererEvent, }; -#[cfg(feature = "renderer-test-readback")] -use polymodel_renderer_protocol::{LoadIdentity, RendererReadback}; #[cfg(target_arch = "wasm32")] use std::cell::RefCell; #[cfg(target_arch = "wasm32")] @@ -56,6 +58,100 @@ pub fn accept_test_readback(current: LoadIdentity, sample: &RendererReadback) -> sample.identity == current } +/// Expose an accepted readback sample through `window.__POLYMODEL_TEST_READBACKS__` +/// as a plain JS object (identity, viewport, DPR, clear colour, non-clear pixels, +/// counts, bounds). No source bytes are exposed to the page. +#[cfg(feature = "renderer-test-readback")] +fn expose_test_readback(sample: &RendererReadback) { + #[cfg(target_arch = "wasm32")] + { + use wasm_bindgen::JsCast; + let Some(window) = web_sys::window() else { + return; + }; + let key = "__POLYMODEL_TEST_READBACKS__"; + let array = match js_sys::Reflect::get(&window, &key.into()) { + Ok(existing) if existing.is_array() => { + existing.dyn_into::().unwrap_or_default() + } + _ => js_sys::Array::new(), + }; + let entry = js_sys::Object::new(); + let _ = js_sys::Reflect::set( + &entry, + &"identity".into(), + &format!( + "{}-{}", + sample.identity.load_epoch, sample.identity.scene_generation + ) + .into(), + ); + let _ = js_sys::Reflect::set( + &entry, + &"viewport_width".into(), + &sample.viewport_width.into(), + ); + let _ = js_sys::Reflect::set( + &entry, + &"viewport_height".into(), + &sample.viewport_height.into(), + ); + let _ = js_sys::Reflect::set( + &entry, + &"device_pixel_ratio_milli".into(), + &sample.device_pixel_ratio_milli.into(), + ); + let clear = js_sys::Array::new(); + for byte in sample.clear_rgba.iter() { + clear.push(&(*byte).into()); + } + let _ = js_sys::Reflect::set(&entry, &"clear_rgba".into(), &clear); + let _ = js_sys::Reflect::set( + &entry, + &"non_clear_pixels".into(), + &sample.non_clear_pixels.into(), + ); + let counts = js_sys::Object::new(); + let _ = js_sys::Reflect::set( + &counts, + &"points".into(), + &sample.primitive_counts.points.into(), + ); + let _ = js_sys::Reflect::set( + &counts, + &"triangles".into(), + &sample.primitive_counts.triangles.into(), + ); + let _ = js_sys::Reflect::set( + &counts, + &"lines".into(), + &sample.primitive_counts.lines.into(), + ); + let _ = js_sys::Reflect::set( + &counts, + &"conditional_lines".into(), + &sample.primitive_counts.conditional_lines.into(), + ); + let _ = js_sys::Reflect::set(&entry, &"primitive_counts".into(), &counts); + let bounds = js_sys::Object::new(); + let min = js_sys::Array::new(); + let max = js_sys::Array::new(); + for axis in 0..3 { + min.push(&sample.bounds.min[axis].into()); + max.push(&sample.bounds.max[axis].into()); + } + let _ = js_sys::Reflect::set(&bounds, &"min".into(), &min); + let _ = js_sys::Reflect::set(&bounds, &"max".into(), &max); + let _ = js_sys::Reflect::set(&entry, &"bounds".into(), &bounds); + array.push(&entry); + let _ = js_sys::Reflect::set(&window, &key.into(), &array); + } + #[cfg(not(target_arch = "wasm32"))] + { + let _ = sample; + } +} + #[derive(Clone, Debug, PartialEq, Eq)] pub enum ViewerSourceFormat { Renderer(MeshFormat), @@ -1838,6 +1934,7 @@ impl WorkerBridgeState { non_clear_pixels = sample.non_clear_pixels, "accepted renderer test readback" ); + expose_test_readback(&sample); } else { tracing::debug!( sample_identity = ?sample.identity, -- 2.54.0