Skip to content

SheetBoard double-applies devicePixelRatio in _doRender scissor/viewport math #89

@rihokirss

Description

@rihokirss

Summary

SheetBoard appears to apply devicePixelRatio twice when computing the WebGL scissor/viewport rectangle inside _doRender().

In our app this causes the rendered drawing content to drift relative to the DOM overlay border when DPR != 1, for example on HiDPI displays or when browser zoom is not 100%.

Upstream source location

Repository: ThatOpen/engine_ui-components
File: packages/obc/src/core/SheetBoard/index.ts
Relevant lines: 670-676

Current code:

const glX = Math.round(sx * dpr);
const glY = Math.round((hostH - sy - sh) * dpr);
const glW = Math.round(sw * dpr);
const glH = Math.round(sh * dpr);

renderer.setScissor(glX, glY, glW, glH);
renderer.setViewport(glX, glY, glW, glH);

Why this looks wrong

renderer is initialized with:

this._renderer.setPixelRatio(window.devicePixelRatio);

Three.js already uses its internal pixel ratio when applying setScissor() / setViewport(). Because of that, pre-multiplying the CSS-pixel coordinates by dpr here seems to double-apply the ratio.

Suggested fix

Pass CSS-pixel coordinates directly to setScissor() and setViewport().

Suggested change in packages/obc/src/core/SheetBoard/index.ts:670-676:

const glX = Math.round(sx);
const glY = Math.round(hostH - sy - sh);
const glW = Math.round(sw);
const glH = Math.round(sh);

renderer.setScissor(glX, glY, glW, glH);
renderer.setViewport(glX, glY, glW, glH);

Or more minimally, keep the current variable names but remove the * dpr multiplications.

Observed behavior

When DPR != 1:

  • the WebGL-rendered viewport content is offset
  • the DOM border overlay remains in the expected place
  • the vertical offset scales with DPR

Workaround we currently need downstream

We currently monkey-patch the internal renderer in our app to divide incoming setScissor / setViewport args by DPR before delegating to the original methods.

That workaround lives here in our app:
client/src/modules/drawing-editor/components/SheetBoardCanvas.tsx:233-257

We would prefer to remove it because it depends on the private _renderer field.

Environment

Package: @thatopen/ui-obc
Version: 3.4.0

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions