Answer the review: budget in drawn pixels, re-ask when the budget moves, sort only the band it runs out in

- The budget counted CSS pixels; lines are drawn in device pixels, so a screen scaled to 150% or
  200% drew 1.5-2x the calibrated line. It now counts the canvas's drawn pixels.
- A graph was re-asked only when the drawn stars changed, so with a star budget covering the whole
  catalogue, or a resize, its budget and centre stayed wherever the layer was turned on. A view that
  chose its stars again now asks, and a graph is rebuilt when the stars, the range or the budget
  changed (the budget by more than half the margin, or its centre by more than 5 pc).
- From inside a system the budget was worked out in astronomical units about the system's origin.
  Graphs are now asked for in parsec space only; the flight back out asks.
- Comparing budgets let a request re-asked with a slightly different one supersede its twin, and the
  twin's rejection cleared the state of the request that replaced it. A rejection now clears it only
  for the latest request.
- The worker sorted every link to keep a few thousand, 2.2x an unbudgeted build. It now bands links
  by distance, keeps every band before the one the budget runs out in, and sorts only that one:
  142-168 ms on the real catalogue against 233-388 ms, 103 ms unbudgeted, returning early when all fit.

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-17 19:03:11 +02:00
co-authored by Claude Opus 5
parent bd5e9d4b0d
commit 539a0f0a3e
4 changed files with 219 additions and 35 deletions
@@ -133,6 +133,13 @@ class FakeEngineService {
resize(): void {}
/** The canvas's device pixels per CSS pixel, as the renderer was told. */
pixelRatio = 1;
getRenderer(): { getPixelRatio(): number } {
return { getPixelRatio: () => this.pixelRatio };
}
/** Test helper: simulates one rendered frame by invoking every registered tick callback. */
tick(deltaSeconds: number): void {
for (const callback of this.tickCallbacks) {
@@ -475,14 +482,71 @@ describe('GalaxySystemSceneComponent camera-flight transitions', () => {
it('asks for as much of the graph as a million pixels of line make, around where the view is centred', async () => {
const links = vi.fn((_rangePc: number, _drawn: Uint32Array, _budget?: LinkBudget) => Promise.resolve(new Float32Array(0)));
Object.defineProperty((fixture.nativeElement as HTMLElement).querySelector('canvas')!, 'clientHeight', { value: 1080 });
// A screen scaled to 200%: 1080 CSS pixels are 2160 drawn ones, and the lines are drawn in those.
engine.pixelRatio = 2;
linkScene(links);
await settle();
const budget = links.mock.calls[0][2];
// The view opens centred on the Sun: its frame's half-height there, over 540 pixels, is a pixel's worth of parsecs.
// The view opens centred on the Sun: its frame's half-height there, over 1080 drawn pixels, is a pixel's worth of parsecs.
const halfHeight = engine.getCamera().position.length() * Math.tan((50 * Math.PI) / 360);
expect(budget?.centre).toEqual({ x: 0, y: 0, z: 0 });
expect(budget?.lengthPc).toBeCloseTo((1_000_000 * halfHeight) / 540, 3);
expect(budget?.lengthPc).toBeCloseTo((1_000_000 * halfHeight) / 1080, 3);
});
it('asks again once the view has zoomed past the budget it asked with, though the drawn stars are the same', async () => {
// All three stars fit the star budget, so the drawn set never changes: only the budget can.
const links = vi.fn((_rangePc: number, _drawn: Uint32Array, _budget?: LinkBudget) => Promise.resolve(new Float32Array(0)));
Object.defineProperty((fixture.nativeElement as HTMLElement).querySelector('canvas')!, 'clientHeight', { value: 1080 });
const component = linkScene(links);
await advanceFrames(engine, 0.3);
await settle();
const asked = links.mock.calls.length;
const camera = engine.getCamera();
camera.position.sub(component.controls.target).multiplyScalar(0.5).add(component.controls.target);
await advanceFrames(engine, 0.3);
await settle();
expect(links.mock.calls.length).toBe(asked + 1);
expect(links.mock.calls.at(-1)![1]).toBe(links.mock.calls[0][1]);
});
it('asks for no graph from inside a system, where distances are in astronomical units', async () => {
const links = vi.fn((_rangePc: number, _drawn: Uint32Array, _budget?: LinkBudget) => Promise.resolve(new Float32Array(0)));
navigationStore.selectStar(SUN.id);
await flushAsync();
await advanceFrames(engine, 2.5);
linkScene(links);
await settle();
expect(links).not.toHaveBeenCalled();
});
it('keeps what it asked for when an older request it replaced is rejected', async () => {
// Off and on again while a graph is still waiting: the waiting one is replaced, and its
// rejection must not be taken for the request that replaced it.
const pending: Array<{ resolve: (segments: Float32Array) => void; reject: (error: Error) => void }> = [];
const setSegments = vi.spyOn(JumpLinkRenderer.prototype, 'setSegments');
const component = linkScene(() => new Promise<Float32Array>((resolve, reject) => pending.push({ resolve, reject })));
await settle();
component.display.update((display) => ({ ...display, jumpLinks: false }));
TestBed.tick();
await settle();
component.display.update((display) => ({ ...display, jumpLinks: true }));
TestBed.tick();
await settle();
expect(pending).toHaveLength(2);
pending[0].reject(new Error('Superseded by a newer request'));
await flushAsync();
const graph = new Float32Array(6);
pending[1].resolve(graph);
await flushAsync();
expect(setSegments).toHaveBeenLastCalledWith(graph);
setSegments.mockRestore();
});
it('gives a view on the move a new graph at least every quarter second, rather than waiting for it to stop', async () => {
@@ -77,6 +77,19 @@ const LABEL_REACH_NDC = 0.3;
* emits per pixel; and how often at most a view on the move gets a graph for its new drawn stars.
*/
const JUMP_LINK_REBUILD_DELAY_MS = 250;
/**
* Whether a graph asked for with one budget still serves another: the same, unless the view has
* zoomed by more than half its margin or its centre has moved by more than a fifth of the
* neighbourhood drawn whole.
*/
function servesTheSame(asked: LinkBudget | undefined, now: LinkBudget | undefined): boolean {
if (!asked || !now) {
return asked === now;
}
const moved = Math.hypot(now.centre.x - asked.centre.x, now.centre.y - asked.centre.y, now.centre.z - asked.centre.z);
return Math.abs(now.lengthPc / asked.lengthPc - 1) <= VIEW_MARGIN / 2 && moved <= STAR_FIELD_REFOCUS_PC;
}
/**
* How much jump-link line the layer draws, in pixels of length on screen: about a million, measured
* where lines are longest.
@@ -388,6 +401,9 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
/** The range and the stars the drawn graph was last asked for, so a rebuild is skipped when neither moved. */
private drawnJumpRangePc: number | null = null;
private linkedStars: Uint32Array | null = null;
private linkedBudget: LinkBudget | undefined;
/** Counts graph requests, so a rejection can tell whether it is for the latest one. */
private linkRequest = 0;
private jumpLinkRebuild?: ReturnType<typeof setTimeout>;
/** The current system's neighbours, resolved on arrival: id, name, distance and bearing. */
private neighbours: readonly { star: StarRecord; distancePc: number; direction: THREE.Vector3 }[] = [];
@@ -840,7 +856,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
const pinned = () => pinnedIds.map((id) => neighbourhood.indexOf(id)).filter((index): index is number => index !== undefined);
const centre = this.controls?.target ?? GALAXY_OVERVIEW_TARGET;
const drawnBefore = this.starField.drawnStars;
let chose = false;
// At galactic scale the whole catalogue is a smudge a few pixels across, and the view sweeps
// hundreds of parsecs a pass: chosen once for the whole sky on the way out, then left alone,
@@ -850,6 +866,7 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
this.starField.refocus({ centre, pinned: pinned(), hosts: this.hostStars });
this.starFieldCamera = null;
this.starFieldPins = pins;
chose = true;
}
} else {
const halfHeight = this.engine.visibleHalfHeight(camera.position.distanceTo(centre));
@@ -882,13 +899,15 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
this.starFieldFocus.copy(centre);
this.starFieldHalfHeight = halfHeight;
this.starFieldPins = pins;
chose = true;
}
}
// The graph links the drawn stars, so a new set wants a new graph. Not one per pass while the
// view keeps moving, and not one pushed back by every pass either, or an orbit would never get
// one: at most one every `JUMP_LINK_REBUILD_DELAY_MS`.
if (this.starField.drawnStars !== drawnBefore && this.jumpLinkRebuild === undefined) {
// The graph links the drawn stars, and spends its budget around the view's centre, so a view that
// has moved may want a new one; `refreshJumpLinks` asks only if the stars or the budget changed.
// Not one per pass while the view keeps moving, and not one pushed back by every pass either, or
// an orbit would never get one: at most one every `JUMP_LINK_REBUILD_DELAY_MS`.
if (chose && this.jumpLinkRebuild === undefined) {
this.scheduleJumpLinks();
}
}
@@ -1561,27 +1580,39 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
this.jumpLinks.setSegments(new Float32Array(0));
this.drawnJumpRangePc = null;
this.linkedStars = null;
this.linkedBudget = undefined;
}
return;
}
// Asked for in parsec space only: inside a system the camera and its centre are in astronomical
// units about the system's own origin, which would make a budget of the wrong size in the wrong
// place. The flight back out chooses the drawn stars again, and that asks.
if (!this.galaxyGroup.visible) {
return;
}
const drawn = this.starField.drawnStars;
if (this.drawnJumpRangePc === rangePc && this.linkedStars === drawn) {
const budget = this.jumpLinkBudget();
if (this.drawnJumpRangePc === rangePc && this.linkedStars === drawn && servesTheSame(this.linkedBudget, budget)) {
return;
}
this.drawnJumpRangePc = rangePc;
this.linkedStars = drawn;
void this.routing.links(rangePc, drawn, this.jumpLinkBudget()).then(
this.linkedBudget = budget;
const request = ++this.linkRequest;
void this.routing.links(rangePc, drawn, budget).then(
(segments) => {
if (this.drawnJumpRangePc === rangePc) {
this.jumpLinks?.setSegments(segments);
}
},
() => {
// Replaced by a newer request, or failed. Either way this graph is not drawn, and must not
// be remembered as if it were, or asking for it again would be skipped.
if (this.drawnJumpRangePc === rangePc && this.linkedStars === drawn) {
// Replaced by a newer request, or failed. Only the latest request's rejection means no graph
// is on its way; then nothing is remembered as drawn, so asking again is not skipped. An older
// one's says nothing about the request that replaced it, which may ask the same thing.
if (request === this.linkRequest) {
this.drawnJumpRangePc = null;
this.linkedStars = null;
this.linkedBudget = undefined;
}
}
);
@@ -1592,7 +1623,8 @@ export class GalaxySystemSceneComponent implements AfterViewInit, OnDestroy {
* `JUMP_LINK_PIXEL_BUDGET` pixels of line make at that depth. None without a canvas to measure.
*/
private jumpLinkBudget(): LinkBudget | undefined {
const heightPx = this.canvasRef().nativeElement.clientHeight;
// In the pixels the lines are drawn in, not in CSS pixels: a scaled or HiDPI screen draws more of them.
const heightPx = this.canvasRef().nativeElement.clientHeight * this.engine.getRenderer().getPixelRatio();
if (heightPx === 0) {
return undefined;
}