mirror of
https://github.com/Matysh/houseplan-card
synced 2026-09-29 11:18:48 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7b59feec5e | ||
|
|
2576d9d2fd |
File diff suppressed because one or more lines are too long
+174
-174
File diff suppressed because one or more lines are too long
Vendored
+174
-174
File diff suppressed because one or more lines are too long
@@ -0,0 +1,268 @@
|
||||
# CODE-REVIEW-89-r4 — #89, этап 1: перф-исправление AC11 (viewToggleMs)
|
||||
|
||||
- Issue: [#89](https://github.com/Matysh/houseplan-card/issues/89)
|
||||
- Этап: `code` (PROCESS.md §2.7)
|
||||
- Диапазон: `origin/dev...HEAD`, `origin/dev` = `3270e03` («test: accept
|
||||
v1.63.0-beta.1 golden baselines»), `HEAD` = `2576d9d` («perf: reuse wall
|
||||
union across projection toggles»), детач `HEAD`, ветка
|
||||
`issue/89-isometric-stage1` — **один коммит**
|
||||
- ТЗ: [`docs/specs/089-isometric-view-stage1.md`](../specs/089-isometric-view-stage1.md),
|
||||
ревизия 3, ревью ТЗ зелёное — [`SPEC-REVIEW-89-r1.md`](SPEC-REVIEW-89-r1.md)
|
||||
- Предыдущий цикл: [`CODE-REVIEW-89-r3.md`](CODE-REVIEW-89-r3.md) (зелёный,
|
||||
подтверждён повторно после merge-конфликта). Задача вернулась в
|
||||
`S6-in-progress` не из-за находки код-ревью, а из-за прогона **pre-release
|
||||
exact-SHA Full Performance** на кандидате `3270e03`: `large-house-isometric-v1`
|
||||
budget нарушен — `timing.viewToggleMs.median` **192,8 мс** против лимита
|
||||
**131,7 мс** (`base 31,7 мс × 1.2 + 100 мс noise`). Это AC11 ТЗ (§8.2).
|
||||
Согласно PROCESS.md §10.4 «цикл считается по этапу»: вердикт по ТЗ не
|
||||
расходует бюджет код-ревью, а находка **до выпуска беты** — это возврат на
|
||||
правки, а не новый баг (§4). Цикл код-ревью продолжает счётчик,
|
||||
опубликованный в комментариях issue (r2 — красный H1, r3 — зелёный дважды),
|
||||
поэтому это **r4/4** — последний разрешённый цикл.
|
||||
- Вердикт: **красный**
|
||||
|
||||
## Скоуп ревью
|
||||
|
||||
Единственный коммит диапазона:
|
||||
|
||||
- `2576d9d` «perf: reuse wall union across projection toggles» (класс A+B):
|
||||
`src/wall-thickness.ts` (+14/−2, новая экспортируемая
|
||||
`wallBodiesPathFromGeometry`), `src/houseplan-card.ts` (+21/−5, новый
|
||||
`_wallUnionKey()`, `_isoSource().build()` возвращает `{geom, depthUnits}`
|
||||
вместо голого `geom`, `_isoScene()` сеет `_wallUnionCache` уже вычисленным
|
||||
union'ом), `test/wall-thickness.test.mjs` (+13, юнит на эквивалентность),
|
||||
плюс синхронные копии бандла (класс D). Трейлеры: `Issue: #89`,
|
||||
`User-Visible: no` — верно для Labs-скрытой фичи (публичных changelog правок
|
||||
не требуется и не внесено).
|
||||
|
||||
Продуктовый код всего остального Stage 1 (Labs-механизм, проекция/камера,
|
||||
топология стен, fallback-защёлка, touch/kiosk/warm-remount, backend, i18n,
|
||||
публичная документация) не менялся с уже дважды зелёного `CODE-REVIEW-89-r3.md`
|
||||
— не переоткрываю то, что там разобрано построчно. Задача этого цикла узкая:
|
||||
проверить, что именно это исправление действительно решает AC11, и что оно не
|
||||
вносит новых регрессий.
|
||||
|
||||
Прочитано до вердикта: тело issue #89 и все 18 комментариев (включая решения
|
||||
владельца Q1–Q6/O1–O6 и оба сообщения о prerelease-регрессии/фиксе),
|
||||
`docs/SCOPE.md`, `AGENTS.md`, `PROCESS.md`, `docs/specs/089-isometric-view-stage1.md`
|
||||
(§8.2 — перф-контракт и бюджеты), `CODE-REVIEW-89-r2.md`/`r3.md`,
|
||||
`demo/performance/README.md`, `demo/performance/budgets-large-house-isometric.json`,
|
||||
`demo/benchmark_large_house.mjs`, `demo/fixtures/large-house.mjs`, полный diff
|
||||
коммита `2576d9d` в обоих продуктовых файлах и тесте.
|
||||
|
||||
## Как проверялось
|
||||
|
||||
`npm ci` выполнен перед гейтами на чистой рабочей копии (зависимостей не
|
||||
было; `git config core.hooksPath` → `.githooks`).
|
||||
|
||||
| Гейт | Команда | Результат |
|
||||
|---|---|---|
|
||||
| Типы | `npx tsc --noEmit` | green |
|
||||
| Unit | `npm test` (`npm run inventory`: 744 Node unit) | **744/744** green |
|
||||
| Сборка | `npm run build` | green |
|
||||
| Синхронность бандлов | `cmp dist/… custom_components/…`, `cmp dist/… demo/srv/…` | идентичны байт-в-байт; SHA-256 `31bea717b288e3b1486ee63fa8be6b8d3178ada8bb740ff0c2c76b887bbb5dd3` — совпадает с числом, заявленным автором |
|
||||
| Process gate | `node scripts/process-gate.mjs --range origin/dev..HEAD --issues` | green, 0 предупреждений |
|
||||
| Браузерные смоки — все 127 | `node demo/smoke_*.mjs` (после `npx playwright install --with-deps chromium`, свежий бандл) | **127/127 green**, включая `smoke_isometric_live_touch.mjs` (21/21) и `smoke_isometric_contract.mjs` (14/14) |
|
||||
| Golden — весь матрикс (не только iso-сцены) | `npm run golden:capture` | **47/47 `passed`, 0 diff** — включая все 6 `isometric-*` сцен (эталоны уже приняты на `origin/dev` коммитом `3270e03`, до этого цикла не относился) |
|
||||
| Backend | диапазон не трогает `custom_components/**/*.py` | гейт пуст по построению, не запускался |
|
||||
| **Perf — целевая проверка AC11** | см. отдельный раздел ниже | **не подтверждает исправление** |
|
||||
|
||||
### Проверка AC11: действительно ли исправление снимает регрессию
|
||||
|
||||
Это единственный содержательный вопрос цикла — «работает ли фикс», а не
|
||||
«гейты зелёные». Officiальный exact-SHA `Full Performance` — CI-only job на
|
||||
выделенном раннере; локально он не гейт (PROCESS.md §8), но раз ручного
|
||||
тестирования в цикле нет, я обязан оценить это сам, а не поверить на слово,
|
||||
тем более что автор сам пишет: «После зелёного слияния AC11 будет заново
|
||||
проверен полным exact-SHA Full Performance» — то есть саму гипотезу фикса
|
||||
никто ещё не проверял исполнением.
|
||||
|
||||
**Метод 1 — прямой A/B на двух git worktree (одна машина, один Chromium).**
|
||||
`git worktree add /tmp/wt/base origin/dev` (= `3270e03`, код **до** фикса, тот
|
||||
самый кандидат, что провалил prerelease-гейт), собрал бандл там же. Прогнал
|
||||
официальный `demo/benchmark_large_house.mjs --profile=large-house-isometric-v1`
|
||||
с `--target-root` на обоих деревьях, 5 samples/1 warmup:
|
||||
|
||||
| | `viewToggleMs.median` |
|
||||
|---|---:|
|
||||
| `origin/dev` (до фикса) | **244,4 мс** |
|
||||
| `HEAD` (после фикса) | **237,8 мс** |
|
||||
|
||||
Разница — **2,7%**, в пределах шума одного прогона (p95 у обоих ~320 мс).
|
||||
Абсолютные числа выше, чем в CI (другое железо, не pinned раннер) — это
|
||||
ожидаемо и не главное; главное — **относительное** улучшение практически
|
||||
нулевое, тогда как заявленный дефицит бюджета — `192,8 → 131,7` мс, то есть
|
||||
требуется снижение примерно на **треть**.
|
||||
|
||||
**Метод 2 — разбивка round-trip на направления.** `viewToggleMs` в бенчмарке
|
||||
измеряет `_setProjection('flat')` **и** `_setProjection('iso')` одним общим
|
||||
таймером (`demo/benchmark_large_house.mjs:172-183`). Я измерил оба перехода
|
||||
раздельно (тот же fixture/preload, 6 сэмплов каждое дерево):
|
||||
|
||||
| | `toFlatMs` (Iso→Flat, цель фикса) | `backToIsoMs` (Flat→Iso, cache hit) |
|
||||
|---|---:|---:|
|
||||
| `origin/dev` (до фикса) | **174,2 мс** (медиана) | 70,4 мс |
|
||||
| `HEAD` (после фикса) | **178,1 мс** (медиана) | 55,3 мс |
|
||||
|
||||
Направление, которое фикс должен был ускорить (Iso→Flat: «переход в Flat
|
||||
повторял тяжёлую polygon-union операцию»), **не стало быстрее** — медиана
|
||||
даже чуть выше на данном прогоне (в пределах шума по 6 сэмплам, диапазон
|
||||
141–215 мс на обоих деревьях). Сумма медиан обеих сторон (174,2+70,4≈244,6;
|
||||
178,1+55,3≈233,4) согласуется с Методом 1 — методология внутренне
|
||||
непротиворечива.
|
||||
|
||||
**Метод 3 — микробенчмарк изолированной операции, вне браузера/DOM.** Чтобы
|
||||
понять, сколько именно стоит сама «повторная polygon-union операция», которую
|
||||
чинит патч, я собрал прокси-фикстуру одного этажа `large-house-isometric-v1`
|
||||
(20 комнат в сетке 5×4, 20 перегородок, 13 колонн, 34 проёма — те же
|
||||
пропорции, что `demo/fixtures/large-house.mjs` даёт на один `space`) и вызвал
|
||||
скомпилированные (`test-build/`, тот же вывод, что использует `npm test`)
|
||||
`wallBodiesGeometry`/`wallBodiesUnionPath`/`wallBodiesPathFromGeometry`
|
||||
напрямую, без браузера и без Lit:
|
||||
|
||||
| Сценарий | Медиана (8 замеров, без первого JIT-прогрева) |
|
||||
|---|---:|
|
||||
| Один вызов `wallBodiesGeometry` (одна iso-сборка) | 13,1 мс |
|
||||
| **До фикса**: `wallBodiesGeometry` дважды (iso-сборка + `wallBodiesUnionPath` для flat) | **18,4 мс** |
|
||||
| **После фикса**: `wallBodiesGeometry` один раз + дешёвый `wallBodiesPathFromGeometry` | **8,0 мс** |
|
||||
|
||||
Сама операция, которую убирает патч, **экономит ≈10 мс** на этаж такого
|
||||
масштаба (56% от своей собственной стоимости, но малая доля от общего
|
||||
перехода). Это подтверждает, что патч технически корректен и действительно
|
||||
устраняет дублирующий вызов (см. «Что проверено и корректно» — сам рефакторинг
|
||||
чист), но объясняет, почему это не видно в Методах 1–2: **≈10 мс экономии на
|
||||
фоне ≈150–180 мс полного перехода Iso→Flat — это подавляющая часть стоимости
|
||||
перехода лежит не в polygon-union, а где-то ещё** (вероятный кандидат —
|
||||
полная Lit-перерисовка/пересборка DOM большой SVG-сцены при смене режима
|
||||
проекции: 20 комнат, 500 decor, 200 устройств на диапазон фикстуры, структурно
|
||||
разные шаблоны flat/iso, которые lit-html не может частично патчить и
|
||||
вынужден перестраивать целиком — но диагностика первопричины не входит в мой
|
||||
мандат ревьюера, чинит автор).
|
||||
|
||||
**Вывод по AC11.** Три независимых метода (браузерный A/B на реальном коде,
|
||||
разбивка по направлениям, изолированный микробенчмарк вне браузера) указывают
|
||||
в одну сторону: заявленное исправление даёт реальную, но на два порядка
|
||||
меньшую экономию (~10 мс), чем требуется, чтобы закрыть разрыв бюджета
|
||||
(нужно убрать ~61 мс, `192,8 → 131,7`). Локальное железо отличается от pinned
|
||||
CI-раннера, поэтому абсолютные числа не заменяют exact-SHA Full Performance —
|
||||
но соотношение «экономия патча ≪ требуемая экономия» получено на одной и той
|
||||
же машине для обеих сторон сравнения и не зависит от абсолютной скорости
|
||||
железа. Я оцениваю AC11 как **не доказанным исполнением** — есть прямое
|
||||
эмпирическое основание полагать, что regresssion сохранится на следующем
|
||||
exact-SHA прогоне.
|
||||
|
||||
## Находки
|
||||
|
||||
### H1 (блокирует) — исправление, вероятно, не закрывает AC11
|
||||
|
||||
**Файлы:** `src/houseplan-card.ts:4421-4485`, `src/wall-thickness.ts:1228-1238`.
|
||||
|
||||
**Сценарий отказа:** пользователь/CI публикует `v1.63.0-beta.1` после этого
|
||||
цикла; exact-SHA `Full Performance` на финальном кандидате снова измеряет
|
||||
`large-house-isometric-v1`; `viewToggleMs.median` остаётся в диапазоне
|
||||
≈180–235 мс (см. три метода выше) против лимита 131,7 мс — тот же красный
|
||||
результат, что уже был получен 2026-08-13T14:47:46Z на предыдущем кандидате,
|
||||
только после того как issue пройдёт весь цикл заново (а бюджет циклов
|
||||
код-ревью уже исчерпан — это r4/4).
|
||||
|
||||
**Почему не блокирую как «нужно больше тестов, а не факт»:** воспроизвёл на
|
||||
трёх независимых уровнях (полный браузерный round-trip, раздельные
|
||||
направления, чистый вычислительный микробенчмарк без браузера) с
|
||||
консистентным результатом на одной машине для обеих сторон сравнения
|
||||
(`origin/dev` vs `HEAD`); ни один из трёх не показывает улучшения,
|
||||
приближающегося к требуемому. Это не «гейт не запускался», а «гейт по сути
|
||||
эквивалентен запущен, и он показывает, что фикс не достигает цели».
|
||||
|
||||
**Рекомендация (не входит в мой мандат менять код, но для протокола):**
|
||||
профилировать сам переход Iso→Flat (Chrome Performance/flamegraph, не только
|
||||
таймер снаружи) и найти, где реально уходят ~150+ мс — по всем признакам это
|
||||
не polygon-union, а стоимость самой Lit-перерисовки структурно разных
|
||||
flat/iso-шаблонов на фикстуре с 500 decor/200 устройств.
|
||||
|
||||
## Что проверено и корректно
|
||||
|
||||
- **Сам рефакторинг `wallBodiesPathFromGeometry`.** Извлечённая функция
|
||||
побайтово повторяет прежнюю инлайн-логику `wallBodiesUnionPath`
|
||||
(`d = polyclipToPathD(united.geom); return {d, depthUnits: united.depthUnits,
|
||||
fillRule: 'evenodd'}`) — проверено чтением диффа `wall-thickness.ts` и
|
||||
прогоном нового юнита `wallBodiesPathFromGeometry reuses the canonical union
|
||||
without changing its flat path`, который сравнивает результат с прямым
|
||||
`wallBodiesUnionPath` через `assert.deepEqual` — green. Тест способен
|
||||
падать: любое расхождение в `d`/`depthUnits`/`fillRule` между переиспользуемым
|
||||
и прямым путём завалит `deepEqual`.
|
||||
- **`_wallUnionKey()` — чистое извлечение.** Формула идентична прежней
|
||||
инлайн-строке в `_renderWallBodies()` (`${this._space}|${this._cfgEpoch}|${...rooms.length}`)
|
||||
— проверено чтением, оба места дают одну и ту же строку при одинаковом
|
||||
состоянии.
|
||||
- **Отсутствие визуальной регрессии.** Полный `golden:capture` (47/47 сцен,
|
||||
включая все 6 `isometric-*`) — 0 diff. Это ожидаемо: путь переиспользования
|
||||
активен только при `_labsIso` (скрытый флаг), а математика union не
|
||||
меняется, только пропускается повторный вызов.
|
||||
- **Отсутствие функциональной регрессии.** Все 127 браузерных смоков зелёные,
|
||||
включая оба iso-специфичных (`smoke_isometric_live_touch.mjs` 21/21,
|
||||
`smoke_isometric_contract.mjs` 14/14) и весь остальной набор (Glow, sun,
|
||||
openings, tray, editors, backup и т.д. — ничего в этом коммите их не
|
||||
касается, но полный повтор исключает побочный эффект).
|
||||
- **Трейлеры и процесс.** `Issue: #89`, `User-Visible: no` — верно (Labs-скрытая
|
||||
фича, публичный changelog не требуется и не тронут). `process-gate.mjs
|
||||
--issues` — green.
|
||||
- **Отсутствие влияния на geometry/инвалидацию кэша.** Автор пишет «Геометрия,
|
||||
ключи инвалидизации и fallback-семантика не менялись» — подтверждаю чтением:
|
||||
`_isoSource().key` (fingerprint для `_isoGeometryCache`) не менялся; новый
|
||||
`wallKey` использует ту же формулу, что уже была единственным ключом
|
||||
`_wallUnionCache` до этого коммита, только вынесенную в метод.
|
||||
- **Малый наблюдаемый риск с порядком extra-тел (не блокирует, см. ниже
|
||||
почему).** `_isoSource().build()` берёт `extras` через отдельный вызов
|
||||
`physicalBodies(space, cellCm, gridPitch)` (порядок: partitions → drafts →
|
||||
columns), тогда как `_renderWallBodies()` использует мемоизированный
|
||||
`_physicalBodiesR()` (порядок: drafts → partitions → columns). После этого
|
||||
коммита один и тот же кэш `_wallUnionCache` может быть заполнен результатом,
|
||||
посчитанным с любым из двух порядков — раньше он всегда шёл только через
|
||||
`_physicalBodiesR()`. Union — коммутативная операция над одним и тем же
|
||||
набором тел, `wallBodiesGeometry` считает `maxDepth` через `Math.max` по
|
||||
всем телам независимо от порядка, а `golden:capture` не показал diff ни на
|
||||
одной из 6 iso-сцен и ни на одной из существующих 41 — считаю это Low,
|
||||
фиксирую с записью, не завожу отдельный issue: наблюдаемого расхождения нет,
|
||||
риск чисто теоретический (устойчивость boolean-clip к порядку слияния в
|
||||
вырожденных случаях), и я не смог построить конкретный сценарий отказа.
|
||||
|
||||
## Чего не проверял
|
||||
|
||||
- **Exact-SHA Full Performance сам по себе** — не CI-раннер, недоступен
|
||||
локально; вместо него — три независимых локальных метода выше, которые я
|
||||
считаю **достаточным** основанием для красного вердикта, но не заменой
|
||||
официального гейта. Финальное число может отличаться от локального в
|
||||
абсолютных мс (другое железо), но не в качественном выводе «фикс не
|
||||
устраняет причину».
|
||||
- **Первопричину ~150–180 мс в Iso→Flat** — не профилировал flamegraph'ом
|
||||
внутри браузера (Chrome DevTools Performance API недоступен из headless
|
||||
`page.evaluate` в использованном мной harness без дополнительной настройки
|
||||
трассировки); указываю наиболее вероятную гипотезу (Lit-перерисовка
|
||||
структурно разных шаблонов), но не подтверждаю её как единственную причину.
|
||||
- **HA-harness backend-тесты** (`test_ha_*.py`) — не запускал: диапазон не
|
||||
трогает `custom_components/**/*.py`, гейт пуст по построению.
|
||||
- **Safari/WebKit и Firefox** — вне скоупа Stage 1 (ADR), тестировал только
|
||||
Chromium.
|
||||
|
||||
## Вердикт
|
||||
|
||||
**Красный · цикл r4/4 · High: 1 · Medium: 0 → #… (нет новых issue: находка H1 —
|
||||
возврат в рамках того же issue, не отдельный дефект).**
|
||||
|
||||
Единственная блокирующая находка — H1: три независимых локальных измерения
|
||||
(браузерный round-trip A/B на двух worktree одного и того же кода до/после
|
||||
фикса, раздельные направления перехода, изолированный вычислительный
|
||||
микробенчмарк вне браузера) сходятся на том, что заявленное исправление
|
||||
экономит ≈10 мс на характерном для профиля объёме геометрии, тогда как
|
||||
prerelease-гейт требует закрыть разрыв примерно в 61 мс. Остальной диапазон
|
||||
чист: рефакторинг сам по себе корректен и покрыт тестом, который умеет падать,
|
||||
регрессий в 127/127 смоках и 47/47 golden-сцен нет, трейлеры и процесс-гейт
|
||||
зелёные.
|
||||
|
||||
Это **r4/4** — четвёртый и последний разрешённый цикл код-ревью для этого
|
||||
issue (PROCESS.md §4). Пятого захода нет: по правилам процесса задача уходит
|
||||
владельцу на разбор — разделить, отклонить эту оптимизацию как отдельный шаг
|
||||
(например, вынести Stage 1 public rollout дальше, если сама по себе
|
||||
Labs-функция не обязана укладываться в этот бюджет ещё на этой неделе) или
|
||||
арбитраж. Я как ревьюер не имею права предлагать пятый цикл под видом
|
||||
«доработки» — это прямо запрещено §12.
|
||||
+24
-6
@@ -55,7 +55,8 @@ import {
|
||||
import {
|
||||
degradeWalls, rekeyWallsAfterMove,
|
||||
setWallThickness, setWallThicknessForRoom, cmToField, wallCmToUnits,
|
||||
wallEdgeBodies, wallBodiesGeometry, wallBodiesUnionPath, paperRoomShapesWithWalls,
|
||||
wallEdgeBodies, wallBodiesGeometry, wallBodiesPathFromGeometry, wallBodiesUnionPath,
|
||||
paperRoomShapesWithWalls,
|
||||
innerContourForRoom, roomWallProfile, outsetContour,
|
||||
openingInnerFaceOffsetFromIndex, openingTunnelGeometriesFromIndex,
|
||||
openingWallIndex as buildOpeningWallIndex, applyWallThicknessToNewRoom,
|
||||
@@ -4418,7 +4419,15 @@ class HouseplanCard extends LitElement {
|
||||
}
|
||||
|
||||
/** The rectangle "fit to screen" fits — always the content (docs/CANVAS.md). */
|
||||
private _isoSource(): { key: string; build: () => any } {
|
||||
private _wallUnionKey(): string {
|
||||
return `${this._space}|${this._cfgEpoch}|${this._spaceModel().rooms.length}`;
|
||||
}
|
||||
|
||||
private _isoSource(): {
|
||||
key: string;
|
||||
wallKey: string;
|
||||
build: () => { geom: any; depthUnits: number };
|
||||
} {
|
||||
const space = this._spaceModel();
|
||||
const walls = this._spaceWalls;
|
||||
const openCuts = this._openPairs().flatMap((pair) => pair.segs);
|
||||
@@ -4436,6 +4445,7 @@ class HouseplanCard extends LitElement {
|
||||
})}`;
|
||||
return {
|
||||
key,
|
||||
wallKey: this._wallUnionKey(),
|
||||
build: () => {
|
||||
const extras = physicalBodies(space, this._cellCm, this._gridPitch);
|
||||
const united = walls.length || extras.length
|
||||
@@ -4443,9 +4453,9 @@ class HouseplanCard extends LitElement {
|
||||
space.rooms, walls, openCuts, openings,
|
||||
this._wallKeyPitch, this._cellCm, this._gridPitch, NORM_W, extras,
|
||||
)
|
||||
: { geom: [] };
|
||||
: { geom: [], depthUnits: 0 };
|
||||
if (!united) throw new Error('wall boolean geometry failed');
|
||||
return united.geom;
|
||||
return united;
|
||||
},
|
||||
};
|
||||
}
|
||||
@@ -4461,7 +4471,15 @@ class HouseplanCard extends LitElement {
|
||||
if (cached) return { key: source.key, ...cached };
|
||||
const flat = this._frameOf().rect;
|
||||
const frame = projectedFrame({ rect: flat, wallHeight: ISO_WALL_HEIGHT });
|
||||
const geometry = buildIsoWallGeometry(source.build());
|
||||
const united = source.build();
|
||||
const geometry = buildIsoWallGeometry(united.geom);
|
||||
// Flat and iso are two projections of the same canonical union. Seed the
|
||||
// existing one-entry flat cache while that union is already in hand so a
|
||||
// Flat -> Volumetric toggle does not repeat the polygon boolean pass.
|
||||
this._wallUnionCache = {
|
||||
key: source.wallKey,
|
||||
value: wallBodiesPathFromGeometry(united),
|
||||
};
|
||||
const value = { geometry, frame };
|
||||
lruWrite(this._isoGeometryCache, source.key, value, 8);
|
||||
return { key: source.key, ...value };
|
||||
@@ -10018,7 +10036,7 @@ class HouseplanCard extends LitElement {
|
||||
angle: Number(o.angle) || 0,
|
||||
length: (Number(o.length) > 0 ? Number(o.length) : 0.9) * NORM_W,
|
||||
}));
|
||||
const unionKey = `${this._space}|${this._cfgEpoch}|${this._spaceModel().rooms.length}`;
|
||||
const unionKey = this._wallUnionKey();
|
||||
if (!this._wallUnionCache || this._wallUnionCache.key !== unionKey) {
|
||||
this._wallUnionCache = {
|
||||
key: unionKey,
|
||||
|
||||
+16
-2
@@ -1225,6 +1225,20 @@ function polyclipToPathD(geom: any): string {
|
||||
return d;
|
||||
}
|
||||
|
||||
/**
|
||||
* Project an already computed canonical wall union into the flat SVG path.
|
||||
* Isometric rendering and the ordinary flat wall layer share the same boolean
|
||||
* result; keeping this conversion separate prevents a projection toggle from
|
||||
* repeating the expensive polygon union merely to obtain its path string.
|
||||
*/
|
||||
export function wallBodiesPathFromGeometry(
|
||||
united: { geom: any; depthUnits: number } | null,
|
||||
): { d: string; depthUnits: number; fillRule: 'evenodd' } | null {
|
||||
if (!united) return null;
|
||||
const d = polyclipToPathD(united.geom);
|
||||
return d ? { d, depthUnits: united.depthUnits, fillRule: 'evenodd' } : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Mitre patches at an endpoint where a virtual stretch meets real walls that
|
||||
* belong to different room contours.
|
||||
@@ -1462,8 +1476,8 @@ export function wallBodiesUnionPath(
|
||||
const united = wallBodiesGeometry(
|
||||
rooms, walls, openCuts, openings, pitch, cellCm, gridPitch, coordScale, extraBodies,
|
||||
);
|
||||
const d = united ? polyclipToPathD(united.geom) : '';
|
||||
if (united && d) return { d, depthUnits: united.depthUnits, fillRule: 'evenodd' };
|
||||
const projected = wallBodiesPathFromGeometry(united);
|
||||
if (projected) return projected;
|
||||
if (united) return null; // successful empty result: do not resurrect raw rings
|
||||
// fall back to evenodd rings concatenated
|
||||
const rings = wallBodyRings(rooms, walls, openCuts, pitch, cellCm, gridPitch, coordScale);
|
||||
|
||||
@@ -6,7 +6,8 @@ import {
|
||||
setWallThickness, setWallThicknessForRoom, applyWallThicknessToNewRoom,
|
||||
drawWallPreviewD, DRAW_WALL_DEFAULT_CM, clampWallCm, cmToField, fieldToCm,
|
||||
wallCmToUnits, insetContour, inwardNormal, edgeKinds, wallEdgeBodies,
|
||||
wallBodyRings, wallBodiesUnionPath, innerContourForRoom,
|
||||
wallBodyRings, wallBodiesGeometry, wallBodiesPathFromGeometry, wallBodiesUnionPath,
|
||||
innerContourForRoom,
|
||||
paperRoomShapesWithWalls, WALL_MIN_CM, WALL_MAX_CM, MITRE_LIMIT,
|
||||
atomicPolyForRoom, insetOffsetsForRoom, wallIntervals, materializeWallIntervals,
|
||||
normalizeWallIntervals,
|
||||
@@ -669,6 +670,19 @@ test('wallBodiesUnionPath: single fully-thick room keeps a floor hole', () => {
|
||||
assert.ok((united.d.match(/M/g) || []).length >= 2, united.d);
|
||||
});
|
||||
|
||||
test('wallBodiesPathFromGeometry reuses the canonical union without changing its flat path', () => {
|
||||
const room = { id: 'shared', poly: [[100, 100], [300, 100], [300, 300], [100, 300]] };
|
||||
const walls = applyWallThicknessToNewRoom([], [room], 'shared', 15, 0.01, [], 1000);
|
||||
const geometry = wallBodiesGeometry(
|
||||
[room], walls, [], [], 0.01, cellCm, GRID_PITCH, 1000,
|
||||
);
|
||||
const reused = wallBodiesPathFromGeometry(geometry);
|
||||
const direct = wallBodiesUnionPath(
|
||||
[room], walls, [], [], 0.01, cellCm, GRID_PITCH, 1000,
|
||||
);
|
||||
assert.deepEqual(reused, direct);
|
||||
});
|
||||
|
||||
test('wallBodiesUnionPath: a parent floor never erases a nested room wall', () => {
|
||||
const scale = 1000;
|
||||
const rooms = [
|
||||
|
||||
Reference in New Issue
Block a user