Skip to content

Wall junction miter order is non-deterministic for exactly-collinear walls #581

Description

@Aymericr

What happened?

Pre-existing bug, found while reviewing #556 (perf(core,viewer): stop rebuilding wall geometry every frame) and promised as a follow-up there. This is not a regression from that PR — the caching work in #556 made junction ordering load-bearing for cache identity, which is what drew attention to it, but the tie-break gap predates it.

buildWallJunctions in packages/core/src/systems/wall/wall-mitering.ts:279 orders the walls meeting at a junction by their outgoing angle and then miters each adjacent pair:

// Sort by outgoing angle
processedWalls.sort((a, b) => a.angle - b.angle)

The comparator has no tie-break. When two walls leave a junction at the same angle — Math.atan2(v.y, v.x) returning bit-identical values — their relative order is whatever Array.prototype.sort produces for the input order. sort is required to be stable in modern JS, so the order follows the order walls were pushed into processedWalls, which follows scene iteration order. That is not a property we control or should rely on: it changes when a wall is deleted and re-added, when nodes are reordered, when a scene is reloaded from storage, or when a remote peer's mutation lands in a different sequence.

Because the loop miters processedWalls[i] against processedWalls[(i + 1) % n], a different order means a different pairing, which means different left/right intersection points for those walls — so geometry can differ between two sessions of the identical scene.

Steps to reproduce

Exactly-collinear same-angle walls at one junction are constructible in a few ways:

  1. Two walls of different thickness continuing along one straight line (this shape is already exercised for slab polygons — see slab-polygon.test.ts:722, "two collinear walls of different thickness along one edge").
  2. A duplicated wall left stacked on top of another after a copy/paste or an interrupted drag.
  3. A wall drawn back over an existing one so both share an endpoint and a direction.

Then: save and reload, or delete and undo one of the two, and compare the mitered corner.

Expected behavior

Junction geometry should be a pure function of the walls at that junction — same walls in, same geometry out, independent of scene iteration order.

Suggested fix

Give the comparator a deterministic tie-break on a stable intrinsic property, so equal angles resolve the same way every time:

processedWalls.sort((a, b) => a.angle - b.angle || (a.id < b.id ? -1 : a.id > b.id ? 1 : 0))

Node id is stable across reload and across peers, which is what's needed. Thickness would be a tempting secondary key but isn't unique.

Two things worth checking as part of the fix rather than assuming:

  • Is angle ever nearly equal rather than exactly equal? A tie-break on strict equality doesn't help two walls that differ by 1e-16 — they'd sort consistently by angle but the pairing is still arbitrary in effect. If near-collinear is the real-world case, the tie-break needs an epsilon, which is a different (and more delicate) change. wall-mitering.ts:39-46 already documents the near-collinear regime as pathological for miter length, so there's prior context to read.
  • Does the miter cache in packages/viewer/src/systems/wall/level-miter-cache.ts key on anything order-dependent? If it does, non-determinism here can also mean a cache that's valid in one session and stale in the next.

Additional context

Low user-visible impact — the visual difference at a collinear junction is small, and the shape is uncommon in normal drawing. Filing it because it's a determinism bug rather than a rendering bug: order-dependent geometry is the kind of thing that surfaces later as a phantom diff in collaborative editing or an unexplained cache miss, and it's much cheaper to fix now with a one-line comparator than to debug from that end.

Good first issue for someone who wants to work in core geometry with a test to write: a unit test that builds the same junction twice with reversed insertion order and asserts identical output would fail today.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinggood first issueGood for newcomers

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions