Skip to content

Commit dfee09f

Browse files
committed
Fix snap picking coordinate scaling
Use the renderer's effective resolution scale when mapping CSS cursor coordinates into WebGL pick/snap buffers and when projecting snapped positions back to canvas coordinates.
1 parent f3dadd9 commit dfee09f

7 files changed

Lines changed: 116 additions & 11 deletions

File tree

packages/sdk/src/viewing/webGLRenderer/internal/ViewManager.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {SceneTransform} from "../../../model/scene/SceneTransform";
1414
import {type MemoryConfigs} from "../MemoryConfigs";
1515
import type {DataTextures} from "./gpuMemoryManager/DataTextures";
1616
import {RenderInspector, ShaderInspector} from "./inspectors";
17+
import {getEffectiveResolutionScale} from "./resolutionScale";
1718

1819
/**
1920
* Top-level, internal rendering and pipeline manager within a {@link WebGLRenderer}.
@@ -764,9 +765,7 @@ export class ViewManager {
764765
const top = Math.round(rect.top);
765766

766767
// Resolution scaling affects only the backing buffer size.
767-
const resolutionScale = view.resolutionScale.applied
768-
? Math.max(0.05, view.resolutionScale.resolutionScale)
769-
: 1.0;
768+
const resolutionScale = getEffectiveResolutionScale(view);
770769

771770
const pixelWidth = Math.max(1, Math.round(cssWidth * resolutionScale));
772771
const pixelHeight = Math.max(1, Math.round(cssHeight * resolutionScale));

packages/sdk/src/viewing/webGLRenderer/internal/pickManager/PickManager.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {SceneMesh} from "../../../../model/scene";
2020
import {RENDER_PASSES} from "../RENDER_PASSES";
2121
import {createRTCViewMat} from "../../../../base/math/rtc";
2222
import {GaussianSplatPickTechnique, SPLAT_PICK_SENTINEL} from "../drawOps/techniques/splats/GaussianSplatPickTechnique";
23+
import {getEffectiveResolutionScale} from "../resolutionScale";
2324

2425
const tempVec3a = createVec3Float64();
2526
const tempVec3b = createVec3Float64();
@@ -289,7 +290,7 @@ export class PickManager {
289290

290291
const view = rendererView.view;
291292
const viewIndex = view.viewIndex;
292-
const resolutionScale = view.resolutionScale;
293+
const effectiveResolutionScale = getEffectiveResolutionScale(view);
293294
const renderContext = this._renderContext;
294295
const gl = renderContext.gl;
295296
const pickBuffer = this._pickBuffer;
@@ -311,8 +312,8 @@ export class PickManager {
311312
renderContext.pickProjMatrix = pickProjMatrix;
312313
renderContext.pickInvisible = !!pickInvisible;
313314
renderContext.pickClipPos = [
314-
this._getClipPosX(pickCanvasPos[0] * resolutionScale.resolutionScale, gl.drawingBufferWidth),
315-
this._getClipPosY(pickCanvasPos[1] * resolutionScale.resolutionScale, gl.drawingBufferHeight)
315+
this._getClipPosX(pickCanvasPos[0] * effectiveResolutionScale, gl.drawingBufferWidth),
316+
this._getClipPosY(pickCanvasPos[1] * effectiveResolutionScale, gl.drawingBufferHeight)
316317
];
317318

318319
gl.viewport(0, 0, 1, 1);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import type {View} from "../../viewer";
2+
3+
/**
4+
* Returns the backing-buffer scale currently applied to a View.
5+
*
6+
* `ResolutionScale.resolutionScale` is only the configured value. The renderer
7+
* applies it only while the View's render mode is included in
8+
* `ResolutionScale.renderModes`; otherwise CSS pixels and drawing-buffer pixels
9+
* are 1:1.
10+
*/
11+
export function getEffectiveResolutionScale(view: Pick<View, "resolutionScale">): number {
12+
const resolutionScale = view.resolutionScale;
13+
return resolutionScale.applied
14+
? Math.max(0.05, resolutionScale.resolutionScale)
15+
: 1.0;
16+
}

packages/sdk/src/viewing/webGLRenderer/internal/snapManager/SnapManager.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {MeshManager} from "../meshManager/MeshManager";
1313
import {GPUMemoryManager} from "../gpuMemoryManager/GPUMemoryManager";
1414
import {getDrawOps, DrawOps, putDrawOps} from "../drawOps/DrawOps";
1515
import {RENDER_PASSES} from "../RENDER_PASSES";
16+
import {getEffectiveResolutionScale} from "../resolutionScale";
1617

1718
const tempVec3a = createVec3Float64();
1819
const tempVec4a = createVec4Float64();
@@ -171,10 +172,10 @@ export class SnapManager {
171172
// Cursor → clip-space NDC for the snap viewport remap. Same
172173
// formula PickManager uses for its 1×1 case; the snap viewport
173174
// gives the cursor a (2r+1)² window of pixels around it.
174-
const resolutionScale = view.resolutionScale;
175+
const effectiveResolutionScale = getEffectiveResolutionScale(view);
175176
renderContext.snapClipPos = createVec2Float64([
176-
this._clipPosX(params.canvasPos[0] * resolutionScale.resolutionScale, gl.drawingBufferWidth),
177-
this._clipPosY(params.canvasPos[1] * resolutionScale.resolutionScale, gl.drawingBufferHeight),
177+
this._clipPosX(params.canvasPos[0] * effectiveResolutionScale, gl.drawingBufferWidth),
178+
this._clipPosY(params.canvasPos[1] * effectiveResolutionScale, gl.drawingBufferHeight),
178179
]);
179180
renderContext.snapBufferSize = createVec2Float64([dim, dim]);
180181

@@ -285,8 +286,8 @@ export class SnapManager {
285286
const wClip = clipHomog[3] || 1;
286287
const ndcX = clipHomog[0] / wClip;
287288
const ndcY = clipHomog[1] / wClip;
288-
const cssWidth = gl.drawingBufferWidth / resolutionScale.resolutionScale;
289-
const cssHeight = gl.drawingBufferHeight / resolutionScale.resolutionScale;
289+
const cssWidth = gl.drawingBufferWidth / effectiveResolutionScale;
290+
const cssHeight = gl.drawingBufferHeight / effectiveResolutionScale;
290291
snappedCanvasPos[0] = (ndcX + 1) * 0.5 * cssWidth;
291292
snappedCanvasPos[1] = (1 - ndcY) * 0.5 * cssHeight;
292293

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/**
2+
* @jest-environment jsdom
3+
*/
4+
5+
jest.mock("../webGL", () => ({
6+
WEBGL_INFO: {MAX_TEXTURE_UNITS: 8},
7+
}));
8+
9+
import {RenderContext} from "../RenderContext";
10+
11+
describe("RenderContext init failure cleanup", () => {
12+
afterEach(() => {
13+
jest.restoreAllMocks();
14+
document.body.innerHTML = "";
15+
});
16+
17+
test("does not append a canvas when WebGL2 context creation fails", () => {
18+
jest.spyOn(HTMLCanvasElement.prototype, "getContext").mockReturnValue(null as any);
19+
const context = new RenderContext({} as any);
20+
21+
const result = context.init({} as any);
22+
23+
expect(result.ok).toBe(false);
24+
expect(document.body.querySelectorAll("canvas")).toHaveLength(0);
25+
expect(() => context.destroy()).not.toThrow();
26+
});
27+
});
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* @jest-environment jsdom
3+
*/
4+
5+
jest.mock("../webGL", () => ({
6+
WEBGL_INFO: {MAX_TEXTURE_UNITS: 8},
7+
}));
8+
jest.mock("../../../viewer", () => ({
9+
Camera: class {},
10+
Effect: class {},
11+
View: class {},
12+
Viewer: class {},
13+
ViewObject: class {},
14+
}));
15+
16+
import {ViewManager} from "../ViewManager";
17+
18+
function createViewer(numViews: number) {
19+
return {
20+
numViews,
21+
viewList: Array.from({length: numViews}, (_, i) => ({id: `view-${i}`})),
22+
};
23+
}
24+
25+
describe("ViewManager init failure cleanup", () => {
26+
test("destroy is safe after init fails before RenderContext creation", () => {
27+
const manager = new ViewManager();
28+
29+
const result = manager.init({
30+
viewer: createViewer(4) as any,
31+
memoryConfigs: {} as any,
32+
});
33+
34+
expect(result.ok).toBe(false);
35+
expect(() => manager.destroy()).not.toThrow();
36+
});
37+
});
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import {getEffectiveResolutionScale} from "../resolutionScale";
2+
3+
function viewWithResolutionScale(applied: boolean, resolutionScale: number) {
4+
return {
5+
resolutionScale: {
6+
applied,
7+
resolutionScale,
8+
},
9+
} as any;
10+
}
11+
12+
describe("getEffectiveResolutionScale", () => {
13+
test("returns 1 when resolution scaling is not active", () => {
14+
expect(getEffectiveResolutionScale(viewWithResolutionScale(false, 0.5))).toBe(1.0);
15+
});
16+
17+
test("returns the configured scale when resolution scaling is active", () => {
18+
expect(getEffectiveResolutionScale(viewWithResolutionScale(true, 0.5))).toBe(0.5);
19+
});
20+
21+
test("clamps active resolution scaling to the renderer minimum", () => {
22+
expect(getEffectiveResolutionScale(viewWithResolutionScale(true, 0.01))).toBe(0.05);
23+
});
24+
});

0 commit comments

Comments
 (0)