Answer the review: size the rings to the band the frame covers, and clear the text

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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016jxMkwA2rbicdGxHosecYi

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');

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);
       }

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([]);
   });
 });
 

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);
   }