fix: preserve wall ownership through compaction

Issue: #299
User-Visible: yes
This commit is contained in:
Sergey Matyunin
2026-08-24 23:28:39 +03:00
parent 6558728519
commit ba63fccbb7
12 changed files with 188 additions and 26 deletions
+8
View File
@@ -570,6 +570,14 @@ reuses ordinary opening projection and wall/open-span rekeying. Unique physical
count, maximum centimetres and skipped candidates stay separate from ordinary
grid movement; no load/save migration invokes this repair.
`normalizeWallIntervals()` compacts atomic real-wall intervals only when both
their centimetre thickness and ownership signature match (#299). The signature
is `outer(A)` or the stable sorted pair `shared(A,B)`; an outer/shared transition,
a change of shared pair, or ambiguous multi-owner geometry is a hard breakpoint.
Explicit Optimize and the room-deletion transaction call this same normalizer,
so neither path can create one saved record whose physical role changes halfway
through its exact span. Ambiguous ownership fails closed per atom.
All physical-geometry writers share the same transaction boundary (#278).
`checkSpacePhysicalGeometry()` validates the exact candidate through canonical
wall and floor builders before history or save. A failed or degraded candidate
+3 -1
View File
@@ -497,7 +497,9 @@ orchestrator. It converts only legacy fields with an exact lossless
mapping, materialises legacy `open_to`, calls the grid projection,
rekeys exact wall/open-span endpoints onto the moved rooms, merges
touching virtual spans per room pair, compacts consecutive real-wall
intervals of equal thickness and stamps `model_version`. Unknown fields
intervals only when their thickness and physical ownership both match, and
stamps `model_version`. Outer/shared transitions and changes of shared-room
pair remain exact breakpoints even at equal thickness. Unknown fields
are preserved and every pass is idempotent.
The explicit pass also repairs pre-existing near-axis room walls, saved wall
+5
View File
@@ -22,6 +22,11 @@
keep both areas visible with leader lines, without covering each other or the
room-settings button
([#300](https://github.com/Matysh/houseplan-card/issues/300)).
- “Optimize plans” and room deletion no longer combine an equal wall thickness
across a shared-to-outer boundary or between different pairs of rooms. The
saved wall profile keeps each physical role at its exact breakpoint, avoiding
a record that changes meaning halfway through its span
([#299](https://github.com/Matysh/houseplan-card/issues/299)).
- While drawing Walls, `Esc` now finishes all accepted segments as independent
walls and releases the last point without deleting geometry or leaving the
tool. The next click starts a new chain; `Ctrl/Cmd+Z` remains the shortcut
+5
View File
@@ -27,6 +27,11 @@
затронутой комнаты с её стороны. Для общих и узких комнат обе площади остаются
видимыми с выносными линиями и не перекрывают друг друга или кнопку настроек
комнаты ([#300](https://github.com/Matysh/houseplan-card/issues/300)).
- «Оптимизировать планы» и удаление комнаты больше не склеивают одинаковую
толщину через границу общей и наружной стены или между разными парами комнат.
Профиль стены сохраняет каждую физическую роль до её точной границы, поэтому
одна запись больше не меняет смысл посередине своего участка
([#299](https://github.com/Matysh/houseplan-card/issues/299)).
- При рисовании стен `Esc` теперь завершает все принятые отрезки как
независимые стены и отцепляется от последней точки, не удаляя геометрию и не
покидая инструмент. Следующий клик начинает новую цепочку, а `Ctrl/Cmd+Z`
+8
View File
@@ -2054,6 +2054,14 @@ require hands on real hardware — they remain for the human pass.
become partly shared and partly outer
[unit: resize.test + fixture 289-mixed-role-resize; auto:
smoke_room_resize; mutation: safe-resize-side-ownership-bypassed]
- [ ] Wall compaction preserves physical ownership (#299): equal thickness on
`shared(A,B) -> outer(A)` and `shared(A,B) -> shared(A,C)` remains split
at the exact role breakpoint, while equal neighbouring atoms inside one
role still compact. Optimize on `real-plan-first-floor.json` is immutable,
invariant-clean and idempotent; real-plan edit-walk seeds 1 and 3 exercise
Optimize and Delete-room/Keep-walls without producing a mixed-role record
[unit: wall-thickness + plan-optimizer; auto: smoke_edit_walk seeds 1/3;
mutation: wall-compaction-owner-role-bypassed]
- [ ] Live badges while dragging: lengths of the dragged wall + both
adjacent walls, and the m² area at the room centre; dragging a shared
wall shows BOTH areas; all numbers update continuously
+5 -2
View File
@@ -332,7 +332,7 @@ Other operations edit existing geometry:
| Split | Cuts a room from one wall to another; the larger part keeps the original room |
| Resize | Moves one eligible horizontal/vertical wall without changing room topology. Live labels report the two changing **inner** side-wall dimensions, highlight those walls, and place each affected room's area beside its side of the moving wall |
| Thickness | Changes one physical span or every wall of a room |
| Delete room | Deletes only the selected room after confirmation |
| Delete room | Deletes the room after choosing whether its exclusive physical walls remain; shared walls always remain |
![Selected partition and its Plan context tray](images/05-plan-context-tray.png)
@@ -687,7 +687,10 @@ pull intentional off-grid or diagonal geometry to a node. Current ordinary
edits apply the same invisible boundary automatically, so the noise cannot
return after a later room, opening, decor or marker-position save.
Equal neighbouring wall-thickness fragments are compacted. Optimize may also
Equal neighbouring wall-thickness fragments are compacted only while they have
the same physical role: one outer room or the same pair of shared rooms. A
shared-to-outer transition or a change of shared-room pair stays as an exact
breakpoint even when the thickness is equal. Optimize may also
remove a different-thickness fragment shorter than half a grid step when equal
pieces of the same straight wall prove the replacement. This includes a
fragment touching exactly one room T-junction: the junction and perpendicular
+4 -2
View File
@@ -447,7 +447,9 @@ Undo оптимизации.
толщиной, а их двери/окна перепривязываются к ним. Виртуальные и нулевые участки
не превращаются в кладку. При варианте **«Удалить комнату и стены»** удаляются
только проёмы, принадлежавшие эксклюзивной стене этой комнаты; общая стена,
явная совпадающая перегородка и их проёмы сохраняются.
явная совпадающая перегородка и их проёмы сохраняются. Оставшиеся записи
толщины нормализуются уже по новому составу комнат: одинаковая толщина не
склеивается через переход «общая ↔ наружная» или между разными парами комнат.
![Выбранная перегородка и её контекстная панель](images/05-plan-context-tray.png)
@@ -1427,7 +1429,7 @@ show_signal: true
| Декор и мебель | Положение и размеры округляются к сетке |
| Устройства и подписи комнат | Позиции округляются к сетке |
| Проёмы | Возвращаются на ближайшую стену, смещение вдоль стены округляется, угол исправляется |
| Стены | Одинаковые соседние участки толщины объединяются. Изолированный участок другой толщины короче половины шага сетки также схлопывается, если с обеих сторон находятся участки одной толщины той же прямой стены. Допустим один T-узел комнаты: он и перпендикулярная стена не двигаются. Два топологических узла или граница проёма защищают участок |
| Стены | Одинаковые соседние участки толщины объединяются только в одной физической роли: наружная стена одной комнаты либо общая стена той же пары комнат. Переход «общая ↔ наружная» и смена пары комнат остаются точными границами даже при одинаковой толщине. Изолированный участок другой толщины короче половины шага сетки также схлопывается, если с обеих сторон находятся участки одной толщины той же прямой стены. Допустим один T-узел комнаты: он и перпендикулярная стена не двигаются. Два топологических узла или граница проёма защищают участок |
| Перегородки и сохранённые цепочки | Коллинеарные соседние отрезки одинаковой толщины сращиваются; топологический узел сохраняется. Каждый участок отдельной стены, точно покрытый одной или несколькими соседними сплошными стенами комнат, поглощается ими, а недоказанные остатки и их проёмы сохраняются. Проёмы на поглощённой части перепривязываются без сдвига и потери датчиков; сохраняется большая исходная толщина. Незавершённая цепочка удаляется только целиком и только если каждый её сегмент полностью избыточен; свободная, частично покрытая или более толстая цепочка не меняется |
| Виртуальные стены | Соседние/перекрывающиеся участки объединяются и приводятся к общей границе |
| Ссылки устройств | Точная подпись независимого импорта восстанавливает пространство, комнату и позицию. Иначе реальное устройство следует однозначной Area HA либо теряет только мёртвую привязку; настройки маркера сохраняются |
+13 -3
View File
@@ -43,8 +43,10 @@ grades a different stored key as an observation, not a violation: valid exact
endpoints now prove that the record is resolvable even when its compatibility
key is old or unparsable. Exact endpoints make a thickness boundary independent of whichever
room topology later happens to split the same straight line. Normalisation
merges consecutive solid pieces into each maximal run of equal thickness; a
different thickness or a virtual gap remains a real break. Likewise,
merges consecutive solid pieces only inside a maximal run of equal thickness
with the same physical ownership: one outer room or the same sorted pair of
shared rooms. A different thickness, virtual gap, outer/shared transition or
change of shared-room pair remains a real break. Likewise,
touching/overlapping `open_spans` of the same room pair are stored as one span;
pair ownership remains a hard boundary so Split can derive exact `open_to` links.
When a maximal wall run crosses a collinear vertex belonging to another room,
@@ -324,7 +326,10 @@ gaps and zero-thickness edges are not materialised. **Delete walls** removes the
room without that conversion and cascades only openings owned by its exclusive
walls. Shared masonry, existing partitions, partition-hosted openings and their
physical thickness remain intact in both cases. The room, wall profile,
partitions and openings are one Undo/Redo and persistence transaction.
partitions and openings are one Undo/Redo and persistence transaction. The
remaining wall profile is normalised against its post-delete ownership, so an
equal thickness cannot be compacted across an outer/shared boundary or across
two different shared-room pairs.
## 7. Out of scope
@@ -354,6 +359,11 @@ the real `349 / 120 / 5` short-ray handoff to a 20 cm shared wall at
`cell_cm: 1/5/30`, reversed endpoints and permuted input (#288);
exact parent-run thickness inherited by atomic children when
closing a virtual neighbour, without partial-span leakage (#201).
Role-aware compaction tests keep `shared(A,B)`, `outer(A)` and `shared(A,C)` as
separate records even at equal thickness, while equal neighbouring atoms within
one role still compact; the real first-floor fixture proves explicit Optimize
is invariant-free and idempotent (#299). The edit-walk real-plan seeds 1 and 3
exercise the same Optimize and Keep-walls entry points.
Browser: seamless frame; fill not in hatch; m² drops with thickness; partial
shared walls, mixed-thickness shared walls and walls containing a partial
virtual stretch keep a visible disabled Resize handle and cannot split or
+13
View File
@@ -1299,6 +1299,19 @@ export const MUTANTS = [
+ ' const eps = Math.max(pitch * scale * 0.02, 1e-9);',
}],
},
{
id: 'wall-compaction-owner-role-bypassed',
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
+ '&& node --test --test-name-pattern="#299" test/wall-thickness.test.mjs',
because: 'equal centimetres must not merge shared(A,B) with outer(A) or shared(A,C); '
+ 'dropping the owner signature recreates the mixed-role wall record from #299',
patches: [{
file: 'src/wall-thickness.ts',
find: ' if (pr.kinds[next] === null || pr.cms[next] !== cm\n'
+ ' || ownerSignatureFor(nextKey) !== ownerSignature) break;',
replace: ' if (pr.kinds[next] === null || pr.cms[next] !== cm) break;',
}],
},
{
id: 'optimizer-single-topology-island-blocked',
guard: 'npx tsc -p tsconfig.test.json && node scripts/fix-test-build.mjs '
+39 -9
View File
@@ -2142,18 +2142,41 @@ export function normalizeWallIntervals(
coordScale = 1,
): WallEntry[] {
if (!walls?.length) return [];
const atomic: WallInterval[] = [];
type OwnedInterval = WallInterval & { ownerSignature: string };
const resolved = wallIntervals(
rooms, walls, openCuts, pitch, cellCm, gridPitch, coordScale,
);
const ownersByKey = new Map<string, Set<string>>();
for (const interval of resolved) {
if (interval.open || !interval.kind || !interval.roomId) continue;
const owners = ownersByKey.get(interval.key) || new Set<string>();
owners.add(interval.roomId);
ownersByKey.set(interval.key, owners);
}
const ownerSignatureFor = (key: string): string => {
const owners = [...(ownersByKey.get(key) || [])].sort();
// A physical wall has one outer owner or two shared owners. Invalid
// multi-owner geometry is preserved fail-closed, one atom at a time: it
// must not become the bridge which compacts two otherwise separate roles.
if (owners.length !== 1 && owners.length !== 2) return `ambiguous:${key}`;
return `${owners.length === 1 ? 'outer' : 'shared'}:${owners.join('|')}`;
};
const atomic: OwnedInterval[] = [];
const atomicKeys = new Set<string>();
for (const iv of wallIntervals(rooms, walls, openCuts, pitch, cellCm, gridPitch, coordScale)) {
for (const iv of resolved) {
if (iv.open || !(iv.cm > 0) || atomicKeys.has(iv.key)) continue;
atomicKeys.add(iv.key);
atomic.push(iv);
atomic.push({ ...iv, ownerSignature: ownerSignatureFor(iv.key) });
}
// Compact every maximal solid run of one thickness. This still restores one
// whole-edge entry when all children agree, but retains an exact breakpoint
// when neighbouring real intervals intentionally have different thicknesses.
const parents: Array<{ a: number[]; b: number[]; key: string; cm: number; len: number }> = [];
// Compact every maximal solid run of one thickness AND one physical owner
// role. Equal centimetres cannot bridge shared(A,B) to outer(A), nor one
// shared pair to another: that creates a record whose thickness changes
// meaning halfway through its own span (#299).
const parents: Array<{
a: number[]; b: number[]; key: string; cm: number; len: number;
ownerSignature: string;
}> = [];
for (const room of rooms || []) {
if (!room?.id) continue;
const pr = roomWallProfile(rooms, room.id, walls, openCuts, pitch, cellCm, gridPitch, coordScale);
@@ -2168,10 +2191,16 @@ export function normalizeWallIntervals(
const first = children[at];
const cm = pr.cms[first];
if (!(cm > 0) || pr.kinds[first] === null) { at++; continue; }
const firstKey = keyOf(pr.poly[first], pr.poly[(first + 1) % pr.poly.length], pitch, coordScale);
const ownerSignature = ownerSignatureFor(firstKey);
let end = at;
while (end + 1 < children.length) {
const next = children[end + 1];
if (pr.kinds[next] === null || pr.cms[next] !== cm) break;
const nextKey = keyOf(
pr.poly[next], pr.poly[(next + 1) % pr.poly.length], pitch, coordScale,
);
if (pr.kinds[next] === null || pr.cms[next] !== cm
|| ownerSignatureFor(nextKey) !== ownerSignature) break;
end++;
}
const last = children[end];
@@ -2179,7 +2208,7 @@ export function normalizeWallIntervals(
const len = Math.hypot(b[0] - a[0], b[1] - a[1]);
if (len > 0) parents.push({
a: [a[0], a[1]], b: [b[0], b[1]],
key: keyOf(a, b, pitch, coordScale), cm, len,
key: keyOf(a, b, pitch, coordScale), cm, len, ownerSignature,
});
at = end + 1;
}
@@ -2194,6 +2223,7 @@ export function normalizeWallIntervals(
for (const parent of parents) {
const matches = atomic.filter((iv) => (
!covered.has(iv.key) && iv.cm === parent.cm &&
iv.ownerSignature === parent.ownerSignature &&
angleClose(segAngle(iv.a, iv.b), segAngle(parent.a, parent.b)) &&
distToSeg(iv.a[0], iv.a[1], parent.a[0], parent.a[1], parent.b[0], parent.b[1]) <= tol &&
distToSeg(iv.b[0], iv.b[1], parent.a[0], parent.a[1], parent.b[0], parent.b[1]) <= tol
+42 -5
View File
@@ -13,6 +13,7 @@ import { GRID_PITCH, GRID_STEP_N as S, NORM_W } from '../test-build/space-geomet
import {
wallBodiesGeometry, wallIntervals, wallKey,
} from '../test-build/wall-thickness.js';
import { checkMixedRoleRecords, checkWallKeys } from '../scripts/model-invariants.mjs';
const room = (id, x0, x1, openTo) => ({
id,
@@ -36,6 +37,10 @@ const wallKeyRoundtripFixture = JSON.parse(readFileSync(
new URL('./fixtures/258-wall-key-roundtrip.json', import.meta.url),
'utf8',
));
const realFirstFloorFixture = JSON.parse(readFileSync(
new URL('./fixtures/real-plan-first-floor.json', import.meta.url),
'utf8',
));
const assertNoPersistedChanges = (result) => {
assert.equal(result.changed, false);
@@ -235,15 +240,20 @@ test('issue 273 Optimize collapses the beta.5 island beside one T-node', () => {
assert.deepEqual(config, before, 'preview must not mutate the T-node source');
assert.equal(first.changed, true);
assert.equal(first.report.canonicalized, 1);
assert.equal(first.report.wallsMerged, 2);
assert.equal(first.report.wallsMerged, 0,
'role breakpoints may keep the record count even after the micro island is repaired');
const canonicalBefore = canonicalizeConfigGeometry(before);
assert.deepEqual(first.config.spaces[0].rooms, canonicalBefore.spaces[0].rooms,
'T coordinate and perpendicular incident room stay byte-equivalent');
assert.equal(first.config.spaces[0].walls.length, 1);
assert.equal(first.config.spaces[0].walls[0].cm, 22);
assert.equal(first.config.spaces[0].walls.length, 3);
assert.ok(first.config.spaces[0].walls.every((wall) => wall.cm === 22),
'the micro thickness is repaired without merging outer/shared owner roles');
const exactNode = 83 / 240;
assert.deepEqual(first.config.spaces[0].walls[0].a, [0.8, exactNode]);
assert.deepEqual(first.config.spaces[0].walls[0].b, [0.95, exactNode]);
const roleBreaks = first.config.spaces[0].walls
.flatMap((wall) => [wall.a?.[0], wall.b?.[0]])
.filter(Number.isFinite);
assert.ok(roleBreaks.includes(203 / 240), 'outer→shared owner boundary stays exact');
assert.ok(roleBreaks.includes(213 / 240), 'shared→outer owner boundary stays exact');
const afterIntervals = wallIntervals(
first.config.spaces[0].rooms, first.config.spaces[0].walls, [], S, 5, S,
@@ -264,6 +274,33 @@ test('issue 273 Optimize collapses the beta.5 island beside one T-node', () => {
assert.deepEqual(second.config, first.config);
});
test('#299 Optimize keeps the real first-floor shared/outer role breakpoint', () => {
const config = {
model_version: PLAN_MODEL_VERSION,
spaces: [structuredClone(realFirstFloorFixture.space)],
markers: [], settings: {},
};
const before = structuredClone(config);
const first = optimizePlans(config, {});
assert.deepEqual(config, before, 'Optimize preview must not mutate the real fixture');
assert.equal(first.changed, true);
assert.deepEqual(checkMixedRoleRecords(first.config), []);
assert.deepEqual(checkWallKeys(first.config, { notes: [] }), []);
const y = 83 / 240;
const line = first.config.spaces[0].walls.filter((wall) => wall.a && wall.b
&& Math.abs(wall.a[1] - y) < 1e-9 && Math.abs(wall.b[1] - y) < 1e-9);
assert.equal(line.length, 2, 'micro cleanup keeps one exact record per owner role');
assert.ok(line.every((wall) => wall.cm === 22));
assert.ok(line.every((wall) => wall.a[0] === 213 / 240 || wall.b[0] === 213 / 240),
'both runs stop at the exact shared/outer boundary');
const second = optimizePlans(first.config, first.layout);
assertNoPersistedChanges(second);
assert.deepEqual(second.config, first.config);
assert.deepEqual(second.layout, first.layout);
});
test('Optimize canonicalizes the six-room ULP source without claiming a visible move', () => {
const config = {
model_version: PLAN_MODEL_VERSION,
+43 -4
View File
@@ -30,7 +30,7 @@ import { polygonArea, paperRoomShapes, splitRoomPath, sharedBoundary } from '../
import { resolveOpenCuts } from '../test-build/open-spans.js';
import { GRID_PITCH, NORM_W } from '../test-build/space-geometry.js';
import { geometryArea } from '../test-build/physical-geometry.js';
import { checkWallRecordsPreserved } from '../scripts/model-invariants.mjs';
import { checkMixedRoleRecords, checkWallRecordsPreserved } from '../scripts/model-invariants.mjs';
import { difference, intersection, union } from 'polyclip-ts';
const closeTo = (got, want, tol = 1e-6) =>
@@ -918,18 +918,57 @@ test('a compacted exact wall covers a shorter collinear side in another room', (
assert.ok(right && right.half > 0, 'hover/body profile must use the real inner face');
});
test('equal solid atomic pieces compact back to one whole-wall key', () => {
test('#299 equal thickness does not compact across a shared-to-outer role boundary', () => {
const rooms = partialRooms();
const walls = [
{ key: wallKey([5, 0], [5, 4], pitch), cm: 30 },
{ key: wallKey([5, 4], [5, 10], pitch), cm: 30 },
];
const next = normalizeWallIntervals(rooms, walls, [], pitch, cellCm, GRID_PITCH);
assert.equal(next.length, 2);
assert.ok(next.some((wall) => wall.key === wallKey([5, 0], [5, 4], pitch)));
assert.ok(next.some((wall) => wall.key === wallKey([5, 4], [5, 10], pitch)));
assert.ok(next.every((wall) => wall.cm === 30));
assert.deepEqual(checkMixedRoleRecords({ spaces: [{ id: 'roles', rooms, walls: next }] }), []);
const reversed = normalizeWallIntervals(
[...rooms].reverse(), [...walls].reverse(), [], pitch, cellCm, GRID_PITCH,
);
assert.deepEqual(
reversed.map((wall) => wall.key).sort(),
next.map((wall) => wall.key).sort(),
'room/input order must not change the physical role breakpoint',
);
});
test('#299 the shared owner pair is part of the compaction role', () => {
const rooms = [
{ id: 'a', poly: [[0, 0], [5, 0], [5, 10], [0, 10]] },
{ id: 'b', poly: [[5, 0], [10, 0], [10, 5], [5, 5]] },
{ id: 'c', poly: [[5, 5], [10, 5], [10, 10], [5, 10]] },
];
const walls = [
{ key: wallKey([5, 0], [5, 5], pitch), cm: 22 },
{ key: wallKey([5, 5], [5, 10], pitch), cm: 22 },
];
const next = normalizeWallIntervals(rooms, walls, [], pitch, cellCm, GRID_PITCH);
const vertical = next.filter((wall) => wall.a?.[0] === 5 && wall.b?.[0] === 5);
assert.equal(vertical.length, 2);
assert.ok(vertical.some((wall) => wall.key === wallKey([5, 0], [5, 5], pitch)));
assert.ok(vertical.some((wall) => wall.key === wallKey([5, 5], [5, 10], pitch)));
assert.deepEqual(checkMixedRoleRecords({ spaces: [{ id: 'pairs', rooms, walls: next }] }), []);
});
test('#299 equal neighbouring outer atoms of one room still compact', () => {
const rooms = [{ id: 'a', poly: [[0, 0], [5, 0], [5, 10], [0, 10]] }];
const walls = [
{ key: wallKey([5, 0], [5, 4], pitch), a: [5, 0], b: [5, 4], cm: 30 },
{ key: wallKey([5, 4], [5, 10], pitch), a: [5, 4], b: [5, 10], cm: 30 },
];
const next = normalizeWallIntervals(rooms, walls, [], pitch, cellCm, GRID_PITCH);
assert.equal(next.length, 1);
assert.equal(next[0].key, wallKey([5, 0], [5, 10], pitch));
assert.equal(next[0].cm, 30);
assert.deepEqual(next[0].a, [5, 0]);
assert.deepEqual(next[0].b, [5, 10]);
});
test('different solid thicknesses remain separate atomic keys', () => {