Closed
Bug 1404273
Opened 7 years ago
Closed 7 years ago
[Pointer Event] Coalescing mousemove for different pointers
Categories
(Core :: DOM: Events, enhancement, P1)
Core
DOM: Events
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: stone, Assigned: stone)
References
Details
Attachments
(1 file, 1 obsolete file)
6.19 KB,
patch
|
stone
:
review+
|
Details | Diff | Splinter Review |
According to [1], we should coalesce the mousemove / pointermove events which belong to the same pointer (has the same poiniter id) [1] https://w3c.github.io/pointerevents/extension.html
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → sshih
Assignee | ||
Comment 1•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8915779 -
Flags: review?(bugs)
Comment 2•7 years ago
|
||
Comment on attachment 8915779 [details] [diff] [review] Coalescing mousemove for different pointers > TabChild::MaybeDispatchCoalescedMouseMoveEvents() > { >- if (!mCoalesceMouseMoveEvents || mCoalescedMouseData.IsEmpty()) { >+ if (!mCoalesceMouseMoveEvents) { > return; > } >- const WidgetMouseEvent* event = mCoalescedMouseData.GetCoalescedEvent(); >- MOZ_ASSERT(event); >- // Dispatch the coalesced mousemove event. Using RecvRealMouseButtonEvent to >- // bypass the coalesce handling in RecvRealMouseMoveEvent. >- RecvRealMouseButtonEvent(*event, >- mCoalescedMouseData.GetScrollableLayerGuid(), >- mCoalescedMouseData.GetInputBlockId()); >- if (mCoalescedMouseEventFlusher) { >- mCoalescedMouseData.Reset(); >- mCoalescedMouseEventFlusher->RemoveObserver(); >+ for (auto iter = mCoalescedMouseData.Iter(); !iter.Done(); iter.Next()) { >+ CoalescedMouseData* data = iter.UserData(); >+ if (!data || data->IsEmpty()) { >+ continue; >+ } >+ const WidgetMouseEvent* event = data->GetCoalescedEvent(); >+ MOZ_ASSERT(event); >+ // Dispatch the coalesced mousemove event. Using RecvRealMouseButtonEvent to >+ // bypass the coalesce handling in RecvRealMouseMoveEvent. >+ RecvRealMouseButtonEvent(*event, >+ data->GetScrollableLayerGuid(), >+ data->GetInputBlockId()); >+ data->Reset(); >+ if (mCoalescedMouseEventFlusher) { >+ mCoalescedMouseEventFlusher->RemoveObserver(); >+ } Why do we call RemoveObserver inside the for loop? Move this to be outside the loop? Since CoalescedMouseData is now explicitly allocated, please add CTOR and DTOR counters to its constructor/destructor.
Attachment #8915779 -
Flags: review?(bugs) → review+
Assignee | ||
Comment 3•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=d1d363b5dd9eda1a1df30e097d8d184200fe22d5
Updated•7 years ago
|
Priority: -- → P1
Assignee | ||
Comment 4•7 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=2209b8e5509c995adbaff02bfacb0cab7f3ea201
Assignee | ||
Comment 5•7 years ago
|
||
Followed review comments to revise the patch
Attachment #8915779 -
Attachment is obsolete: true
Attachment #8916828 -
Flags: review+
Pushed by sshih@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5e0a1d82ef67 [Pointer Event] Coalescing mousemove for different pointers. r=smaug.
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/5e0a1d82ef67
Status: NEW → RESOLVED
Closed: 7 years ago
status-firefox58:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•