From 5dec528cee36b4161e3aee2db413f37db284f99a Mon Sep 17 00:00:00 2001 From: Senrokai Date: Fri, 18 Sep 2026 13:24:16 +0200 Subject: [PATCH 1/3] Size the distance rings by what the frame reaches, and keep their labels off the star names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From the review of #19. The rings are distances from the Sun, but their step was taken from `effectiveDistance`, which under the plan view means the extent of the frame rather than how far the camera is from the Sun. Centred on a star 200 pc out and flipped to 2D, the grid became rings of 2 to 20 pc: not one of them on screen. The step now comes from where the view is centred plus how far the camera is orbiting it, which is the same distance under either projection. The set was also rebuilt while the grid was hidden, and every rebuild disposes the rings and builds every vertex again; it now happens only while the grid is drawn. The ring labels went straight to the overlay: never culled to the frame, and free to land on a star's name. They now have to be on screen and clear of the names already placed, by half the separation two names keep — they are a ladder up one ray a twentieth of the screen apart, and holding them apart from each other would take "Survey edge" off the map. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi --- .../galaxy-system-scene.component.spec.ts | 89 +++++++++++++++++++ .../galaxy-system-scene.component.ts | 57 ++++++++++-- src/app/shared/format/scale-bar.ts | 7 +- 3 files changed, 141 insertions(+), 12 deletions(-) diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts index e669b9b..70d5d66 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts @@ -15,6 +15,7 @@ import { HudDisplay } from '../hud/hud-dock.component'; import { GalaxySystemSceneComponent } from './galaxy-system-scene.component'; import { JumpLinkRenderer } from './jump-link-renderer'; import { StarFieldRenderer } from './star-field-renderer'; +import { LabeledPoint, StarLabelOverlay } from './star-label-overlay'; // jsdom does not implement ResizeObserver; the component only uses it to react to real // layout changes, which never happen in this headless test. @@ -418,6 +419,94 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { }); }); + describe('the local grid of distance rings', () => { + type GridScene = { + controls: { target: THREE.Vector3; update(): void }; + display: { update(change: (display: HudDisplay) => HudDisplay): void }; + localGridRadii: readonly number[]; + }; + + it('sizes the rings by how far the frame reaches from the Sun, under either projection', async () => { + const component = fixture.componentInstance as unknown as GridScene; + const camera = engine.getCamera(); + // Centred on a star 200 pc out, seen from 20 pc away: the rings have to reach it. + component.controls.target.set(200, 0, 0); + camera.position.set(200, 0, 20); + component.controls.update(); + await advanceFrames(engine, 0.3); + const underPerspective = [...component.localGridRadii]; + + component.display.update((display) => ({ ...display, plan: true })); + TestBed.tick(); + await advanceFrames(engine, 0.3); + + expect(underPerspective.at(-1)).toBeGreaterThanOrEqual(200); + // The plan view's wheel moves the frame rather than the camera, so "how far out the camera + // is" means something else there; what the rings have to cover does not. + expect([...component.localGridRadii]).toEqual(underPerspective); + }); + + it('leaves the rings alone while the grid is not drawn', async () => { + const component = fixture.componentInstance as unknown as GridScene; + const camera = engine.getCamera(); + await advanceFrames(engine, 0.3); + component.display.update((display) => ({ ...display, grid: false })); + TestBed.tick(); + await advanceFrames(engine, 0.3); + const hidden = [...component.localGridRadii]; + + // A zoom this size crosses two round steps, and each crossing rebuilds every ring's vertices. + camera.position.setLength(camera.position.length() / 8); + component.controls.update(); + await advanceFrames(engine, 0.3); + + expect([...component.localGridRadii]).toEqual(hidden); + }); + + it('drops a ring label that a star name has taken, or that is off screen, and keeps the ladder otherwise', () => { + const component = fixture.componentInstance as unknown as { + ringLabelsInTheClear(candidates: readonly LabeledPoint[], camera: THREE.Camera, stars: readonly LabeledPoint[]): LabeledPoint[]; + }; + const camera = engine.getCamera(); + camera.updateMatrixWorld(true); + const at = (x: number, y: number) => new THREE.Vector3(x, y, 0.5).unproject(camera); + const near = at(0.1, 0.1); + const nextRungUp = at(0.1, 0.16); + const offScreen = at(1.6, 0.1); + const ladder: LabeledPoint[] = [ + { id: 'ring-50', name: '50 pc', x: near.x, y: near.y, z: near.z }, + { id: 'ring-100', name: '100 pc', x: nextRungUp.x, y: nextRungUp.y, z: nextRungUp.z }, + { id: 'ring-150', name: '150 pc', x: offScreen.x, y: offScreen.y, z: offScreen.z } + ]; + + // A ladder of rings stays whole, though its rungs are closer than two star names would be. + expect(component.ringLabelsInTheClear(ladder, camera, []).map((label) => label.id)).toEqual(['ring-50', 'ring-100']); + // A star's name is worth more than a distance. + const star: LabeledPoint = { id: 7, name: 'Sirius', x: near.x, y: near.y, z: near.z }; + expect(component.ringLabelsInTheClear(ladder, camera, [star]).map((label) => label.id)).toEqual(['ring-100']); + }); + + it('places the ring labels with the star names rather than over them', async () => { + const component = fixture.componentInstance as unknown as GridScene; + const update = vi.spyOn(StarLabelOverlay.prototype, 'update'); + const cleared = vi.spyOn(GalaxySystemSceneComponent.prototype as unknown as { ringLabelsInTheClear: (...args: unknown[]) => LabeledPoint[] }, 'ringLabelsInTheClear'); + const camera = engine.getCamera(); + camera.position.set(0, 4, 10); + component.controls.target.set(0, 0, 0); + component.controls.update(); + await advanceFrames(engine, 0.3); + + const labels = (update.mock.calls.at(-1)?.[0] ?? []) as LabeledPoint[]; + const rings = labels.filter((label) => String(label.id).startsWith('ring-')); + expect(rings.length).toBeGreaterThan(0); + // Handed over as the clearing pass left them, not as the grid produced them. + expect(cleared).toHaveBeenCalled(); + expect(rings).toEqual(cleared.mock.results.at(-1)?.value); + update.mockRestore(); + cleared.mockRestore(); + }); + }); + it('keeps the stars of a plotted route drawn, and the selected star', async () => { const component = fixture.componentInstance as unknown as { routeResult: { set(value: unknown): void } }; const refocus = vi.spyOn(StarFieldRenderer.prototype, 'refocus'); diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.ts index 971993d..2250ae1 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.ts @@ -70,6 +70,12 @@ const LABEL_MAX_COUNT = 15; const LABEL_MIN_SEPARATION_NDC = 0.12; /** Beyond this the text of a right-hand label would run off the view: hang it on the left. */ const LABEL_EDGE_NDC = 0.7; +/** + * How far a ring label has to sit from a star's name, in NDC — half what two star names keep + * between them. A ring label is one short line, and the rungs of its ladder are a twentieth of the + * screen apart, so the full separation would have one name clear three rungs. + */ +const RING_LABEL_CLEARANCE_NDC = LABEL_MIN_SEPARATION_NDC / 2; /** How far right of its point a label's text reaches, in aspect-scaled NDC (~135px at 1440). */ const LABEL_REACH_NDC = 0.3; /** @@ -194,10 +200,11 @@ const GALACTIC_FAR_PC = 250000; */ const SURVEY_EDGE_PC = 250; /** - * The local grid's rings are distances from the Sun, at a round step that follows the camera: - * five of them out to about the camera's own distance, so 50 to 250 pc from the opening view and - * 2 to 10 pc from beside the Sun. A fixed set could only serve one end of the zoom: 50 pc rings - * say nothing from inside a 2 pc hop, and nothing marked the stars now drawn past 250 pc. + * How many rings the local grid aims for: the step is rounded down from a fifth of how far the + * frame reaches from the Sun, which makes five to fourteen of them. So 50 to 350 pc from the + * opening view, and 2 to 10 pc from beside the Sun. A fixed set could only serve one end of the + * zoom: 50 pc rings say nothing from inside a 2 pc hop, and nothing marked the stars now drawn + * past 250 pc. */ const LOCAL_GRID_RING_COUNT = 5; const LOCAL_GRID_SPOKES = 12; @@ -912,11 +919,29 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { } } + /** + * The ring labels worth drawing: the ones on screen, and clear of the star names already placed. + * + * Not held apart from each other, as the star names are: they are a ladder up one ray, a few + * hundredths of the screen apart, and reading them in order is the point. What they must not do + * is sit on a star's name, which is worth more than a distance — or be handed to the overlay + * from behind or beside the camera, which draws them at the edge of the page rather than not at all. + */ + private ringLabelsInTheClear(candidates: readonly LabeledPoint[], camera: SceneCamera, stars: readonly LabeledPoint[]): LabeledPoint[] { + const projected = new THREE.Vector3(); + const onScreen = (label: LabeledPoint): THREE.Vector2 | null => { + projected.set(label.x, label.y, label.z).project(camera); + const outside = projected.z < -1 || projected.z > 1 || Math.abs(projected.x) > 1 || Math.abs(projected.y) > 1; + return outside ? null : new THREE.Vector2(projected.x * this.viewportAspect(), projected.y); + }; + const taken = stars.map(onScreen).filter((point): point is THREE.Vector2 => point !== null); + return candidates.filter((label) => { + const point = onScreen(label); + return point !== null && !taken.some((other) => other.distanceTo(point) < RING_LABEL_CLEARANCE_NDC); + }); + } + private updateLabels(camera: SceneCamera): void { - const radii = distanceRings(this.effectiveDistance(camera), LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC); - if (radii.join() !== this.localGridRadii.join()) { - this.setLocalGridRadii(radii); - } const selectedId = this.navigationStore.selectedStarId(); // Measured from what the camera is looking at, not from where it is. Those differ by the // orbit distance, so a camera-relative rule names the stars closest to the near edge of the @@ -928,6 +953,20 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { // Individual star names mean nothing once the whole Galaxy is in frame — at that range the // entire catalogue is inside one pixel — so the labels hand over to the structural ones. const isGalactic = this.galacticStrength >= GALACTIC_LEVEL_THRESHOLD; + // The rings are distances from the Sun, so what they have to cover is how far from the Sun the + // frame reaches: where the view is centred, plus how far out the camera is orbiting it. Under + // the plan view the orbit distance is the frame's own extent, since that is what the wheel + // moves there. Read as "how far the camera is from the Sun" instead, panning away from the Sun + // and flipping to the plan view left every ring off the frame. + // Rebuilt only while the grid is drawn: each new set disposes the old rings and builds every + // vertex of the new ones, and the set changes on any zoom that crosses a round step. + if (!isGalactic && this.display().grid) { + const orbitPc = this.engine.currentProjection === 'perspective' ? camera.position.distanceTo(target) : this.effectiveDistance(camera); + const radii = distanceRings(target.length() + orbitPc, LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC); + if (radii.join() !== this.localGridRadii.join()) { + this.setLocalGridRadii(radii); + } + } // Brightest first, not nearest first. Proximity was the right ranking when the catalogue was // a 50 pc bubble and everything in it was equally worth naming; across 250 pc it labels a // clump of whatever happens to be closest to the middle of the screen and never names the @@ -944,7 +983,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { }; const starLabels: LabeledPoint[] = isGalactic ? [] : this.spreadLabels(candidates(this.stars, this.starsByBrightness), camera, selectedId); const backdropLabels = isGalactic ? this.galacticLabels : this.deepSkyLabels; - const ringLabels = isGalactic || !this.display().grid ? [] : this.ringLabels(camera); + const ringLabels = isGalactic || !this.display().grid ? [] : this.ringLabelsInTheClear(this.ringLabels(camera), camera, starLabels); this.labelOverlay?.update([...starLabels, ...ringLabels, ...backdropLabels]); } diff --git a/src/app/shared/format/scale-bar.ts b/src/app/shared/format/scale-bar.ts index 7eb8f88..ff10fbd 100644 --- a/src/app/shared/format/scale-bar.ts +++ b/src/app/shared/format/scale-bar.ts @@ -17,9 +17,10 @@ export function roundLengthAtMost(value: number): number | null { /** * Rings at a round step of about `reach / count`, out to `reach` or just past it, plus `callout` - * wherever it falls among them: the grid's own radii are round, and the one radius that means - * something in its own right is marked whether the step lands on it or not. Rounding the step - * down makes for `count` to `2.5 × count` rings, never fewer than it takes to cover `reach`. + * where it falls between the first ring and the last: the grid's own radii are round, and the one + * radius that means something in its own right is marked whether the step lands on it or not. A + * frame that does not reach it has no ring for it. Rounding the step down makes for `count` to + * `ceil(2.5 × count)` rings, and the callout can add one: 5 to 14 for a count of 5. */ export function distanceRings(reach: number, count: number, callout: number): number[] { const step = roundLengthAtMost(reach / count); From 7fba48c80834550486dc919faf41c64ad220c6a7 Mon Sep 17 00:00:00 2001 From: Senrokai Date: Fri, 18 Sep 2026 15:01:05 +0200 Subject: [PATCH 2/3] Answer the review: size the rings to the band the frame covers, and clear the text MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rings are centred on the Sun and the frame need not be. Sizing their step from how far the frame reaches — 210 pc for a star at 190 with the camera 20 pc back — gives 20 pc rings at 180 and 200, both outside a frame 19 pc deep, so a view away from the Sun still had no ring on it and no ladder of labels either. `distanceRings` now takes the span the frame covers rather than its far edge, and the step is a fifth of that: 5 pc rings from 165 to 210 for the same view. Measured in the app, centred on a star 187 pc out in the galactic plane: 4 ring labels drawn 20 pc above the plane and 7 from 2 pc, against 1 and none before. The clearance was a radius around the anchor, and a label is a line of text hanging 135 px to one side of its anchor: at 0.065 NDC apart, past the radius, "50 pc" printed inside "Alpha Centauri". It is now tested against the span the name occupies, on the side it hangs, with the radius kept for the pair whose text runs the other way. Also from the review: the ladder in the clearance test was built at exactly the constant it tests, so 1057 of 2000 camera poses would have decided it by float round-trip error — the rungs now sit 0.02 either side of the rule. And two comments that were wrong: a frame one step short of the survey edge does get its callout, and CSS2DRenderer hides a label behind the camera rather than drawing it at the page edge. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi --- .../galaxy-system-scene.component.spec.ts | 29 ++++++++++++++- .../galaxy-system-scene.component.ts | 37 +++++++++++++++---- src/app/shared/format/scale-bar.spec.ts | 27 ++++++++++---- src/app/shared/format/scale-bar.ts | 29 ++++++++++----- 4 files changed, 97 insertions(+), 25 deletions(-) diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts index 70d5d66..b79dc1f 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts @@ -441,6 +441,10 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { await advanceFrames(engine, 0.3); expect(underPerspective.at(-1)).toBeGreaterThanOrEqual(200); + // And one of them has to cross the frame, which is a band about 19 pc either side of 200 pc: + // rings out to 220 at a step sized to all 220 are 180 and 200, both of them off screen. + const halfHeight = engine.visibleHalfHeight(20); + expect(underPerspective.some((radius) => Math.abs(radius - 200) < halfHeight)).toBe(true); // The plan view's wheel moves the frame rather than the camera, so "how far out the camera // is" means something else there; what the rings have to cover does not. expect([...component.localGridRadii]).toEqual(underPerspective); @@ -470,8 +474,10 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { const camera = engine.getCamera(); camera.updateMatrixWorld(true); const at = (x: number, y: number) => new THREE.Vector3(x, y, 0.5).unproject(camera); + // Rungs at a twentieth of the screen: well inside the separation two names would keep, and + // well outside the clearance a ring label keeps from a name, so neither test is a coin toss. const near = at(0.1, 0.1); - const nextRungUp = at(0.1, 0.16); + const nextRungUp = at(0.1, 0.18); const offScreen = at(1.6, 0.1); const ladder: LabeledPoint[] = [ { id: 'ring-50', name: '50 pc', x: near.x, y: near.y, z: near.z }, @@ -486,6 +492,27 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { expect(component.ringLabelsInTheClear(ladder, camera, [star]).map((label) => label.id)).toEqual(['ring-100']); }); + it('stays out of the text of a name, not just off its point', () => { + const component = fixture.componentInstance as unknown as { + ringLabelsInTheClear(candidates: readonly LabeledPoint[], camera: THREE.Camera, stars: readonly LabeledPoint[]): LabeledPoint[]; + viewportAspect(): number; + }; + const camera = engine.getCamera(); + camera.updateMatrixWorld(true); + const aspect = component.viewportAspect(); + const at = (x: number, y: number) => new THREE.Vector3(x / aspect, y, 0.5).unproject(camera); + // A hand's breadth apart on screen — past any clearance around the point — and on the same + // line, with the name's text running right through where the ring label starts. + const ring = at(0.125, -0.123); + const rung: LabeledPoint = { id: 'ring-50', name: '50 pc', x: ring.x, y: ring.y, z: ring.z }; + const beside = at(0.06, -0.12); + const rightHand: LabeledPoint = { id: 7, name: 'Alpha Centauri', side: 'right', x: beside.x, y: beside.y, z: beside.z }; + + expect(component.ringLabelsInTheClear([rung], camera, [rightHand])).toEqual([]); + // The same name hanging the other way leaves that space empty, and the rung with it. + expect(component.ringLabelsInTheClear([rung], camera, [{ ...rightHand, side: 'left' }])).toEqual([rung]); + }); + it('places the ring labels with the star names rather than over them', async () => { const component = fixture.componentInstance as unknown as GridScene; const update = vi.spyOn(StarLabelOverlay.prototype, 'update'); diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.ts index 2250ae1..5cf5050 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.ts @@ -72,8 +72,9 @@ const LABEL_MIN_SEPARATION_NDC = 0.12; const LABEL_EDGE_NDC = 0.7; /** * How far a ring label has to sit from a star's name, in NDC — half what two star names keep - * between them. A ring label is one short line, and the rungs of its ladder are a twentieth of the - * screen apart, so the full separation would have one name clear three rungs. + * between them, as a clearance around the anchor and as the height of the row its text occupies. + * A ring label is one short line, and the rungs of its ladder are a twentieth of the screen apart, + * so the full separation would have one name clear three rungs. */ const RING_LABEL_CLEARANCE_NDC = LABEL_MIN_SEPARATION_NDC / 2; /** How far right of its point a label's text reaches, in aspect-scaled NDC (~135px at 1440). */ @@ -621,7 +622,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { centre: new THREE.Vector3(centre.x, centre.y, centre.z), emphasisRadii: [SUN_GALACTOCENTRIC_RADIUS_PC] }); - this.setLocalGridRadii(distanceRings(GALAXY_OVERVIEW_POSITION.length(), LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC)); + this.setLocalGridRadii(distanceRings(0, GALAXY_OVERVIEW_POSITION.length(), LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC)); // A fixed set rather than whatever is currently labelled: a tether that appears and vanishes // as the camera drifts reads as a glitch. this.tethers = new TetherField(TETHERED_STAR_COUNT); @@ -925,7 +926,13 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { * Not held apart from each other, as the star names are: they are a ladder up one ray, a few * hundredths of the screen apart, and reading them in order is the point. What they must not do * is sit on a star's name, which is worth more than a distance — or be handed to the overlay - * from behind or beside the camera, which draws them at the edge of the page rather than not at all. + * from beside the camera, which CSS2DRenderer places past the edge of the container rather than + * hiding, since all it tests is depth. + * + * A name is a line of text hanging to one side of its point, about 135 px of it, not the point: + * two anchors a tenth of the screen apart still print one inside the other. So the test is + * against the span the name occupies, with the anchors' own clearance kept for the pair whose + * text runs the other way. */ private ringLabelsInTheClear(candidates: readonly LabeledPoint[], camera: SceneCamera, stars: readonly LabeledPoint[]): LabeledPoint[] { const projected = new THREE.Vector3(); @@ -934,10 +941,21 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { const outside = projected.z < -1 || projected.z > 1 || Math.abs(projected.x) > 1 || Math.abs(projected.y) > 1; return outside ? null : new THREE.Vector2(projected.x * this.viewportAspect(), projected.y); }; - const taken = stars.map(onScreen).filter((point): point is THREE.Vector2 => point !== null); + const taken = stars + .map((star) => ({ at: onScreen(star), side: star.side })) + .filter((name): name is { at: THREE.Vector2; side: LabelSide | undefined } => name.at !== null) + .map(({ at, side }) => ({ at, from: side === 'left' ? at.x - LABEL_REACH_NDC : at.x, to: side === 'left' ? at.x : at.x + LABEL_REACH_NDC })); return candidates.filter((label) => { const point = onScreen(label); - return point !== null && !taken.some((other) => other.distanceTo(point) < RING_LABEL_CLEARANCE_NDC); + // Ring labels hang right, as `applyPresentation` leaves anything with no side of its own. + return ( + point !== null && + !taken.some( + (name) => + name.at.distanceTo(point) < RING_LABEL_CLEARANCE_NDC || + (Math.abs(name.at.y - point.y) < RING_LABEL_CLEARANCE_NDC && name.from < point.x + LABEL_REACH_NDC && point.x < name.to) + ) + ); }); } @@ -962,7 +980,12 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { // vertex of the new ones, and the set changes on any zoom that crosses a round step. if (!isGalactic && this.display().grid) { const orbitPc = this.engine.currentProjection === 'perspective' ? camera.position.distanceTo(target) : this.effectiveDistance(camera); - const radii = distanceRings(target.length() + orbitPc, LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC); + // Half the frame's diagonal, at the depth it is centred on: how near the Sun the frame + // reaches, as well as how far. A step sized to the far edge alone is no use to a frame that + // does not contain the Sun — 20 pc rings for a view of a 19 pc band at 190 pc drew none of + // them on screen, and the ladder of labels went with them. + const frameRadiusPc = this.engine.visibleHalfHeight(orbitPc) * Math.hypot(1, this.viewportAspect()); + const radii = distanceRings(Math.max(0, target.length() - frameRadiusPc), target.length() + orbitPc, LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC); if (radii.join() !== this.localGridRadii.join()) { this.setLocalGridRadii(radii); } diff --git a/src/app/shared/format/scale-bar.spec.ts b/src/app/shared/format/scale-bar.spec.ts index f673386..9fda567 100644 --- a/src/app/shared/format/scale-bar.spec.ts +++ b/src/app/shared/format/scale-bar.spec.ts @@ -28,30 +28,41 @@ describe('distanceRings', () => { // The opening view sits about 307 pc from the Sun: the rings the map always had, with the // survey edge at the fifth, and on out past the camera for the stars now drawn beyond it. it('reaches past the camera from the opening view', () => { - expect(distanceRings(307, 5, 250)).toEqual([50, 100, 150, 200, 250, 300, 350]); + expect(distanceRings(0, 307, 5, 250)).toEqual([50, 100, 150, 200, 250, 300, 350]); }); it('closes in with the camera', () => { - expect(distanceRings(20, 5, 250)).toEqual([2, 4, 6, 8, 10, 12, 14, 16, 18, 20]); - expect(distanceRings(1, 5, 250)).toEqual([0.2, 0.4, 0.6, 0.8, 1]); + expect(distanceRings(0, 20, 5, 250)).toEqual([2, 4, 6, 8, 10, 12, 14, 16, 18, 20]); + expect(distanceRings(0, 1, 5, 250)).toEqual([0.2, 0.4, 0.6, 0.8, 1]); }); // Near Mirfak the camera is 155 pc out; rounding the step down to 20 pc must not leave the // rings stopping at 100. it('covers the whole distance whatever the rounding', () => { - expect(distanceRings(155, 5, 250)).toEqual([20, 40, 60, 80, 100, 120, 140, 160]); + expect(distanceRings(0, 155, 5, 250)).toEqual([20, 40, 60, 80, 100, 120, 140, 160]); + }); + + // A star 190 pc out seen from 20 pc away: the frame is a band about 19 pc either side of it and + // the Sun is nowhere in it. Sized to the 210 pc it reaches, the step would be 20 pc and the + // nearest rings — 180 and 200 — would both miss the frame. + it('spaces the rings for a frame that does not hold the Sun', () => { + const radii = distanceRings(171, 210, 5, 250); + + expect(radii).toEqual([170, 175, 180, 185, 190, 195, 200, 205, 210]); + expect(radii.some((radius) => Math.abs(radius - 190) < 19)).toBe(true); }); it('marks the callout among rings the step does not land on', () => { - expect(distanceRings(1000, 5, 250)).toEqual([200, 250, 400, 600, 800, 1000]); + expect(distanceRings(0, 1000, 5, 250)).toEqual([200, 250, 400, 600, 800, 1000]); }); - it('leaves the callout out when it is past the last ring', () => { - expect(distanceRings(100, 5, 250)).toEqual([20, 40, 60, 80, 100]); + it('leaves the callout out when it is past the last ring, or behind the first', () => { + expect(distanceRings(0, 100, 5, 250)).toEqual([20, 40, 60, 80, 100]); + expect(distanceRings(400, 440, 5, 250)).toEqual([400, 405, 410, 415, 420, 425, 430, 435, 440]); }); it('draws no rings for a camera with no distance', () => { - expect(distanceRings(0, 5, 250)).toEqual([]); + expect(distanceRings(0, 0, 5, 250)).toEqual([]); }); }); diff --git a/src/app/shared/format/scale-bar.ts b/src/app/shared/format/scale-bar.ts index ff10fbd..f5b1e54 100644 --- a/src/app/shared/format/scale-bar.ts +++ b/src/app/shared/format/scale-bar.ts @@ -16,20 +16,31 @@ export function roundLengthAtMost(value: number): number | null { } /** - * Rings at a round step of about `reach / count`, out to `reach` or just past it, plus `callout` - * where it falls between the first ring and the last: the grid's own radii are round, and the one - * radius that means something in its own right is marked whether the step lands on it or not. A - * frame that does not reach it has no ring for it. Rounding the step down makes for `count` to - * `ceil(2.5 × count)` rings, and the callout can add one: 5 to 14 for a count of 5. + * Rings across the span from `nearest` to `reach`, at a round step of about a `count`th of it, + * plus `callout` where it falls between the first ring and the last: the grid's own radii are + * round, and the one radius that means something in its own right is marked whether the step lands + * on it or not. A frame short of it by less than one step still gets it, since the last ring + * overshoots `reach`; one that stops well short does not. + * + * Two numbers rather than one because these rings are centred on a fixed point — the Sun — and a + * frame need not be. Looking at something 200 pc out from 20 pc away, what is on screen is a band + * 200 pc wide at its narrowest and nowhere near the Sun; a step sized to the whole 220 puts every + * ring off the frame. The span is what the frame covers, so the step is what it can resolve. + * + * Rounding the step down makes for `count` to `ceil(2.5 × count)` rings, and the callout can add + * one: 5 to 14 for a count of 5. */ -export function distanceRings(reach: number, count: number, callout: number): number[] { - const step = roundLengthAtMost(reach / count); +export function distanceRings(nearest: number, reach: number, count: number, callout: number): number[] { + const step = roundLengthAtMost((reach - nearest) / count); if (step === null) { return []; } + // The ring just inside the near edge of the span, so the band is crossed rather than started at. + const first = Math.max(1, Math.floor(nearest / step)); + const last = Math.ceil(reach / step); // `toPrecision` clears the binary noise of stepping by a tenth: 0.1 × 3 is 0.30000000000000004. - const radii = Array.from({ length: Math.ceil(reach / step) }, (_, index) => Number((step * (index + 1)).toPrecision(12))); - if (callout > step && callout < radii[radii.length - 1] && !radii.includes(callout)) { + const radii = Array.from({ length: last - first + 1 }, (_, index) => Number((step * (first + index)).toPrecision(12))); + if (callout > radii[0] && callout < radii[radii.length - 1] && !radii.includes(callout)) { radii.push(callout); radii.sort((a, b) => a - b); } From 0c5efec5050e86561d6de411292d95edca2d22ff Mon Sep 17 00:00:00 2001 From: Senrokai Date: Fri, 18 Sep 2026 15:57:44 +0200 Subject: [PATCH 3/3] Measure the ring span along the plane the rings lie in MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The span went to `distanceRings` as the target's straight-line distance from the Sun, but a ring of radius r passes within |r - p| of the view's centre, where p is how far out that centre is *along* the galactic plane. For a target above the plane the two differ by its height, so the band was centred on a radius no ring has — and `ringLabels` picks its bearing by comparing its own in-plane distance against the innermost ring, a comparison the new first ring quietly broke. Two comments and a constant, from the same review. A frame short of the survey edge gets its callout only when its last ring overshoots it: 245 pc does, 235 pc does not, which is now a test rather than a sentence. The ring count can reach 16, not 14, now that the span need not start at the Sun. And a ring label was measured as 135 px of star name when "50 pc" is a third of that, which rejected rungs a hand's breadth clear of the name: RING_LABEL_REACH_NDC, 0.23, is the widest of them — "1.5 kpc" with "Survey edge" under it. Three mutants, three caught. Measured again in the app: unchanged for a star in the plane (4 labels at 20 pc above it, 7 at 2 pc), and the rings now follow the plane for one 195 pc above it rather than ringing a place the grid does not reach. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi --- .../galaxy-system-scene.component.spec.ts | 39 ++++++++++++++++--- .../galaxy-system-scene.component.ts | 16 +++++++- src/app/shared/format/scale-bar.spec.ts | 4 ++ src/app/shared/format/scale-bar.ts | 10 +++-- 4 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts index b79dc1f..9966290 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.spec.ts @@ -13,6 +13,7 @@ import { NavigationStore } from '../../shared/state/navigation.store'; import { LinkBudget } from '../../shared/astro/jump-links'; import { HudDisplay } from '../hud/hud-dock.component'; import { GalaxySystemSceneComponent } from './galaxy-system-scene.component'; +import { galacticNormal } from './grid-plane'; import { JumpLinkRenderer } from './jump-link-renderer'; import { StarFieldRenderer } from './star-field-renderer'; import { LabeledPoint, StarLabelOverlay } from './star-label-overlay'; @@ -429,9 +430,12 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { it('sizes the rings by how far the frame reaches from the Sun, under either projection', async () => { const component = fixture.componentInstance as unknown as GridScene; const camera = engine.getCamera(); - // Centred on a star 200 pc out, seen from 20 pc away: the rings have to reach it. - component.controls.target.set(200, 0, 0); - camera.position.set(200, 0, 20); + // Centred on a point 200 pc out along the galactic plane — where the rings are — seen from + // 20 pc above it. The rings have to reach it, and one of them has to cross the frame. + const normal = galacticNormal(); + const centre = new THREE.Vector3(1, 0, 0).projectOnPlane(normal).normalize().multiplyScalar(200); + component.controls.target.copy(centre); + camera.position.copy(centre).addScaledVector(normal, 20); component.controls.update(); await advanceFrames(engine, 0.3); const underPerspective = [...component.localGridRadii]; @@ -441,8 +445,8 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { await advanceFrames(engine, 0.3); expect(underPerspective.at(-1)).toBeGreaterThanOrEqual(200); - // And one of them has to cross the frame, which is a band about 19 pc either side of 200 pc: - // rings out to 220 at a step sized to all 220 are 180 and 200, both of them off screen. + // The frame is a band about 19 pc either side of 200 pc: rings out to 220 at a step sized to + // all 220 are 180 and 200, both of them off screen. const halfHeight = engine.visibleHalfHeight(20); expect(underPerspective.some((radius) => Math.abs(radius - 200) < halfHeight)).toBe(true); // The plan view's wheel moves the frame rather than the camera, so "how far out the camera @@ -450,6 +454,22 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { expect([...component.localGridRadii]).toEqual(underPerspective); }); + it('measures the span in the plane the rings lie in, not through it', async () => { + const component = fixture.componentInstance as unknown as GridScene; + const camera = engine.getCamera(); + // The same 200 pc out along the plane, but lifted 150 pc above it: 250 pc from the Sun as the + // crow flies, and still 200 pc out among the rings, which is the distance they are drawn at. + const normal = galacticNormal(); + const centre = new THREE.Vector3(1, 0, 0).projectOnPlane(normal).normalize().multiplyScalar(200).addScaledVector(normal, 150); + component.controls.target.copy(centre); + camera.position.copy(centre).addScaledVector(normal, 20); + component.controls.update(); + await advanceFrames(engine, 0.3); + + const halfHeight = engine.visibleHalfHeight(20); + expect([...component.localGridRadii].some((radius) => Math.abs(radius - 200) < halfHeight)).toBe(true); + }); + it('leaves the rings alone while the grid is not drawn', async () => { const component = fixture.componentInstance as unknown as GridScene; const camera = engine.getCamera(); @@ -511,6 +531,15 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => { expect(component.ringLabelsInTheClear([rung], camera, [rightHand])).toEqual([]); // The same name hanging the other way leaves that space empty, and the rung with it. expect(component.ringLabelsInTheClear([rung], camera, [{ ...rightHand, side: 'left' }])).toEqual([rung]); + + // And a rung to the left of a name keeps its place: "50 pc" is a third of a star name's + // width, so it ends well before the name starts, whatever the anchors' spacing suggests. + const centred = at(0, 0); + const spanning: LabeledPoint = { id: 8, name: 'Alnitak', side: 'right', x: centred.x, y: centred.y, z: centred.z }; + const toTheLeft = at(-0.25, 0.02); + const clearRung: LabeledPoint = { id: 'ring-100', name: '100 pc', x: toTheLeft.x, y: toTheLeft.y, z: toTheLeft.z }; + + expect(component.ringLabelsInTheClear([clearRung], camera, [spanning])).toEqual([clearRung]); }); it('places the ring labels with the star names rather than over them', async () => { diff --git a/src/app/features/galaxy-system/galaxy-system-scene.component.ts b/src/app/features/galaxy-system/galaxy-system-scene.component.ts index 5cf5050..15e7585 100644 --- a/src/app/features/galaxy-system/galaxy-system-scene.component.ts +++ b/src/app/features/galaxy-system/galaxy-system-scene.component.ts @@ -79,6 +79,12 @@ const LABEL_EDGE_NDC = 0.7; const RING_LABEL_CLEARANCE_NDC = LABEL_MIN_SEPARATION_NDC / 2; /** How far right of its point a label's text reaches, in aspect-scaled NDC (~135px at 1440). */ const LABEL_REACH_NDC = 0.3; +/** + * The same for a ring label, which is shorter: "1.5 kpc" with "Survey edge" under it is the widest + * of them, about 100px at 1440. Measuring those as a star name's width rejected rungs a hand's + * breadth clear of it. + */ +const RING_LABEL_REACH_NDC = 0.23; /** * How long the range control has to be still before the graph is rebuilt at its value, since a drag * emits per pixel; and how often at most a view on the move gets a graph for its new drawn stars. @@ -953,7 +959,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { !taken.some( (name) => name.at.distanceTo(point) < RING_LABEL_CLEARANCE_NDC || - (Math.abs(name.at.y - point.y) < RING_LABEL_CLEARANCE_NDC && name.from < point.x + LABEL_REACH_NDC && point.x < name.to) + (Math.abs(name.at.y - point.y) < RING_LABEL_CLEARANCE_NDC && name.from < point.x + RING_LABEL_REACH_NDC && point.x < name.to) ) ); }); @@ -985,7 +991,13 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy { // does not contain the Sun — 20 pc rings for a view of a 19 pc band at 190 pc drew none of // them on screen, and the ladder of labels went with them. const frameRadiusPc = this.engine.visibleHalfHeight(orbitPc) * Math.hypot(1, this.viewportAspect()); - const radii = distanceRings(Math.max(0, target.length() - frameRadiusPc), target.length() + orbitPc, LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC); + // Measured in the plane the rings lie in, not through it: a ring of radius r passes within + // `|r - p|` of the view's centre, where p is how far out the centre is *along the plane*. For + // a target above it the two differ by its height, which would put the band around a radius no + // ring has — and `ringLabels` compares its own in-plane bearing against the innermost. + const normal = galacticNormal(); + const inPlanePc = target.clone().addScaledVector(normal, -target.dot(normal)).length(); + const radii = distanceRings(Math.max(0, inPlanePc - frameRadiusPc), inPlanePc + orbitPc, LOCAL_GRID_RING_COUNT, SURVEY_EDGE_PC); if (radii.join() !== this.localGridRadii.join()) { this.setLocalGridRadii(radii); } diff --git a/src/app/shared/format/scale-bar.spec.ts b/src/app/shared/format/scale-bar.spec.ts index 9fda567..2913a6d 100644 --- a/src/app/shared/format/scale-bar.spec.ts +++ b/src/app/shared/format/scale-bar.spec.ts @@ -58,6 +58,10 @@ describe('distanceRings', () => { it('leaves the callout out when it is past the last ring, or behind the first', () => { expect(distanceRings(0, 100, 5, 250)).toEqual([20, 40, 60, 80, 100]); + // Short of the survey edge by less than one step is not the rule — the last ring is: 245 pc + // overshoots to 260 and gets it, 235 pc stops at 240 and does not, on the same 20 pc step. + expect(distanceRings(0, 245, 5, 250)).toContain(250); + expect(distanceRings(0, 235, 5, 250)).not.toContain(250); expect(distanceRings(400, 440, 5, 250)).toEqual([400, 405, 410, 415, 420, 425, 430, 435, 440]); }); diff --git a/src/app/shared/format/scale-bar.ts b/src/app/shared/format/scale-bar.ts index f5b1e54..96bb777 100644 --- a/src/app/shared/format/scale-bar.ts +++ b/src/app/shared/format/scale-bar.ts @@ -19,16 +19,18 @@ export function roundLengthAtMost(value: number): number | null { * Rings across the span from `nearest` to `reach`, at a round step of about a `count`th of it, * plus `callout` where it falls between the first ring and the last: the grid's own radii are * round, and the one radius that means something in its own right is marked whether the step lands - * on it or not. A frame short of it by less than one step still gets it, since the last ring - * overshoots `reach`; one that stops well short does not. + * on it or not. A frame that stops short of it gets it only when the last ring — the first multiple + * of `step` at or past `reach` — is past it: reach 245 with a 20 pc step gets it, reach 235 does + * not, since its last ring is 240. * * Two numbers rather than one because these rings are centred on a fixed point — the Sun — and a * frame need not be. Looking at something 200 pc out from 20 pc away, what is on screen is a band * 200 pc wide at its narrowest and nowhere near the Sun; a step sized to the whole 220 puts every * ring off the frame. The span is what the frame covers, so the step is what it can resolve. * - * Rounding the step down makes for `count` to `ceil(2.5 × count)` rings, and the callout can add - * one: 5 to 14 for a count of 5. + * Rounding the step down, over a span that need not start at the Sun, makes for `count` to + * `ceil(2.5 × count) + 2` rings — `ceil(reach / step) - floor(nearest / step) + 1` — and the + * callout can add one: 5 to 16 for a count of 5. */ export function distanceRings(nearest: number, reach: number, count: number, callout: number): number[] { const step = roundLengthAtMost((reach - nearest) / count);