feat(input): add pressedPosition and target/currentTarget to pointer event data#3031
feat(input): add pressedPosition and target/currentTarget to pointer event data#3031cptbtptpbcptdtptp wants to merge 8 commits into
Conversation
…event data - Pointer.pressedPosition: captures screen position at pointer-down, useful for drag threshold and gesture recognition - PointerEventData.target: the deepest hit-tested entity (stays constant during bubble) - PointerEventData.currentTarget: the entity currently handling the event (changes during bubble traversal) This aligns the event model with the DOM convention where target is the originating element and currentTarget is the listener host. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis PR enriches pointer events with entity context information throughout the event pipeline. ChangesPointer Event Entity Context Tracking
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev/2.0 #3031 +/- ##
===========================================
+ Coverage 77.31% 79.18% +1.86%
===========================================
Files 914 914
Lines 101723 101688 -35
Branches 10437 11214 +777
===========================================
+ Hits 78647 80520 +1873
+ Misses 22892 20981 -1911
- Partials 184 187 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/input/pointer/PointerManager.ts`:
- Line 248: The code in PointerManager sets pointer.pressedPosition from
pointer.position (which was updated to latestEvent before the event loop), so a
batched pointerdown can record wrong coordinates; update the pointerdown branch
to copy coordinates from the current event (use event.position / event.clientX/Y
or the per-event coordinate object) instead of pointer.position, i.e., in the
pointerdown handling logic inside PointerManager, assign pressedPosition from
the event's coordinates rather than from pointer.position to ensure the down
location reflects that specific event.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: befad6e6-c8c8-4571-86f8-25f80a822e1f
📒 Files selected for processing (6)
packages/core/src/input/pointer/Pointer.tspackages/core/src/input/pointer/PointerEventData.tspackages/core/src/input/pointer/PointerManager.tspackages/core/src/input/pointer/emitter/PhysicsPointerEventEmitter.tspackages/core/src/input/pointer/emitter/PointerEventEmitter.tspackages/ui/src/input/UIPointerEventEmitter.ts
`pressedPosition` was copied from `pointer.position`, which is already overwritten with the frame's last event before the per-event loop runs, so a pointerdown batched with later events in the same frame recorded the wrong (last-event) coordinates. Compute it from the pointerdown event's own coordinates instead, and add a regression test that batches pointerdown + move in a single frame. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ty refs ClearableObjectPool gains disposeUsedElements(), which drops each used element's external references before resetting the index, so a peak-usage frame no longer keeps dangling references alive in slots that later frames don't reuse. PointerManager moves the pool reset out of _update's frame start into the end of _firePointerScript via disposeUsedElements(), so each frame's target/currentTarget entity refs are cleared instead of lingering. Also simplify _createEventData(pointer, e, e) to (pointer, e) in the physics and UI emitters (currentTarget defaults to target for non-bubbling fires), and add a UI test asserting target stays the deepest hit entity while currentTarget changes per bubble step. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… dispose The per-frame disposeUsedElements from the previous commit cleared target/currentTarget too eagerly: code that saved an eventData and read it in the same frame's onUpdate got null, and after pause the refs lingered anyway. No major engine does per-frame dispose (DOM/Pixi/Unity all defer cleanup to reuse). Revert to clear-on-reuse, and instead hook the pool into engine gc (PointerManager._gc -> InputManager._gc -> Engine._gc) like the render element pools, so peak memory and entity refs are reclaimed on gc and at destroy. Add a test for pool reclamation on gc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pointer no longer threads the event-data pool through to emitter constructors: it drops the ClearableObjectPool/PointerEventData imports and the awkward _addEmitters<T extends new (pool) => ...> generic, keeping only the _emitters array. PointerManager, which owns the pool, now creates the emitters and injects the pool directly. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pool is infrastructure that doesn't belong in PointerEventEmitter's construction contract -- it had leaked into the emitter constructor, the registry type, and the registerPointerEventEmitter decorator. Emitters now have a no-arg constructor and PointerManager injects the pool into the _pool field right after creating each emitter, so the registry and decorator signatures simplify to `new () => Emitter`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pressedPosition regression test already pins this behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…structor" This reverts commit 82b6f8b. Keep the constructor-injected pool for now; the field-injection cleanup will land in a follow-up PR. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GuoLei1990
left a comment
There was a problem hiding this comment.
总结
本轮 HEAD 7b2a4d4 自上次 LGTM 以来未变(HEAD commit 时间 02:48 早于上轮 review 03:14),无新增 commit。逐项复核确认全部问题已闭环,故本次正式 APPROVE 解除历史 CHANGES_REQUESTED 门控。
- 上轮 P0(
pressedPosition批处理帧捕获错误坐标)已在4f2bbc9真修复——采集挪进_updatePointerInfo的 per-event 循环、用 pointerdown 事件自身的event.clientX/clientY算(PointerManager 247-250),配套回归测试同帧批处理pointerdown@(1,1)+pointermove@(3,3)断言pressedPosition==(2,2)而position==(6,6),revert 即红,反向证伪成立。 target/currentTarget的 DOM 语义忠实度已独立核验:引擎确有冒泡模型——UIPointerEventEmitter._composedPath从命中元素(path[0],最深)沿 entity 父链构建到根 Canvas,_bubble逐级触发同一事件并逐级改写eventData.currentTarget = path[i],target恒为path[0]。故currentTarget有意义(非摆设),与 DOMevent.target(事件起源)/event.currentTarget(当前 handler 宿主)一致。物理拾取无冒泡,target === currentTarget经默认参数正确成立。- 事件数据池接入 engine gc 与既有
_renderElementPool生命周期同构:garbageCollection()仅清_elements、不重置_usedElementCount,但下一帧_update首句eventPool.clear()必在任何get()之前归零,不变量成立,非 bug。热路径无新增分配(池get()复用、Vector2.set原地、整条冒泡路径复用同一 eventData)。
CI 全绿(build/lint/e2e/codecov-patch/codecov-project)。
问题
无阻塞问题(无 P0/P1/P2)。
[P3] PointerEventData.ts:16 — currentTarget 的 JSDoc 偏机制描述
当前注释描述的是实现机制而非对外契约。可选择更贴近 DOM 文档口径的写法,例如「冒泡过程中当前正在处理事件的实体」。纯文案、非阻塞,当前措辞准确且带句号,不影响合并。
简化建议
无。上轮 _createEventData(pointer, entity, entity) 双传简化已采纳。构造器注入 vs 字段注入池的反复是作者明确把字段注入 cleanup 推迟到 follow-up PR 的主动决策,当前构造器注入工作正常、无对外影响,无需处理。
LGTM,APPROVE。
Summary
Pointer.pressedPosition: captures screen position at pointer-down, useful for drag threshold and gesture recognitionPointerEventData.target: the deepest hit-tested entity (stays constant during bubble)PointerEventData.currentTarget: the entity currently handling the event (changes during bubble traversal)Aligns the event model with the DOM convention (
event.targetvsevent.currentTarget).Test plan
pnpm run test)targetstays as the deepest entity during bubble;currentTargetchanges per handlerpressedPositionis captured at pointer-down and does not change during drag🤖 Generated with Claude Code
Summary by CodeRabbit