Skip to content

fix(sokol): remove HiDPI camera offset double-scaling - #29

Merged
apotema merged 1 commit into
mainfrom
fix/sokol-camera-offset-double-scale
Apr 14, 2026
Merged

fix(sokol): remove HiDPI camera offset double-scaling#29
apotema merged 1 commit into
mainfrom
fix/sokol-camera-offset-double-scale

Conversation

@apotema

@apotema apotema commented Apr 14, 2026

Copy link
Copy Markdown
Contributor

Problem

On HiDPI mobile (Android, 2400×1080 physical / 1024×768 design), world-space entities render far off-screen. The flying-platform scene appears empty because the ship carcase ends up at NDC (1.34, ~−2.4) — completely outside the visible range.

Root cause

a610afd added cam.offset * (screen / design) scaling in toNdcX/toNdcY/screenToWorld/worldToScreen under the assumption that the camera offset was in design coordinates. It isn't.

labelle-gfx's camera.toBackend() builds the offset as:

```zig
pub fn toBackend(self: *const Self) BackendImpl.Camera2D {
const dims = self.getViewportDimensions();
return .{
.offset = .{ .x = dims.width / 2.0, .y = dims.height / 2.0 },
...
};
}

pub fn getViewportDimensions(self: *const Self) struct { width: f32, height: f32 } {
...
return .{
.width = @floatFromInt(BackendImpl.getScreenWidth()),
.height = @floatFromInt(BackendImpl.getScreenHeight()),
};
}
```

In the sokol backend getScreenWidth/getScreenHeight return screen_w/screen_h — the physical framebuffer dimensions set by setScreenSize(). So by the time beginMode2D(camera) stores it, cam.offset is already in physical pixels.

Multiplying that already-physical value by screen/design (≈ 2.3 on this device) shoves the world origin off-screen:

```
cam.offset.x = screen_w / 2 = 1200
1200 * (2400 / 1024) = 2812.5
NDC = (2812.5 / 2400) * 2 - 1 = 1.34 // outside [-1, 1]
```

Fix

Drop the * (fw/fdw) / * (fh/fdh) correction from all four functions and use cam.offset directly. Dividing physical-pixel screen_x by physical screen_w already yields correct NDC: (1200 / 2400) * 2 - 1 = 0.

The screen-space (no-camera) branch still divides by design_w/design_h — screen-space positions are still authored in design coordinates, so that path is untouched.

Test plan

  • Deploy to flying-platform-labelle Android emulator build
  • Verify world-space entities render at expected positions (ship carcase centered in view, not offset off-screen)
  • Screen-space background still fills the whole screen
  • Desktop sokol path unchanged (design == physical → ratio = 1, so the old math collapsed to the same thing; still want a sanity check)

labelle-gfx's camera.toBackend() builds the offset as
`{ getScreenWidth()/2, getScreenHeight()/2 }`. In the sokol backend
getScreenWidth/Height return `screen_w`/`screen_h` — the physical
framebuffer dimensions set by setScreenSize() — so cam.offset is
already in physical pixels when the camera is handed to the backend.

a610afd added `cam.offset * (screen / design)` in toNdcX/Y,
screenToWorld and worldToScreen under the assumption that the offset
was in design coordinates. On HiDPI mobile (e.g. 2400×1080 physical
vs 1024×768 design) this multiplies an already-physical value by
screen/design ≈ 2.3, shoving the world-space origin off-screen:

  cam.offset.x = screen_w/2 = 1200
  1200 * (2400/1024) = 2812.5
  NDC = (2812.5 / 2400) * 2 - 1 = 1.34   // outside [-1,1]

Use cam.offset directly: (offset.x / screen_w) * 2 - 1 lands at 0 as
intended. Verified on Android emulator (arm64) by loading the
flying-platform scene — before the fix the world was offset far
off-screen; after the fix entities render centered on the camera.

Screen-space (no-camera) branch still divides by design_w/design_h
because screen-space positions remain in design coordinates.
@cursor

cursor Bot commented Apr 14, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes core camera coordinate transforms in the Sokol backend; low blast radius outside rendering, but mistakes would visibly break world/screen mapping on HiDPI or non-HiDPI devices.

Overview
Fixes HiDPI camera positioning in the Sokol backend by removing the extra screen/design scaling applied to Camera2D.offset.

toNdcX/toNdcY, screenToWorld, and worldToScreen now use camera.offset directly (treated as physical pixels), while the screen-space (no-camera) NDC mapping continues to use design_w/design_h. The camera math comments were updated to document this contract.

Reviewed by Cursor Bugbot for commit df8260e. Bugbot is set up for automated code reviews on this repo. Configure here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request simplifies coordinate transformations in the Sokol backend by removing redundant HiDPI scaling for camera offsets. The logic was updated in toNdcX, toNdcY, screenToWorld, and worldToScreen because the camera offset is already provided in physical pixels, making the manual scaling between design and physical dimensions unnecessary. I have no feedback to provide.

@apotema
apotema requested a review from Copilot April 14, 2026 13:17
@apotema
apotema merged commit 63d44e3 into main Apr 14, 2026
6 checks passed
@apotema
apotema deleted the fix/sokol-camera-offset-double-scale branch April 14, 2026 13:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes incorrect camera coordinate transforms in the sokol backend on HiDPI/mobile by removing an extra (and incorrect) design→physical scaling of Camera2D.offset, which was pushing world-space content off-screen when screen != design.

Changes:

  • Remove double-scaling of cam.offset in toNdcX / toNdcY when a camera is active.
  • Remove the same offset scaling from screenToWorld and worldToScreen.
  • Update the in-file documentation to clarify that cam.offset is already in physical framebuffer pixels for the sokol backend.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants