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

No one assigned

    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