bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

[APZ] Use PanGestureInput events to trigger APZ scrolling on OS X

RESOLVED FIXED in Firefox 43

Status

()

Core
Panning and Zooming
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

(Depends on: 1 bug)

Trunk
mozilla43
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox43 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(21 attachments)

40 bytes, text/x-review-board-request
kats
: review+
masayuki
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kip
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
smichaud
: review+
Details | Review
40 bytes, text/x-review-board-request
smichaud
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
smichaud
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
40 bytes, text/x-review-board-request
kats
: review+
Details | Review
Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats

These events are used to show / hide scrollbars before the fingers have moved. This is done from default handling in EventStateManager which is skipped when the events are marked with mHandledByAPZ.
Attachment #8646338 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 2

3 years ago
Created attachment 8646339 [details]
MozReview Request: Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

WidgetWheelEvents that are not handled by APZ include those used for zooming, and WHEEL_START / WHEEL_STOP.
Attachment #8646339 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 3

3 years ago
Created attachment 8646340 [details]
MozReview Request: Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

When scrolling an inactive subframe, the target APZC confirmation needs to be sent along with the layers transaction. If a new wheel event comes in while that target APZC notification is in-flight, the subframe will already have a display port, so the confirmation will be sent in a more direct fashion and can arrive at the APZ controller thread before the new APZC for the scroll frame has been created. Then the current input block will have a null target APZC and no scrolling will happen until a new input block is created. (For wheel transactions, a null target APZC ends the transaction, so the next event will create a new input block.)
Attachment #8646340 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 4

3 years ago
Created attachment 8646341 [details]
MozReview Request: Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

This was discussed in bug 1107716.
Attachment #8646341 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 5

3 years ago
Created attachment 8646342 [details]
MozReview Request: Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

We are going to create WidgetWheelEvents from them and need more information than what they currently have.
Attachment #8646342 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 6

3 years ago
Created attachment 8646343 [details]
MozReview Request: Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

It shouldn't be checking the w coordinate of the difference of two Point4Ds. The 4D difference isn't really meaningful. Instead, it should be checking each point's w coordinate individually.
Attachment #8646343 - Flags: review?(kgilbert)
(Assignee)

Comment 7

3 years ago
Created attachment 8646344 [details]
MozReview Request: Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

PanGestureInput events need to be sent to the pre-scroll position, just like wheel events.
Attachment #8646344 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 8

3 years ago
Created attachment 8646345 [details]
MozReview Request: Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats

Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats
Attachment #8646345 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 9

3 years ago
Created attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats
Attachment #8646346 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 10

3 years ago
Created attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

Bug 1193062 - Add PanGestureBlockState. r?kats
Attachment #8646347 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 11

3 years ago
Created attachment 8646348 [details]
MozReview Request: Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats

Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats
Attachment #8646348 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 12

3 years ago
Created attachment 8646349 [details]
MozReview Request: Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

This will make bug 1156606 unnecessary.
Attachment #8646349 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 13

3 years ago
Created attachment 8646350 [details]
MozReview Request: Bug 1193062 - Remove mPanGestureState. r?kats

Bug 1193062 - Remove mPanGestureState. r?kats

Now that PanGestureInput events are only processed during a PanGestureBlock, we can get rid of the mPanGestureState workaround.
Attachment #8646350 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 14

3 years ago
Created attachment 8646351 [details]
MozReview Request: Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats

Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats
Attachment #8646351 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 15

3 years ago
Created attachment 8646352 [details]
MozReview Request: Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

We don't want to split native NSEvents into two PanGestureInput events.
Attachment #8646352 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 16

3 years ago
Created attachment 8646353 [details]
MozReview Request: Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats

Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats
Attachment #8646353 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 17

3 years ago
Created attachment 8646354 [details]
MozReview Request: Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud

Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud
Attachment #8646354 - Flags: review?(smichaud)
(Assignee)

Comment 18

3 years ago
Created attachment 8646355 [details]
MozReview Request: Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

NSEvent timestamps are relative to the system start time, not to the reference date.
Attachment #8646355 - Flags: review?(smichaud)
(Assignee)

Comment 19

3 years ago
Created attachment 8646356 [details]
MozReview Request: Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud

Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud
Attachment #8646356 - Flags: review?(smichaud)
Attachment #8646356 - Flags: review?(bugmail.mozilla)
Attachment #8646338 - Flags: review?(bugmail.mozilla)
Comment on attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

https://reviewboard.mozilla.org/r/15687/#review13999

::: widget/cocoa/nsChildView.mm:4824
(Diff revision 1)
> -  mGeckoChild->DispatchAPZAwareEvent(wheelEvent.AsInputEvent());
> +  nsEventStatus status;

I think it might be better to ignore NS_WHEEL_START and NS_WHEEL_STOP events in the WillHandleWheelEvent function in APZCTreeManager. That way the mHandledByAPZ flag won't get set, but the event will still be untransformed properly. And if in the future it is needed for something like breaking up wheel transactions or whatever we will still have access to it.
Attachment #8646339 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646339 [details]
MozReview Request: Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

https://reviewboard.mozilla.org/r/15689/#review14001

Ship It!
Attachment #8646340 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646340 [details]
MozReview Request: Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

https://reviewboard.mozilla.org/r/15691/#review14003

Nice catch! Since this fixes a pre-existing issue I would have preferred it land as a separate bug rather than as part of this queue, but probably not worth doing that now.

::: gfx/layers/apz/util/APZCCallbackHelper.cpp:756
(Diff revision 1)
> +    APZCCH_LOG("Not resending target APZC confirmation for input block %lu\n", aInputBlockId);

"%lu\n" should be "%" PRIu64 "\n"
Comment on attachment 8646341 [details]
MozReview Request: Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

https://reviewboard.mozilla.org/r/15693/#review14011

Ship It!
Attachment #8646341 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646342 [details]
MozReview Request: Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

https://reviewboard.mozilla.org/r/15695/#review14013

::: widget/InputData.cpp:248
(Diff revision 1)
> +  wheelEvent.refPoint = RoundedToInt(mPanStartPoint * ScreenToLayoutDeviceScale(1));

I'd prefer wheelEvent.refPoint = RoundedToInt(ViewAs<LayoutDevicePixel>(mPanStartPoint, PixelCastJustification::LayoutDeviceToScreenForUntransformedEvent))

::: widget/InputData.cpp:256
(Diff revision 1)
> +  wheelEvent.mFlags.mHandledByAPZ = true;

Feels a bit wrong to be setting mHandledByAPZ directly here; might be better to move this to the callsite. I haven't looked at the future patches though so maybe it makes more sense here.

::: widget/InputData.cpp:318
(Diff revision 1)
> +  wheelEvent.refPoint = RoundedToInt(mOrigin * ScreenToLayoutDeviceScale(1));

Ditto on using PixelCastJustification

::: widget/InputData.cpp:326
(Diff revision 1)
> +  wheelEvent.mFlags.mHandledByAPZ = true;

Ditto on mHandledByAPZ
Attachment #8646342 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646344 [details]
MozReview Request: Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

https://reviewboard.mozilla.org/r/15699/#review14017

Ship It!
Attachment #8646344 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646345 [details]
MozReview Request: Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats

https://reviewboard.mozilla.org/r/15701/#review14019

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1690
(Diff revision 1)
> -  mX.UpdateWithTouchAtDevicePoint(aEvent.mLocalPanStartPoint.x, aEvent.mTime);
> +  mX.UpdateWithTouchAtDevicePoint(aEvent.mLocalPanStartPoint.x, aEvent.mLocalPanDisplacement.x, aEvent.mTime);

Can we just pass in aEvent.mLocalPanStartPoint.x + aEvent.mLocalPanDisplacement.x for the first parameter here, do the same for the y component, and then drop the rest of the changes? It will cause Axis::mPos to be different, but I don't think that should break anything.
Attachment #8646345 - Flags: review?(bugmail.mozilla)
Attachment #8646346 - Flags: review?(bugmail.mozilla)
Comment on attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

https://reviewboard.mozilla.org/r/15703/#review14023

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1487
(Diff revision 1)
> +    delta = LayoutDevicePoint(panInput.mPanDisplacement.x, panInput.mPanDisplacement.y);

Hm, I think this might need to use mLocalPanDisplacement instead of mPanDisplacement. And I think there's a pre-existing issue with this code that it's use LayoutDevicePoint instead of ParentLayerPoint for the delta variable. I'm not 100% sure of this though.
(Assignee)

Comment 28

3 years ago
https://reviewboard.mozilla.org/r/15701/#review14019

> Can we just pass in aEvent.mLocalPanStartPoint.x + aEvent.mLocalPanDisplacement.x for the first parameter here, do the same for the y component, and then drop the rest of the changes? It will cause Axis::mPos to be different, but I don't think that should break anything.

Will this work? If I have a pan start point at y = 10, and I have two events with deltaY 5 and 3, with your suggestion I'd call StartTouch(10), Update(15), Update(13), so the velocity will be negative for the deltaY 3 one.
Oh, my bad. I was thinking of the displacement values as cumulative but they're not. In that case I think the patch is fine as-is.
Comment on attachment 8646345 [details]
MozReview Request: Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats

https://reviewboard.mozilla.org/r/15701/#review14039
Attachment #8646345 - Flags: review+
Attachment #8646347 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

https://reviewboard.mozilla.org/r/15705/#review14027

::: gfx/layers/apz/src/InputQueue.h:93
(Diff revision 1)
> +  PanGestureBlockState* CurrentPanGestureBlock() const;

Might as well generify the comment above CurrentTouchBlock() and use that single generic comment for all three functions. (i.e.:

/* Generic comment
 */
Foo* function() const;
Bar* function() const;
Baz* function() const;

::: gfx/layers/apz/src/InputBlockState.h:251
(Diff revision 1)
> +class PanGestureBlockState : public CancelableBlockState

It's probably time to refactor these classes so more of the code is shared. Please file a follow-up bug for that.
Comment on attachment 8646348 [details]
MozReview Request: Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats

https://reviewboard.mozilla.org/r/15707/#review14045

::: gfx/layers/apz/src/InputQueue.h:109
(Diff revision 1)
> +   * 

Missing comment

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:2083
(Diff revision 1)
>    {

Move brace up to end of previous line

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:2100
(Diff revision 1)
>    return OverscrollForPanning(overscroll, aOverscrollHandoffState.mPanDistance);

Doesn't this allow overscrolling with wheel events too? It's not clear to me why you allow this to run when handoff is disabled, when it didn't run in the original code.
Attachment #8646348 - Flags: review?(bugmail.mozilla)
Comment on attachment 8646349 [details]
MozReview Request: Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

https://reviewboard.mozilla.org/r/15709/#review14049

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1555
(Diff revision 1)
> -      // wheel and touchpad scroll gestures, so we invert x/y here. Since the
> +          ScreenPoint(fabs(delta.x), fabs(delta.y)),

This casting from LayoutDevicePoint to ScreenPoint is suspicious-looking to me. I'm having trouble wrapping my head around the units right now, and my gut feeling is that something isn't quite right. If we end up supporting both zoom and wheel scrolling on a platform we might need to revisit this.
Attachment #8646349 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646350 [details]
MozReview Request: Bug 1193062 - Remove mPanGestureState. r?kats

https://reviewboard.mozilla.org/r/15711/#review14051

yay code deletion \o/
Attachment #8646350 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646351 [details]
MozReview Request: Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats

https://reviewboard.mozilla.org/r/15713/#review14053

I think we should be able to get rid of ScrollSource entirely at this point. (If bug 1190895 were fixed I'd be able to say that with more certainty...)
Attachment #8646351 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 36

3 years ago
https://reviewboard.mozilla.org/r/15707/#review14045

> Doesn't this allow overscrolling with wheel events too? It's not clear to me why you allow this to run when handoff is disabled, when it didn't run in the original code.

True, this is a behavior change. It still won't do anything unless you set the overscroll pref to true, though.

We might need a ScrollSource difference between PanGestureInput and ScrollWheelInput events once we start supporting overscrolling with PanGestureInput events, and then disallow overscrolling for wheel events. Or we could choose to also support overscrolling with wheel events.
Comment on attachment 8646352 [details]
MozReview Request: Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

https://reviewboard.mozilla.org/r/15715/#review14055

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1690
(Diff revision 1)
> +    // Call PanBegin. It will call back into OnPan so we return here.

s/OnPan so we return here/this function with mState == PANNING/

But from the comments it's not clear why we drop momentum events that weren't preceded by a start, but we allow pan events that weren't preceded by a start.

I considered the scenario where the user starts panning with the trackpad, then pauses. Content triggers a smooth scroll animation, and then while that's going the user resumes panning on the trackpad. In this scenario we're going to enter the mState == SMOOTH_SCROLL block above, cancel the scroll animation, and then enter this mState == NOTHING block. In this case we do want to pan, but it's technically not correct to say that "we started panning without a pan begin event" because we did get a pan begin event back before the smooth scroll.

I think the comments here need updating. They should reflect that *if* we enter this code with state mState == NOTHING, it means that the block was interrupted by something else; after interruption we want to trigger panning if the user's fingers are still on the touchpad but not for momentum events.

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1678
(Diff revision 1)
>      } else {

While you're here, please remove the else-after-return.
Attachment #8646352 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646353 [details]
MozReview Request: Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats

https://reviewboard.mozilla.org/r/15717/#review14069

I guess this addresses my concern from part 5 (bug 1193062 comment 24) about mHandledByAPZ.
Attachment #8646353 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646356 [details]
MozReview Request: Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud

https://reviewboard.mozilla.org/r/15723/#review14071

Partly a rubber-stamp on this one. The event-dispatching bits look ok to me but I don't know much about the different decisions that drive which event type gets used. Assuming smichaud will review that bit.
Attachment #8646356 - Flags: review?(bugmail.mozilla) → review+
(In reply to Markus Stange [:mstange] from comment #36)
> https://reviewboard.mozilla.org/r/15707/#review14045
> 
> > Doesn't this allow overscrolling with wheel events too? It's not clear to me why you allow this to run when handoff is disabled, when it didn't run in the original code.
> 
> True, this is a behavior change. It still won't do anything unless you set
> the overscroll pref to true, though.

Ok, makes sense.

> We might need a ScrollSource difference between PanGestureInput and
> ScrollWheelInput events once we start supporting overscrolling with
> PanGestureInput events, and then disallow overscrolling for wheel events. Or
> we could choose to also support overscrolling with wheel events.

Ok. We can leave the ScrollSource in for now. I think it makes sense to disallow overscrolling for wheel events since.
Comment on attachment 8646343 [details]
MozReview Request: Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

https://reviewboard.mozilla.org/r/15697/#review14073

Good catch!  You are right -- can't just subtract w's of homogenous coordinates.  Ship it.
Attachment #8646343 - Flags: review?(kgilbert) → review+
(Assignee)

Updated

3 years ago
Attachment #8646338 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 42

3 years ago
Comment on attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats

These events are used to show / hide scrollbars before the fingers have moved. This is done from default handling in EventStateManager which is skipped when the events are marked with mHandledByAPZ.
(Assignee)

Comment 43

3 years ago
Comment on attachment 8646339 [details]
MozReview Request: Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

WidgetWheelEvents that are not handled by APZ include those used for zooming, and WHEEL_START / WHEEL_STOP.
(Assignee)

Comment 44

3 years ago
Comment on attachment 8646340 [details]
MozReview Request: Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

When scrolling an inactive subframe, the target APZC confirmation needs to be sent along with the layers transaction. If a new wheel event comes in while that target APZC notification is in-flight, the subframe will already have a display port, so the confirmation will be sent in a more direct fashion and can arrive at the APZ controller thread before the new APZC for the scroll frame has been created. Then the current input block will have a null target APZC and no scrolling will happen until a new input block is created. (For wheel transactions, a null target APZC ends the transaction, so the next event will create a new input block.)
(Assignee)

Comment 45

3 years ago
Comment on attachment 8646341 [details]
MozReview Request: Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

This was discussed in bug 1107716.
(Assignee)

Comment 46

3 years ago
Comment on attachment 8646342 [details]
MozReview Request: Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

We are going to create WidgetWheelEvents from them and need more information than what they currently have.
(Assignee)

Comment 47

3 years ago
Comment on attachment 8646343 [details]
MozReview Request: Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

It shouldn't be checking the w coordinate of the difference of two Point4Ds. The 4D difference isn't really meaningful. Instead, it should be checking each point's w coordinate individually.
(Assignee)

Comment 48

3 years ago
Comment on attachment 8646344 [details]
MozReview Request: Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

PanGestureInput events need to be sent to the pre-scroll position, just like wheel events.
(Assignee)

Comment 49

3 years ago
Comment on attachment 8646345 [details]
MozReview Request: Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats

Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats
(Assignee)

Comment 50

3 years ago
Comment on attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats
Attachment #8646346 - Attachment description: MozReview Request: Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats → MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats
Attachment #8646346 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 51

3 years ago
Created attachment 8646626 [details]
MozReview Request: Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats

Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats
Attachment #8646626 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 52

3 years ago
Comment on attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

Bug 1193062 - Add PanGestureBlockState. r?kats
(Assignee)

Comment 53

3 years ago
Comment on attachment 8646348 [details]
MozReview Request: Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats

Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats
Attachment #8646348 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 54

3 years ago
Comment on attachment 8646349 [details]
MozReview Request: Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

This will make bug 1156606 unnecessary.
(Assignee)

Comment 55

3 years ago
Comment on attachment 8646350 [details]
MozReview Request: Bug 1193062 - Remove mPanGestureState. r?kats

Bug 1193062 - Remove mPanGestureState. r?kats

Now that PanGestureInput events are only processed during a PanGestureBlock, we can get rid of the mPanGestureState workaround.
(Assignee)

Comment 56

3 years ago
Comment on attachment 8646351 [details]
MozReview Request: Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats

Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats
(Assignee)

Comment 57

3 years ago
Comment on attachment 8646352 [details]
MozReview Request: Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

We don't want to split native NSEvents into two PanGestureInput events.
(Assignee)

Comment 58

3 years ago
Comment on attachment 8646353 [details]
MozReview Request: Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats

Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats
(Assignee)

Comment 59

3 years ago
Comment on attachment 8646354 [details]
MozReview Request: Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud

Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud
(Assignee)

Comment 60

3 years ago
Comment on attachment 8646355 [details]
MozReview Request: Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

NSEvent timestamps are relative to the system start time, not to the reference date.
(Assignee)

Comment 61

3 years ago
Comment on attachment 8646356 [details]
MozReview Request: Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud

Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud
Comment on attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

https://reviewboard.mozilla.org/r/15687/#review14165

LGTM, not sure if DOM people need to review also.
Attachment #8646338 - Flags: review?(bugmail.mozilla) → review+
https://reviewboard.mozilla.org/r/15691/#review14167

::: gfx/layers/apz/util/APZCCallbackHelper.h:170
(Diff revision 2)
> +protected:

Realized that we can make this private rather than protected, which is a little better.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #24)
> ::: widget/InputData.cpp:248
> (Diff revision 1)
> > +  wheelEvent.refPoint = RoundedToInt(mPanStartPoint * ScreenToLayoutDeviceScale(1));
> 
> I'd prefer wheelEvent.refPoint =
> RoundedToInt(ViewAs<LayoutDevicePixel>(mPanStartPoint,
> PixelCastJustification::LayoutDeviceToScreenForUntransformedEvent))
> 

I noticed that this bit wasn't addressed in the updated patches.
https://reviewboard.mozilla.org/r/15703/#review14179

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1497
(Diff revision 2)
>    if (mY.CanScroll(aDelta.y) && mFrameMetrics.AllowVerticalScrollWithWheel()) {

Could you please change Axis::CanScroll() to take a ParentLayerCoord rather than a double?
Comment on attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

https://reviewboard.mozilla.org/r/15703/#review14259

I mentioned this to mstange on IRC but for posterity I'll put it here too in condensed form:

- In GetScrollWheelForDelta, the line and page scroll amounts should be converted from LD to screen pixels by multiplying by GetZoom() / GetDevPixelsPerCSSPixel(). If it's a line delta, multiply by aEvent.mDelta[XY] to get the ScreenPixel delta; if it's a pixel delta, just use aEvent.mDelta[XY] directly. Convert to ParentLayerPoint by using ToParentLayerCoordinates with aEvent.mOrigin as the anchor for the return value.
- APZC::CanScroll(InputData) can just use -ToParentLayerCoordinates(aEvent.mPanDisplacement, aEvent.mPanStartPoint) for computing the delta in the case of a pan gesture input.
- The comment for mDelta[XY] in InputData.h should say that for pixel deltas they are in ScreenPixel units

Also clearing review flags on these remaining patches for now; Markus said he would upload new patches with this addressed.
Attachment #8646346 - Flags: review?(bugmail.mozilla)
Comment on attachment 8646348 [details]
MozReview Request: Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats

https://reviewboard.mozilla.org/r/15707/#review14261
Attachment #8646348 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646626 [details]
MozReview Request: Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats

Dropping this review request for now, see previous comment.
Attachment #8646626 - Flags: review?(bugmail.mozilla)
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #66)
> - In GetScrollWheelForDelta, the line and page scroll amounts should be
> converted from LD to screen pixels by multiplying by GetZoom() /
> GetDevPixelsPerCSSPixel().

Something feels off there, as GetZoom() is a CSSToParentLayerScale, so the result of that multiplication would be ParentLayer pixels.
Yeah that's a good point. We can discuss it when Markus gets back from PTO.
Comment on attachment 8646355 [details]
MozReview Request: Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

https://reviewboard.mozilla.org/r/15721/#review14525

Good catch!
Attachment #8646355 - Flags: review?(smichaud) → review+
Comment on attachment 8646354 [details]
MozReview Request: Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud

https://reviewboard.mozilla.org/r/15719/#review14527

Ship It!
Attachment #8646354 - Flags: review?(smichaud) → review+
Comment on attachment 8646356 [details]
MozReview Request: Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud

https://reviewboard.mozilla.org/r/15723/#review14713

This looks reasonable to me, though I haven't tested it.
Attachment #8646356 - Flags: review?(smichaud) → review+
(Assignee)

Comment 74

3 years ago
Comment on attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

These events are used to show / hide scrollbars before the fingers have moved. This is done from default handling in EventStateManager which is skipped when the events are marked with mHandledByAPZ.
Attachment #8646338 - Attachment description: MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats → MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki
Attachment #8646338 - Flags: review?(masayuki)
(Assignee)

Comment 75

3 years ago
Comment on attachment 8646339 [details]
MozReview Request: Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

WidgetWheelEvents that are not handled by APZ include those used for zooming, and WHEEL_START / WHEEL_STOP.
(Assignee)

Comment 76

3 years ago
Created attachment 8652694 [details]
MozReview Request: Bug 1193062 - Only send a content response for wheel events that have also been processed by APZ. r?kats

Bug 1193062 - Only send a content response for wheel events that have also been processed by APZ. r?kats
Attachment #8652694 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 77

3 years ago
Comment on attachment 8646340 [details]
MozReview Request: Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

When scrolling an inactive subframe, the target APZC confirmation needs to be sent along with the layers transaction. If a new wheel event comes in while that target APZC notification is in-flight, the subframe will already have a display port, so the confirmation will be sent in a more direct fashion and can arrive at the APZ controller thread before the new APZC for the scroll frame has been created. Then the current input block will have a null target APZC and no scrolling will happen until a new input block is created. (For wheel transactions, a null target APZC ends the transaction, so the next event will create a new input block.)
(Assignee)

Comment 78

3 years ago
Comment on attachment 8646341 [details]
MozReview Request: Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

This was discussed in bug 1107716.
(Assignee)

Comment 79

3 years ago
Comment on attachment 8646342 [details]
MozReview Request: Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

We are going to create WidgetWheelEvents from them and need more information than what they currently have.
(Assignee)

Comment 80

3 years ago
Comment on attachment 8646343 [details]
MozReview Request: Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

It shouldn't be checking the w coordinate of the difference of two Point4Ds. The 4D difference isn't really meaningful. Instead, it should be checking each point's w coordinate individually.
(Assignee)

Comment 81

3 years ago
Comment on attachment 8646344 [details]
MozReview Request: Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

PanGestureInput events need to be sent to the pre-scroll position, just like wheel events.
(Assignee)

Comment 82

3 years ago
Comment on attachment 8646345 [details]
MozReview Request: Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats

Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats
(Assignee)

Comment 83

3 years ago
Comment on attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats
Attachment #8646346 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 84

3 years ago
Comment on attachment 8646626 [details]
MozReview Request: Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats

Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats
Attachment #8646626 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 85

3 years ago
Comment on attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

Bug 1193062 - Add PanGestureBlockState. r?kats
(Assignee)

Comment 86

3 years ago
Comment on attachment 8646348 [details]
MozReview Request: Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats

Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats
(Assignee)

Comment 87

3 years ago
Comment on attachment 8646349 [details]
MozReview Request: Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

This will make bug 1156606 unnecessary.
(Assignee)

Comment 88

3 years ago
Comment on attachment 8646350 [details]
MozReview Request: Bug 1193062 - Remove mPanGestureState. r?kats

Bug 1193062 - Remove mPanGestureState. r?kats

Now that PanGestureInput events are only processed during a PanGestureBlock, we can get rid of the mPanGestureState workaround.
(Assignee)

Comment 89

3 years ago
Comment on attachment 8646351 [details]
MozReview Request: Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats

Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats
(Assignee)

Comment 90

3 years ago
Comment on attachment 8646352 [details]
MozReview Request: Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

We don't want to split native NSEvents into two PanGestureInput events.
(Assignee)

Comment 91

3 years ago
Comment on attachment 8646353 [details]
MozReview Request: Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats

Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats
(Assignee)

Comment 92

3 years ago
Comment on attachment 8646354 [details]
MozReview Request: Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud

Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud
(Assignee)

Comment 93

3 years ago
Comment on attachment 8646355 [details]
MozReview Request: Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

NSEvent timestamps are relative to the system start time, not to the reference date.
(Assignee)

Comment 94

3 years ago
Comment on attachment 8646356 [details]
MozReview Request: Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud

Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud
Comment on attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

https://reviewboard.mozilla.org/r/15687/#review15335

Although, the original line is over 80 characters, keeping current style is okay.
Attachment #8646338 - Flags: review?(masayuki) → review+
Comment on attachment 8652694 [details]
MozReview Request: Bug 1193062 - Only send a content response for wheel events that have also been processed by APZ. r?kats

https://reviewboard.mozilla.org/r/17249/#review15349
Attachment #8652694 - Flags: review?(bugmail.mozilla) → review+
For part 5, s/_APZ// in the commit message.
Comment on attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

https://reviewboard.mozilla.org/r/15703/#review15351

Nice work!

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1437
(Diff revision 3)
> +  ParentLayerPoint scrollAmount;

Not sure why you made these ParentLayerPoint instead of ParentLayerSize. The later would be more appropriate and also simplify the computation slightly, I think.

::: gfx/layers/apz/src/AsyncPanZoomController.cpp:1600
(Diff revision 3)
>        // Cast velocity from ParentLayerPoints/ms to CSSPoints/ms then convert to

Comment needs updating
Attachment #8646346 - Flags: review?(bugmail.mozilla) → review+
Attachment #8646626 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8646626 [details]
MozReview Request: Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats

https://reviewboard.mozilla.org/r/15817/#review15353

Ship It!
(Assignee)

Comment 100

3 years ago
https://reviewboard.mozilla.org/r/15703/#review15351

> Not sure why you made these ParentLayerPoint instead of ParentLayerSize. The later would be more appropriate and also simplify the computation slightly, I think.

Oops, I think I assumed that operator/ and * were only defined for points, not for sizes. Not sure why I thought that.
Comment on attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

https://reviewboard.mozilla.org/r/15705/#review15357

::: gfx/layers/apz/src/InputBlockState.cpp:484
(Diff revision 3)
> +    GetTargetApzc()->HandleInputEvent(event, mTransformToApzc);

This needs rebasing; it ahould call DispatchEvent() which I added recently.
Attachment #8646347 - Flags: review+
(Assignee)

Comment 102

3 years ago
Comment on attachment 8646338 [details]
MozReview Request: Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r?kats, r?masayuki

These events are used to show / hide scrollbars before the fingers have moved. This is done from default handling in EventStateManager which is skipped when the events are marked with mHandledByAPZ.
(Assignee)

Comment 103

3 years ago
Comment on attachment 8646339 [details]
MozReview Request: Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r?kats

WidgetWheelEvents that are not handled by APZ include those used for zooming, and WHEEL_START / WHEEL_STOP.
(Assignee)

Comment 104

3 years ago
Comment on attachment 8652694 [details]
MozReview Request: Bug 1193062 - Only send a content response for wheel events that have also been processed by APZ. r?kats

Bug 1193062 - Only send a content response for wheel events that have also been processed by APZ. r?kats
(Assignee)

Comment 105

3 years ago
Comment on attachment 8646340 [details]
MozReview Request: Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r?kats

When scrolling an inactive subframe, the target APZC confirmation needs to be sent along with the layers transaction. If a new wheel event comes in while that target APZC notification is in-flight, the subframe will already have a display port, so the confirmation will be sent in a more direct fashion and can arrive at the APZ controller thread before the new APZC for the scroll frame has been created. Then the current input block will have a null target APZC and no scrolling will happen until a new input block is created. (For wheel transactions, a null target APZC ends the transaction, so the next event will create a new input block.)
(Assignee)

Comment 106

3 years ago
Comment on attachment 8646341 [details]
MozReview Request: Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

Bug 1193062 - Add a PAN_MOMENTUM APZ state. r?kats

This was discussed in bug 1107716.
(Assignee)

Comment 107

3 years ago
Comment on attachment 8646342 [details]
MozReview Request: Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r?kats

We are going to create WidgetWheelEvents from them and need more information than what they currently have.
(Assignee)

Comment 108

3 years ago
Comment on attachment 8646343 [details]
MozReview Request: Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

Bug 1193062 - Fix UntransformVector w coordinate checks. r?kip

It shouldn't be checking the w coordinate of the difference of two Point4Ds. The 4D difference isn't really meaningful. Instead, it should be checking each point's w coordinate individually.
(Assignee)

Comment 109

3 years ago
Comment on attachment 8646344 [details]
MozReview Request: Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r?kats

PanGestureInput events need to be sent to the pre-scroll position, just like wheel events.
(Assignee)

Comment 110

3 years ago
Comment on attachment 8646345 [details]
MozReview Request: Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats

Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r?kats
(Assignee)

Comment 111

3 years ago
Comment on attachment 8646346 [details]
MozReview Request: Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats

Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r?kats
(Assignee)

Comment 112

3 years ago
Comment on attachment 8646626 [details]
MozReview Request: Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats

Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r?kats
(Assignee)

Updated

3 years ago
Attachment #8646347 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 113

3 years ago
Comment on attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

Bug 1193062 - Add PanGestureBlockState. r?kats
(Assignee)

Comment 114

3 years ago
Comment on attachment 8646348 [details]
MozReview Request: Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats

Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r?kats
(Assignee)

Comment 115

3 years ago
Comment on attachment 8646349 [details]
MozReview Request: Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r?kats

This will make bug 1156606 unnecessary.
(Assignee)

Comment 116

3 years ago
Comment on attachment 8646350 [details]
MozReview Request: Bug 1193062 - Remove mPanGestureState. r?kats

Bug 1193062 - Remove mPanGestureState. r?kats

Now that PanGestureInput events are only processed during a PanGestureBlock, we can get rid of the mPanGestureState workaround.
(Assignee)

Comment 117

3 years ago
Comment on attachment 8646351 [details]
MozReview Request: Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats

Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r?kats
(Assignee)

Comment 118

3 years ago
Comment on attachment 8646352 [details]
MozReview Request: Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

Bug 1193062 - Process pan gesture deltas in begin+end events. r?kats

We don't want to split native NSEvents into two PanGestureInput events.
(Assignee)

Comment 119

3 years ago
Comment on attachment 8646353 [details]
MozReview Request: Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats

Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r?kats
(Assignee)

Comment 120

3 years ago
Comment on attachment 8646354 [details]
MozReview Request: Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud

Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r?smichaud
(Assignee)

Comment 121

3 years ago
Comment on attachment 8646355 [details]
MozReview Request: Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r?smichaud

NSEvent timestamps are relative to the system start time, not to the reference date.
(Assignee)

Comment 122

3 years ago
Comment on attachment 8646356 [details]
MozReview Request: Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud

Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r?kats, r?smichaud
Comment on attachment 8646347 [details]
MozReview Request: Bug 1193062 - Add PanGestureBlockState. r?kats

not sure what mozreview did there...
Attachment #8646347 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Comment 124

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a9a922d49c6cd0a3645bd9c9bfbc9125e627e9fb
Bug 1193062 - Make NS_WHEEL_START/STOP events bypass APZ. r=kats, r=masayuki

https://hg.mozilla.org/integration/mozilla-inbound/rev/c5638a1466dbd3aad52b19f33a132796af5457c3
Bug 1193062 - Only send target confirmations for wheel events that were handled by APZ. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/29850f2b5708117911eabe11d6fb6854468dcdc7
Bug 1193062 - Only send a content response for wheel events that have also been processed by APZ. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/895278844d2cf93a9ef9f433c06fa75c30a51f23
Bug 1193062 - Don't double-send target APZC confirmations that might race each other. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/7fa5473a3593793c6efe74bafb125b94f40fd486
Bug 1193062 - Add a PAN_MOMENTUM state. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/6639c69d29ee07ae0ad5b9b1e7ee55bbf93c2b0f
Bug 1193062 - Add fields to PanGestureInput and ScrollWheelInput. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/aec1ea06a1ca87d954864c075ca1ecc7fbc007d1
Bug 1193062 - Fix UntransformVector w coordinate checks. r=kip

https://hg.mozilla.org/integration/mozilla-inbound/rev/448be9e44e89acb17cf2150981cfb3c2d9359d36
Bug 1193062 - Make PanGestureInput transform processing work like ScrollWheelInput processing. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/5aabf8e26779d828f3d775586736a10e9b46c65b
Bug 1193062 - Set correct axis velocities when using PanGestureInput events. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/2eef76258ff51cbfda420612c6108cbc111bbb74
Bug 1193062 - CanScrollWithWheel needs to use ParentLayerCoords for the scroll delta. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/4178531f85b2d539eb2da75e886628320ac33729
Bug 1193062 - Make OverscrollHandoffChain::FindFirstScrollable and AsyncPanZoomController::CanScroll able to deal with PanGestureInput events. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f3585512590ebef256e09219f9a12efdce28891
Bug 1193062 - Add PanGestureBlockState. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/b2a0c3de54336e4cd2f69b58404839438a513968
Bug 1193062 - Make AllowScrollHandoff work for both ScrollWheelInput and PanGestureInput blocks. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/5109d2920b9638b547c174684f0cc817e3776f45
Bug 1193062 - Don't use PanGestureInput events for instant wheel scrolling. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/78e2467eff0bef54d54b4894dc65039eda2e707e
Bug 1193062 - Remove mPanGestureState. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/73542be5e88030a35d085fbc93cb65390c6e388b
Bug 1193062 - Use ScrollSource::Wheel for pan gesture events. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/4aac3d312e5df5b25f2218b68f4b3dd42db8917e
Bug 1193062 - Process pan gesture deltas in begin+end events. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/529d7e708472f0f10948f66888e9602fa4e23764
Bug 1193062 - Add mHandledByAPZ on PanGestureInput and ScrollWheelInput, and sync the information to the WidgetWheelEvent. r=kats

https://hg.mozilla.org/integration/mozilla-inbound/rev/853c265bf5c5f343066f97d531809c395ec48bea
Bug 1193062 - Add nsCocoaUtils::ModifiersForEvent. r=smichaud

https://hg.mozilla.org/integration/mozilla-inbound/rev/11bee7112108523af59da545dc44ddbd1aee6d49
Bug 1193062 - Give synthesized NSEvents a timestamp that is in the right space. r=smichaud

https://hg.mozilla.org/integration/mozilla-inbound/rev/b24bf166441b9dae774dd92c8187f2453091c4ce
Bug 1193062 - Make nsChildView send PanGestureInput events into APZ. r=kats, r=smichaud
You need to log in before you can comment on or make changes to this bug.