Skip to content

fix(input): account for world offset in pointer events - #1605

Merged
obiot merged 4 commits into
melonjs:masterfrom
Vareniel:fix/flex-scale-pointer-events
Aug 27, 2026
Merged

fix(input): account for world offset in pointer events#1605
obiot merged 4 commits into
melonjs:masterfrom
Vareniel:fix/flex-scale-pointer-events

Conversation

@Vareniel

Copy link
Copy Markdown
Contributor

Description

Fix pointer event hit detection when flex-height or flex-width scaling introduces an offset on the root game world.

Previously, pointer coordinates and non-floating renderable bounds could use different coordinate spaces. This caused registered pointer events, such as pointerdown, to become offset from their visual renderables.

This change:

  • Accounts for the root world offset when querying non-floating renderables.
  • Preserves level-local coordinates for floating renderables.
  • Uses the appropriate coordinate space for bounds checks and local pointer coordinates.
  • Adds regression tests for vertical flex-height and horizontal flex-width world offsets.

Type of change

  • Bug fix
  • New feature
  • Documentation update
  • Performance improvement
  • Refactoring (no functional changes)

Checklist

  • I have read the Contributing Guide
  • My code follows the existing code style (pnpm lint passes)
  • I have tested my changes locally (pnpm test passes)
  • I have added tests that cover my changes (if applicable)
  • The build succeeds (pnpm build)

Related issues

No related issue.

Copilot AI lite review requested due to automatic review settings August 26, 2026 08:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes pointer event hit detection when the root world is offset (e.g., due to flex-height / flex-width centering), ensuring pointer coordinates, bounds checks, and local pointer coordinates use the correct coordinate space for floating vs non-floating renderables.

Changes:

  • Adjust pointer candidate retrieval and bounds checks to account for root world offset for non-floating renderables while preserving level-local logic for floating renderables.
  • Add regression tests covering both vertical and horizontal world-offset scenarios.
  • Ensure test cleanup resets world position between cases.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
packages/melonjs/src/input/pointerevent.ts Updates pointer event dispatch candidate retrieval and coordinate-space handling to correctly detect hits when the world has a root offset.
packages/melonjs/tests/input.spec.js Adds regression coverage for pointerdown hit detection under flex-induced world offsets and resets world position during cleanup.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +343 to +348
currentPointer.pos.set(absoluteWorldX, absoluteWorldY);
const absoluteCandidates = _app.world.broadphase.retrieve(
currentPointer,
(a: any, b: any) => _app.world._sortReverseZ(a, b),
undefined,
);
obiot and others added 2 commits August 27, 2026 08:46
Confirms the reported bug is real and that the fix holds, by reverting
`pointerevent.ts` to master and watching these fail: without it the pointer
handler is never called at all for a non-floating region in an offset world —
not a coordinate drift, a total loss of input. 12 of the 15 new cases fail on
master and pass with the fix.

The three that pass either way are the regression guards: a floating region,
a child of a floating container, and an unshifted world must all behave
exactly as before.

Covered beyond the original two cases: negative, fractional and both-axis
offsets; that a pointer OUTSIDE a shifted region still misses (the fix runs a
second broadphase query and merges, so the hazard is a widened net — asserted
with a positive control so it cannot pass by never hitting); that
`gameWorldX/Y` stay level-local; children of floating containers, since
`isFloating` inherits from the ancestor and the fix branches on it; a nested
container with its own position; a scrolled camera; z-order between two
overlapping regions, since the merged candidate list gets re-sorted;
pointermove; and an offset changed after registration.

Also pins that no scale method introduces a world offset, at init or through
a resize. That is the blast radius: `world.pos` is moved by GAME code to
centre a level, never by the engine, so an ordinary game keeps taking the
original path in every mode. The bug is therefore not specific to
flex-height/flex-width — any non-zero world offset reaches it.

Deliberately not asserted: that a click lands on a region under each scale
method. Those call `renderer.resize()` against the parent element, which in a
headless harness has no meaningful size (the canvas comes out at 800x1731),
so such a test measures the harness rather than the engine. Verified
separately that behaviour under every mode is identical before and after this
change.

Adds the CHANGELOG entry, and picks up the biome formatting `pnpm lint` wants
on the spec file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Copilot AI review requested due to automatic review settings August 27, 2026 00:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@obiot

obiot commented Aug 27, 2026

Copy link
Copy Markdown
Member

Reviewed this, and the bug is real. Thanks for tracking it down — this one is nastier than the title suggests.

Confirmed

Camera2d.localToWorld subtracts world.pos, so a pointer's gameWorldX/Y come out level-local, while a non-floating renderable's bounds are absolute and already include that offset. At the origin the two spaces coincide, which is why nobody hit this sooner. Move the world and the two disagree by exactly world.pos.

I checked the failure mode rather than assuming it, by reverting pointerevent.ts to master and rerunning: the handler is never called at all. Not a drift, not an off-by-a-few — total loss of input for every non-floating region. That is worth stating plainly in the changelog, which I have added.

What I pushed to your branch

maintainerCanModify was on, so I have added coverage rather than asking you to write it. 15 new cases in pointer-world-offset.spec.js; 12 fail against master and pass with your fix. The other 3 are the regression guards and pass either way, which is the point of them.

Beyond the two cases you had:

  • negative, fractional and both-axis offsets
  • a pointer outside a shifted region still misses. This was my main worry: the fix runs a second broadphase query and merges, so the hazard is a widened net that starts reporting hits nowhere near the pointer. Asserted with a positive control alongside, so it cannot pass by simply never hitting anything
  • gameWorldX/Y stay level-local (the coordinates a game reasons in, not screen ones)
  • a child of a floating container. isFloating inherits from the ancestor and your fix branches on it, so a child that never set floating itself must still take the floating path or a HUD built out of parts breaks
  • a nested container with its own position, a scrolled camera, pointermove, and an offset changed after registration
  • z-order between two overlapping regions, since the merged candidate list gets re-sorted and a dropped or duplicated entry shows up as the wrong region winning

On regressions in other modes

No behaviour change: scale-method handling is identical before and after, and world.pos stays at the origin under all seven methods, at init and across a resize. pointer-scale-modes.spec.js pins that.

Which leads to the one thing I would flag about the framing: nothing in the engine ever sets world.pos — a game does, to centre a level. So this is not really specific to flex-height/flex-width; those just made it easy to notice. Any non-zero world offset reaches it. The tests therefore drive world.pos directly instead of going through a scale mode.

Deliberately not asserted: a click landing on a region under each scale method. Those call renderer.resize() against the parent element, which headlessly has no meaningful size (the canvas comes out 800x1731), so such a test measures the harness and not the engine. I verified that behaviour separately instead.

Two things left

  1. pnpm lint did not pass — biome wanted to reformat input.spec.js. Picked up in my commit, but worth running locally before the next push.
  2. Input-rate cost. When the offset is non-zero the handler does a second broadphase query, a merge and a re-sort on every pointer event, including pointermove. Correct, and not something to hold this PR for, but a scene with a large candidate set will feel it. A follow-up could query once in the right space rather than querying twice and reconciling.

Also merged current master in, since 20.2.0 landed after you opened this. Full suite on the merged result: 6280 passed, 9 skipped, types and lint clean.

Bare handle in a trailing `(thanks @user)`, matching every other credited
entry in the file, rather than a linked handle inside the leading ref.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Aa37KGXZcnVrbn1yG4j1N
Copilot AI review requested due to automatic review settings August 27, 2026 01:40

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@obiot
obiot merged commit 008980a into melonjs:master Aug 27, 2026
3 checks passed
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.

3 participants