Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions app/src/city/components/buildings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ export interface Buildings extends SceneComponent {
disposeAdPanels(): void;
/** Building lookup by file path → { mesh, building, instanceId }. */
getBuildingByPath(p: string): { mesh: THREE.Mesh; building: Building; instanceId: number } | null;
/** Tallest building (layout pos + dims), or null when empty. */
getTallest(): { x: number; y: number; w: number; d: number; h: number } | null;
/** Cell map (consumed by picker / fader / outline / diff mirror). */
getCells(): Map<number, CellTile>;
/** Building index, or null pre-rebuild. */
Expand Down Expand Up @@ -479,16 +477,6 @@ export function createBuildings(ctx: SceneContext): Buildings {
onResize,
dispose,
getBuildingByPath: (p) => _buildingsByPath[p] || null,
getTallest: () => {
let tallest: Building | null = null;
for (const cell of _cells.values()) {
for (const b of cell.buildings) {
if (b && (!tallest || b.h > tallest.h)) tallest = b;
}
}
if (!tallest) return null;
return { x: tallest.x, y: tallest.y, w: tallest.w, d: tallest.d, h: tallest.h };
},
getCells: () => _cells,
getBuildingIndex: () => _buildingIndex,
getMeshForBuilding,
Expand Down
24 changes: 11 additions & 13 deletions app/src/city/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,32 +108,30 @@ export async function createCity(canvas: HTMLCanvasElement, manifest: Manifest):
// BEFORE the rig (so bbox is set and the rig's first frame can frame the city).
await applyManifest(manifest);

// cityState is threaded so the rig re-frames reactively when bbox changes
// and reads its world-framing inputs (bbox/gem/root street) directly. deps
// carries only the component-geometry accessors the rig can't reach via state.
// cityState is threaded so the rig re-frames reactively when bbox changes and
// reads its world-framing inputs (bbox/gem/root street/tallest building)
// directly. deps carries only the component-geometry accessors the rig can't
// reach via state.
const rig = createCameraRig({
canvas,
cityState,
deps: {
getTallestBuilding: () => buildings.getTallest(),
getRepoLabelBounds: () => repoLabel.getPanelBounds(),
getTreeBoundsBySha: (sha) => trees.getRenderer()?.getTreeBoundsBySha(sha) ?? null,
},
});
// Reframe the camera on a SOURCE change (repo switch) — the one camera action
// that used to live in the view. bbox is reassigned on every non-reuse apply,
// so the rig's framing is freshly captured by its own effect just above by the
// time this fires; peeking CURRENT_SOURCE_KEY filters to repo switches only —
// a config rebuild / live-update / the skeleton apply (before the fetch layer
// commits the new key) all keep the same key → no snap. reset() is untracked
// so this effect depends only on bbox.
// Snap the camera when a NEW source's city has applied (initial load or repo
// switch). Track cityRevision (every apply), not bbox: the final manifest is a
// reuse apply that leaves bbox frozen, so a bbox-only effect would miss it. The
// key guard skips the empty boot (key null — no source yet) and same-source
// re-applies (live-updates, config saves) — only a real source change reframes.
let lastReframedSourceKey: string | null = null;
const stopReframe = effect(() => {
void cityState.bbox.value;
void cityState.cityRevision.value;
const key = CURRENT_SOURCE_KEY.peek();
if (key !== null && key !== lastReframedSourceKey) {
untracked(() => rig.reset());
lastReframedSourceKey = key;
untracked(() => rig.reset());
}
});

Expand Down
39 changes: 12 additions & 27 deletions app/src/city/render/cameraRig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,10 @@ import type { Building, PickTarget, Street } from '@/types';
import type { CityState } from '@/city/state';

/** Narrow accessor surface the rig needs from the composer for component
* geometry (buildings/repoLabel/trees). World framing inputs (bbox, gem,
* root street) come from cityState directly, not through here. createCity
* geometry (repoLabel/trees). World framing inputs (bbox, gem, root street,
* tallest building) come from cityState directly, not through here. createCity
* (city/index.ts) passes a small deps literal that satisfies this. */
export interface CameraRigDeps {
getTallestBuilding(): { x: number; y: number; w: number; d: number; h: number } | null;
getRepoLabelBounds(): {
centerX: number;
centerY: number;
Expand Down Expand Up @@ -258,7 +257,7 @@ export function createCameraRig({
// Take the max across the 4 roof corners. HEADROOM scales D up for
// breathing room above the roof (1.0 = spire flush against top edge).
let heightDist = 0;
const tallest = gemPos && rootStreet ? deps.getTallestBuilding() : null;
const tallest = gemPos && rootStreet ? cityState.tallestBuilding.value : null;
const labelBounds = gemPos && rootStreet ? deps.getRepoLabelBounds() : null;
if (tallest || labelBounds) {
const sinElev = dir.y;
Expand Down Expand Up @@ -293,22 +292,13 @@ export function createCameraRig({
}
}
}
// Include the floating repo-label panel so empty worlds (no
// buildings) still frame to show the label, and crowded worlds
// never crop it off the top edge. The panel billboards to face
// the camera, so its horizontal extent could rotate either way —
// sample the top corners along BOTH world axes to bound it.
// Fit the label's top-edge HEIGHT only, not its width: the panel is
// centered on the gem and already inside the width-fit frame, and its
// width comes from the text-texture aspect, which only settles on web-font
// load — sampling it made the frame jump after load (issue #62).
if (labelBounds) {
const topY = labelBounds.centerY + labelBounds.halfHeight;
const r = labelBounds.halfWidth;
for (const [dx, dz] of [
[-r, 0],
[r, 0],
[0, -r],
[0, r],
]) {
_fitPoint(labelBounds.centerX + dx, topY, labelBounds.centerZ + dz);
}
_fitPoint(labelBounds.centerX, topY, labelBounds.centerZ);
}
heightDist *= TALLEST_BUILDING_HEADROOM_MULT;
}
Expand Down Expand Up @@ -397,15 +387,10 @@ export function createCameraRig({
}

function reset() {
// If framing wasn't captured yet (or was cleared by a disposal cycle),
// try once more before giving up — silent no-op on R is worse than
// re-running the cheap framing computation. Returns false only when
// the city has no bbox at all (e.g., pre-manifest), in which case
// there's nothing to reset to.
if (!initialCamPos || !initialTarget) {
if (!_captureFraming()) return;
if (!initialCamPos || !initialTarget) return;
}
// Recapture from the CURRENT layout every call: the final manifest is a reuse
// apply (bbox frozen), so the bbox effect that normally caches the pose
// doesn't re-fire there — a cached pose could be stale. False only pre-manifest.
if (!_captureFraming() || !initialCamPos || !initialTarget) return;
// Cancel any in-flight focus/reset animation so it can't keep
// walking the camera away from the snap target.
camAnimToken++;
Expand Down
27 changes: 23 additions & 4 deletions app/src/city/state/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,16 @@
// trees) rebuild off it. treePlacements is a source signal trees writes.
// structureRevision / cityRevision / decorationRevision — change-notification
// counters; consumers track them and peek the data.
// bbox / sceneBbox / cityHeight / latestWorldBounds / rootStreet / gemWorldPos
// COMPUTED, never written. bbox = union of street rects + building footprints
// + footprint halo.
// bbox / sceneBbox / cityHeight / latestWorldBounds / rootStreet / gemWorldPos /
// tallestBuilding — COMPUTED, never written. bbox = union of street rects +
// building footprints + footprint halo.
//
// applyManifest + invalidateLayoutCache are defined here (the store owns its
// transition); every scene component rebuilds reactively off the signals above.
import { signal, computed, batch, type Signal, type ReadonlySignal } from '@preact/signals';
import * as THREE from 'three';
import { FOOTPRINT } from '@/state/stores/settings/footprint';
import type { CityBbox, CityLayout, Manifest, Street } from '@/types';
import type { Building, CityBbox, CityLayout, Manifest, Street } from '@/types';
import { getWorldBounds, type WorldBounds } from '../utils/floorBounds';
import { rectOfStreet } from '@/city/layout/rect';
import type { TreePlacement } from '../components/trees/treePlacement';
Expand Down Expand Up @@ -45,6 +45,9 @@ export interface CityState {
treePlacements: Signal<TreePlacement[] | null>;
readonly rootStreet: ReadonlySignal<Street | null>;
readonly gemWorldPos: ReadonlySignal<THREE.Vector3 | null>;
// Tallest building (by height) for the camera start-framing height-fit. From
// layout data, not the async building meshes (see the computed).
readonly tallestBuilding: ReadonlySignal<Building | null>;
// { street dir.path → Street } from the layout. The buildings fader, pathLine,
// picker, and the CityWorld debug API resolve a street by directory off this
// instead of reaching into the streets component.
Expand Down Expand Up @@ -176,6 +179,21 @@ export function createCityState(layoutClient: ReturnType<typeof createLayoutClie
return new THREE.Vector3(a.x, 0, a.y);
});

// Tracks `layout` (every apply), NOT structureRevision: per-building dims
// (incl. the worker's media-silhouette sizing) recompute on a reuse apply —
// the skeleton→final swap turns placeholder heights real without bumping
// structureRevision — so the height must refresh there. Reading the layout
// directly also means framing never waits on the async building-mesh rebuild.
const tallestBuilding = computed<Building | null>(() => {
const l = layout.value;
if (!l) return null;
let tallest: Building | null = null;
for (const b of l.buildings) {
if (!tallest || b.h > tallest.h) tallest = b;
}
return tallest;
});

// Street lookup by directory path. Tracks structureRevision + peeks layout
// (ref-stable on a reuse apply, recomputes on a structure change) — a
// { dir.path → Street } map derived straight from layout.streets.
Expand Down Expand Up @@ -274,6 +292,7 @@ export function createCityState(layoutClient: ReturnType<typeof createLayoutClie
treePlacements,
rootStreet,
gemWorldPos,
tallestBuilding,
streetsByDirMap,
structureRevision,
cityRevision,
Expand Down
35 changes: 34 additions & 1 deletion app/tests/city/cityState.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

import { describe, it, expect } from 'vitest';
import { StreetAxis, NodeKind } from '@/types';
import type { CityLayout, Street } from '@/types';
import type { Building, CityLayout, Street } from '@/types';
import { makeCityState } from '../_helpers/cityFixtures';

// Minimal Street — rootStreet only reads streets[].isRoot; gemWorldPos reads
Expand Down Expand Up @@ -159,4 +159,37 @@ describe('createCityState', () => {
expect(cs.rootStreet.value).toBe(r2);
expect(cs.gemWorldPos.value!.x).not.toBe(z1);
});

describe('tallestBuilding', () => {
const bld = (over: Partial<Building>): Building =>
({ x: 0, y: 0, w: 10, d: 10, h: 10, ...over }) as unknown as Building;
const layoutOf = (buildings: Building[]): CityLayout =>
({ buildings, streets: [] }) as unknown as CityLayout;

it('is null with no layout and with no buildings', () => {
const cs = makeCityState();
expect(cs.tallestBuilding.value).toBeNull();
cs.layout.value = layoutOf([]);
expect(cs.tallestBuilding.value).toBeNull();
});

it('returns the max-height building', () => {
const cs = makeCityState();
const tall = bld({ x: 5, y: 6, w: 3, d: 4, h: 90 });
cs.layout.value = layoutOf([bld({ h: 10 }), tall, bld({ h: 50 })]);
expect(cs.tallestBuilding.value).toBe(tall);
});

// Crux of #62: a reuse apply (skeleton placeholder → real heights) reassigns
// `layout` without bumping structureRevision, so tallestBuilding must track
// `layout` or it'd frame to the stale placeholder height.
it('updates when layout is reassigned without a structureRevision bump', () => {
const cs = makeCityState();
cs.layout.value = layoutOf([bld({ h: 20 })]); // skeleton placeholder height
expect(cs.tallestBuilding.value!.h).toBe(20);

cs.layout.value = layoutOf([bld({ h: 200 })]); // final real height — no structureRevision++
expect(cs.tallestBuilding.value!.h).toBe(200);
});
});
});
5 changes: 1 addition & 4 deletions app/tests/city/components/buildings/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// Tests for the persistent createBuildings(ctx) component.
// API: createBuildings(ctx) → { group, rebuild(layout, dateRanges),
// disposeAdPanels, tick(dt, frame), onResize(), dispose(),
// getBuildingByPath, getTallest, getCells, getBuildingIndex,
// getBuildingByPath, getCells, getBuildingIndex,
// getMeshForBuilding }.
//
// The shared-material theme effect reacts to BUILDINGS/SCENE/BLOOM at
Expand Down Expand Up @@ -166,9 +166,6 @@ describe('createBuildings()', () => {
expect(buildings.getBuildingByPath('src/b.ts')!.building).toBe(b1);
expect(buildings.getBuildingByPath('nope')).toBeNull();

// getTallest reflects b1 (h=9).
expect(buildings.getTallest()).toEqual({ x: -20, y: -20, w: b1.w, d: b1.d, h: 9 });

// getMeshForBuilding resolves through the live cell (cellId/slotId were
// assigned during rebuild).
const resolved = buildings.getMeshForBuilding(b0);
Expand Down
Loading