Closed
Bug 1091283
Opened 10 years ago
Closed 10 years ago
Refactor APZ test code to more easily allow dispatching input through APZCTM
Categories
(Core :: Panning and Zooming, defect)
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: kats, Assigned: kats)
Details
Attachments
(2 files)
4.91 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
48.44 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
Bug 1073250 added an ApzctmPan function which essentially duplicated the ApzcPan functionality, with a TODO to refactor stuff later. As part of APZ hit-testing (bug 918288 and friends) I'm going to be writing some tests that send input events through APZCTM, so now is a good time to clean that up.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8513893 -
Flags: review?(botond)
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8513894 -
Flags: review?(botond)
Comment 3•10 years ago
|
||
Comment on attachment 8513893 [details] [diff] [review] Part 1 - Ensure gesture events can get to APZC via ReceiveInputEvent Review of attachment 8513893 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/apz/src/InputQueue.cpp @@ +29,5 @@ > InputQueue::ReceiveInputEvent(const nsRefPtr<AsyncPanZoomController>& aTarget, const InputData& aEvent, uint64_t* aOutInputBlockId) { > AsyncPanZoomController::AssertOnControllerThread(); > > if (aEvent.mInputType != MULTITOUCH_INPUT) { > + // The return value for non-touch input isn't really used, so just pass s/isn't really used/is only used by tests
Attachment #8513893 -
Flags: review?(botond) → review+
Comment 4•10 years ago
|
||
Comment on attachment 8513894 [details] [diff] [review] Part 2 - Templat-ify the input event helpers and reuse code Review of attachment 8513894 [details] [diff] [review]: ----------------------------------------------------------------- Looks pretty good overall! I have two related requests: - For all functions that are made function templates in this patch, please name the template parameter something descriptive, such as 'EventTarget', rather than 'T'. - Add a comment saying what the requirements are for an EventTarget (has a method named ReceiveInputEvent() with a certain signature) and what classes currently meet these requirements (APZCTreeManager, TestAsyncPanZoomController). r+ with these changes. ::: gfx/tests/gtest/TestAsyncPanZoomController.cpp @@ +291,4 @@ > } > > +template<class T> static void > +Tap(const nsRefPtr<T> aTarget, int aX, int aY, int& aTime, int aTapLength, You probably meant to take |aTarget| by reference here too. @@ +326,5 @@ > { > + // Reduce the touch start tolerance to a tiny value. > + // We can't use a scoped pref because this value might be read at some later > + // time when the events are actually processed, rather than when we deliver > + // them. Tricky!
Attachment #8513894 -
Flags: review?(botond) → review+
Assignee | ||
Comment 5•10 years ago
|
||
(In reply to Botond Ballo [:botond] from comment #4) > - For all functions that are made function templates in this patch, > please name the template parameter something descriptive, such > as 'EventTarget', rather than 'T'. > > - Add a comment saying what the requirements are for an EventTarget > (has a method named ReceiveInputEvent() with a certain signature) > and what classes currently meet these requirements (APZCTreeManager, > TestAsyncPanZoomController). Cool, made these changes (and the one in part 1). > > > > +template<class T> static void > > +Tap(const nsRefPtr<T> aTarget, int aX, int aY, int& aTime, int aTapLength, > > You probably meant to take |aTarget| by reference here too. Indeed, nice catch! :) Will land once the trees reopen.
Assignee | ||
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/543860322c50 https://hg.mozilla.org/integration/mozilla-inbound/rev/84c97cd075da
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/543860322c50 https://hg.mozilla.org/mozilla-central/rev/84c97cd075da
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•