Measure the ring span along the plane the rings lie in

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi
This commit is contained in:
2026-09-18 15:57:44 +02:00
co-authored by Claude Opus 5
parent 7fba48c808
commit 0c5efec505
4 changed files with 58 additions and 11 deletions
@@ -13,6 +13,7 @@ import { NavigationStore } from '../../shared/state/navigation.store';
import { LinkBudget } from '../../shared/astro/jump-links'; import { LinkBudget } from '../../shared/astro/jump-links';
import { HudDisplay } from '../hud/hud-dock.component'; import { HudDisplay } from '../hud/hud-dock.component';
import { GalaxySystemSceneComponent } from './galaxy-system-scene.component'; import { GalaxySystemSceneComponent } from './galaxy-system-scene.component';
import { galacticNormal } from './grid-plane';
import { JumpLinkRenderer } from './jump-link-renderer'; import { JumpLinkRenderer } from './jump-link-renderer';
import { StarFieldRenderer } from './star-field-renderer'; import { StarFieldRenderer } from './star-field-renderer';
import { LabeledPoint, StarLabelOverlay } from './star-label-overlay'; 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 () => { 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 component = fixture.componentInstance as unknown as GridScene;
const camera = engine.getCamera(); const camera = engine.getCamera();
// Centred on a star 200 pc out, seen from 20 pc away: the rings have to reach it. // Centred on a point 200 pc out along the galactic plane — where the rings are — seen from
component.controls.target.set(200, 0, 0); // 20 pc above it. The rings have to reach it, and one of them has to cross the frame.
camera.position.set(200, 0, 20); 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(); component.controls.update();
await advanceFrames(engine, 0.3); await advanceFrames(engine, 0.3);
const underPerspective = [...component.localGridRadii]; const underPerspective = [...component.localGridRadii];
@@ -441,8 +445,8 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => {
await advanceFrames(engine, 0.3); await advanceFrames(engine, 0.3);
expect(underPerspective.at(-1)).toBeGreaterThanOrEqual(200); 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: // The frame is a band about 19 pc either side of 200 pc: rings out to 220 at a step sized to
// rings out to 220 at a step sized to all 220 are 180 and 200, both of them off screen. // all 220 are 180 and 200, both of them off screen.
const halfHeight = engine.visibleHalfHeight(20); const halfHeight = engine.visibleHalfHeight(20);
expect(underPerspective.some((radius) => Math.abs(radius - 200) < halfHeight)).toBe(true); 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 // 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); 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 () => { it('leaves the rings alone while the grid is not drawn', async () => {
const component = fixture.componentInstance as unknown as GridScene; const component = fixture.componentInstance as unknown as GridScene;
const camera = engine.getCamera(); const camera = engine.getCamera();
@@ -511,6 +531,15 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => {
expect(component.ringLabelsInTheClear([rung], camera, [rightHand])).toEqual([]); expect(component.ringLabelsInTheClear([rung], camera, [rightHand])).toEqual([]);
// The same name hanging the other way leaves that space empty, and the rung with it. // 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]); 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 () => { it('places the ring labels with the star names rather than over them', async () => {
@@ -79,6 +79,12 @@ const LABEL_EDGE_NDC = 0.7;
const RING_LABEL_CLEARANCE_NDC = LABEL_MIN_SEPARATION_NDC / 2; 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). */ /** How far right of its point a label's text reaches, in aspect-scaled NDC (~135px at 1440). */
const LABEL_REACH_NDC = 0.3; 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 * 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. * 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( !taken.some(
(name) => (name) =>
name.at.distanceTo(point) < RING_LABEL_CLEARANCE_NDC || 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 // 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. // them on screen, and the ladder of labels went with them.
const frameRadiusPc = this.engine.visibleHalfHeight(orbitPc) * Math.hypot(1, this.viewportAspect()); 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()) { if (radii.join() !== this.localGridRadii.join()) {
this.setLocalGridRadii(radii); this.setLocalGridRadii(radii);
} }
+4
View File
@@ -58,6 +58,10 @@ describe('distanceRings', () => {
it('leaves the callout out when it is past the last ring, or behind the first', () => { 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(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]); expect(distanceRings(400, 440, 5, 250)).toEqual([400, 405, 410, 415, 420, 425, 430, 435, 440]);
}); });
+6 -4
View File
@@ -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, * 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 * 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 * 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 * on it or not. A frame that stops short of it gets it only when the last ring — the first multiple
* overshoots `reach`; one that stops well short does not. * 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 * 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 * 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 * 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. * 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 * Rounding the step down, over a span that need not start at the Sun, makes for `count` to
* one: 5 to 14 for a count of 5. * `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[] { export function distanceRings(nearest: number, reach: number, count: number, callout: number): number[] {
const step = roundLengthAtMost((reach - nearest) / count); const step = roundLengthAtMost((reach - nearest) / count);