Encode scroll motion events in the Android touch processor #8149
Conversation
| boolean isPointerEvent = Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR2 | ||
| && event.isFromSource(InputDevice.SOURCE_CLASS_POINTER); | ||
| if (!isPointerEvent || event.getActionMasked() != MotionEvent.ACTION_HOVER_MOVE) { | ||
| if (!(isPointerEvent && |
There was a problem hiding this comment.
Can we pull this conditional into another boolean variable? I'm not clear on what this condition signifies. I'm wondering what it means if its not a pointer event but it is a hover move. I'm also wondering what it affirmatively means to be a pointer + hover move, or a pointer + scroll.
| int pointerKind = getPointerDeviceTypeForToolType(event.getToolType(pointerIndex)); | ||
|
|
||
| int signalKind = PointerSignalKind.NONE; | ||
| int signalKind = event.getActionMasked() == MotionEvent.ACTION_SCROLL ? |
There was a problem hiding this comment.
Stylistically, can we either keep this on one line, or otherwise break at the "?" and ":"
int signalKind = event.getActionMasked() == MotionEvent.ACTION_SCROLL
? PointerSignalKind.SCROLL
: PointerSignalKind.NONE;| if (maskedAction == MotionEvent.ACTION_CANCEL) { | ||
| return PointerChange.CANCEL; | ||
| } | ||
| if (maskedAction == MotionEvent.ACTION_SCROLL) { |
There was a problem hiding this comment.
What's the relationship between scrolling and hovering? They seem to be showing up in pairs...
There was a problem hiding this comment.
The iOS embedding looked like it was encoding scroll events as (PointerChange=hover, PointerSignal=scroll), so I duplicated that.
fwiw, it looks like the PointerEventConverter logic in the framework will consider any event with PointerSignal=scroll to be a scroll event regardless of the PointerChange type.
There was a problem hiding this comment.
The intent is that discrete scroll events use hover, while trackpad gesture scrolling (on platforms that support it as something other than just a sequence of individual scrolls, such as macOS) would use down/move/up sequences. Only the former is implemented in the framework currently, which is why it doesn't check the type, but in the future it will (unless that design changes).
There was a problem hiding this comment.
@stuartmorgan is that documented anywhere? If not, can we document here or in an appropriate place that is referenced from here (not holding up this PR for that).
There was a problem hiding this comment.
It's not currently documented.
Documenting it sounds good; suggestions on an appropriate place? It seems like there should really be a document about pointer event streams in general, but if there is one I'm not sure where. (E.g., the only reason I know the distinction between kMove and kHover in general, or what kAdd and kRemove are for, is because I've read the converter code.) Perhaps this falls under the bug about documenting the embedding API in general?
There was a problem hiding this comment.
A thorough explanation in a Wiki doc would probably be good. I would then try to provide surrounding context and basic explanation in the code, and then link to the full doc from those comments. Generally speaking, I try to convey as much context in javadoc comments as possible because leaving the IDE for a website is a distraction if it can be avoided.
jason-simmons
force-pushed
the
mouse_scroll_event
branch
from
e92af37 to
cb45918
Compare
* 59715b7 Disable build_ios task due to lack of credits. (flutter/engine#8150) * 9f50dae Look up ICU symbols based on the path to libflutter.so as a fallback (flutter/engine#8139) * 8b1a299 [Skia] Rollback Skia to 29d5dec9a0783a033b921dc483fb98d565d684f6 (flutter/engine#8151) * 8be2aca Roll src/third_party/dart 1bd36d694d..674fd0e060 (48 commits) #8152 * 2e42703 Revert "Disable build_ios task due to lack of credits. (#8150)" (flutter/engine#8153) * 2daebeb Fix text.dart height docs (flutter/engine#8079) * b1b388f Encode scroll motion events in the Android touch processor (flutter/engine#8149)
…lutter#8149)" This reverts commit 417753e.
…lutter#8149)" This reverts commit 417753e.
4 participants
No description provided.