move resampler to handlePointerEvent and fix complex_layout_android__scroll_smoothness with PointerEvent#66745
move resampler to handlePointerEvent and fix complex_layout_android__scroll_smoothness with PointerEvent#66745fluttergithubbot merged 4 commits intoflutter:masterfrom CareF:relocate_resampler
Conversation
flutter-dashboard
bot
added
a: tests
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat. Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
flutter-dashboard
bot
added
the
c: contributor-productivity
|
No extra test is necessary because we already prepared the test that fits both current solution and this one in #64424 |
There was a problem hiding this comment.
I'm still feeling this is a workaround, see also #60558 (comment)
This is a public method because it's needed in WidgetTester.
There was a problem hiding this comment.
why is this needed in WidgetTester? If needed then maybe this is find but ideally the name would just be "sample()" instead of triggerResample.
There was a problem hiding this comment.
why is this needed in WidgetTester? If needed then maybe this is find but ideally the name would just be "sample()" instead of triggerResample.
Because triggerResample is called in _flushPointerEventQueue, which is before the input injection in WidgetTester. triggerResample is in _flushPointerEventQueue because otherwise sample is not properly called: when _pendingPointerEvents is empty, we still need to call _resampler.sample for updated resampled event (see #60558 (comment)) @dreveman
There was a problem hiding this comment.
As for the name, I'm feeling binding.sample is confusing because binding is not the subject of sample...
| void _handleSampleTimeChanged() { | ||
| if (!locked) { | ||
| _flushPointerEventQueue(); | ||
| _resampler.sample(samplingOffset); |
There was a problem hiding this comment.
What if resamplingEnabled is false here? Should we stop the resampler instead then?
There was a problem hiding this comment.
This can happen when resample was on but is turned off, and the logic should be to just discard the remaining events queued in the resampler? @dreveman
5 participants
Description
This is to move the resampling logic to
handlePointerEvent, so that it's accessible toWidgetTester, as enabled by #64846. This also fixes #66608 and closes #66612 .In terms of fixing #66608, this is an alternative to #66611. #66611 is more close to the solution before #64846, so I suggest to land #66611 and observe for a couple of runs before merge this PR to see if the change makes some difference to the test result.
Checklist
Before you create this PR, confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Did any tests fail when you ran them? Please read Handling breaking changes.