Pre-release fixes: playback version warning, O(1) same-name re-add, line_width compat - #760
Merged
Conversation
Live connections reject client/server version mismatches via the websocket subprotocol, but .viser recordings only console.log'd the version they were saved with — an old recording played back silently wrong when the message format had drifted (e.g. the line_width -> thickness rename). Show a persistent notification when the recording's version differs from the viewer's. Embeds from as_html() bundle the client build that wrote them, so they can't mismatch.
…sage Three fixes from the pre-release regression audit: - Same-name scene node re-add scanned the entire persistent message buffer to purge the old node's per-entity state, making re-add-per-frame animation loops quadratic in scene size (~4.5ms per re-add at 2k nodes). Add an entity-state index to AsyncMessageBuffer -- the materialized form of Message.targets_entity_state -- maintained by a single deletion primitive (pop_message_locked), and route the replacement purge, push()'s remove-time update purge, and the GC's second pass through it. Re-add is now O(per-entity messages): flat ~0.5ms regardless of scene size. - The deprecated line_width handle setter wrote thickness without pinning units, so a v1.0.x '3 px' intent became 3 world units on handles created with the new world-space defaults. The setter now forces thickness_units='screen', preserving the alias's historical pixel meaning on every handle. - The folder/tab context-guard error claimed 're-entered inside itself' for what can also be a concurrent enter from another thread; say so.
- Folder/tab handles: overlapping 'with' entry (self-nested or from another thread) raises; sequential re-entry stays legal and elements land in the container. - Same-name scene node replacement: end-to-end check that the old node's buffered state is purged, the replacement's forced default-pose broadcast survives, bystander nodes are untouched, and the buffer's entity-state index stays consistent.
…ath cleanups Applying a reuse/simplification/efficiency/altitude review of the recent changes: - The entity-state taxonomy now has one definition: Message.entity_state_key() (next to its data), with targets_entity_state reduced to a predicate view of it. The buffer's parallel _entity_state_key copy -- and the test loop that existed only to police their agreement -- are gone. push()'s remove purge filters by 'lifecycle_phase is not None' instead of restating the phase tuple a third time. - push() hot path: entity key computed outside the buffer lock (matching the existing pre-compute pattern) and the per-push throwaway set() from setdefault() eliminated. - The four copy-pasted line_width alias bodies now delegate to shared module-level helpers carrying the warning text and units-first rationale once. - Test hygiene: assert_entity_index_consistent moved to tests/infra_utils (the declared shared home) instead of a cross-module private import; new tests use the viser_server() context manager and broadcast_messages() helpers rather than leaking bare servers and hand-walking the buffer. Kept: remove_from_buffer stays (public infra API for downstream servers, now index-consistent); the coalesce-path double key computation and a single-message thickness update were considered and skipped as marginal or behavior-changing.
…e on remount Three fixes from the code-review pass: - add_arrows was the one line_width site migrated without a deprecation shim: passing the old kwarg raised TypeError and the handle alias was silently dead. It now warns-and-ignores (arrows have no thickness equivalent -- the old prop only fed a client fallback rendering path that no longer exists), with the same Never-typed overload pattern as the sibling APIs and a warn+no-op handle property. - The share tunnel's wait_job/on_disconnect daemon threads caught only EOFError around dead-manager proxy reads, but those also raise BrokenPipeError/ConnectionResetError -- the same trio close() already tolerates -- printing a spurious traceback at interpreter exit after a failed tunnel. - Line's fringe material applied its VISER_LINE_FRINGE define in an effect keyed on [showFringe], so a material remounted by a segments/dashed branch flip while showFringe stayed true missed the define and lost the antialiasing pass. The define is now applied via callback ref at attach time. Not reachable through viser's own wrappers (constant props), but Line is exported for reuse.
The prior commit accidentally included lockfile churn from a local npm version rewriting peer flags; no dependency actually changed.
brentyi
force-pushed
the
pre-release-fixes
branch
from
August 14, 2026 05:47
41d80ca to
7403b0c
Compare
The slider number box limits displayed/typed decimals via decimalScale, fed by a server-computed precision that was derived from step alone. A slider's legal values are min + k*step clamped to max, so an off-grid min or max (e.g. min=0.5, step=1.0) produced values whose decimals the box could neither display (2.5 rendered as "3") nor accept as input. Precision is now the max decimal digits across min, max, and step, which exactly bounds the grid's digits; integer sliders keep precision 0, so the fractional-input rejection that motivated decimalScale is preserved (pinned by the existing e2e test). New unit tests cover the grid rule and a new e2e test verifies exact display and typed round-trip in a real browser.
…unt, orbit-bound snap Four fixes: - Setting a camera pose whose up direction is parallel to the view (the canonical top-down/bird's-eye setup with default +Z up) raised ValueError from the new degenerate-basis hardening; inside exception-isolated on_client_connect callbacks the pose silently never applied. The server now degrades like the client's orbit controls (pole clamp): it falls back to the previous orientation's up vector, then to a world axis, and still sends the pose. Garbage input (NaN, zero up, zero look distance) still raises. - Numeric-input display precision (decimalScale) was derived from step alone. Sliders now cover their value grid (min + k*step clamped to max), and number/vector inputs additionally cover creation-time values and bounds, via a shared _compute_precision_digits_covering helper -- so add_number(initial_value=0.25, step=0.5) no longer displays "0.2". Integer grids keep precision 0. - Crossing the mobile breakpoint swapped the element type wrapping the canvas (dock surface mounted/unmounted), remounting the entire R3F subtree: new WebGL context and camera snapped to the initial pose on every phone rotation. The dock surface now stays mounted in websocket mode as a passthrough container and toggles an enabled flag (panes emptied, sync nodes dropped -- both siblings after the canvas, so the canvas never reparents). Spec section 3.6 updated to match. - A camera legitimately placed beyond max_orbit_distance (large- coordinate scenes under the 1e4 default) was teleported to the bound by the first scroll tick, since camera-controls clamps every user dolly. The effective bound now ratchets to the current distance while outside the configured one: zooming in is smooth, zooming further out stays blocked, and the configured clamp restores once inside. An explicit server-set bound still pulls the camera in smoothly. New coverage: top-down pose unit tests, precision-grid unit tests for all five numeric widgets, and browser e2e tests asserting canvas identity + camera pose survive breakpoint crossings and that far cameras dolly smoothly instead of snapping.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes from a pre-release regression audit:
.viserrecording saved with a different viser version now shows a version-mismatch notification instead of silently rendering with possible format drift.line_widthhandle setter now forcesthickness_units="screen"so assigned values keep their historical pixel meaning on handles created with world-space defaults.add_arrowsaccepts the removedline_widthargument with a deprecation warning instead of raisingTypeError, andArrowsHandle.line_widthwarns instead of silently writing a dead attribute.BrokenPipeError/ConnectionResetErrorfrom dead manager proxies, matchingclose()and avoiding spurious tracebacks at interpreter exit after a failed tunnel.segments/dashedbranch flips.Message.entity_state_key()definition, with new tests covering the buffer index, the re-add purge, overlapping container-context entry, and the arrows/line-width deprecations.