-
Notifications
You must be signed in to change notification settings - Fork 30
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
kinetic-scroll-view: Various fixes for the motion buffer #98
Open
pwithnall
wants to merge
6
commits into
clutter-project:master
Choose a base branch
from
pwithnall:kinetic-scrolling
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pwithnall
force-pushed
the
kinetic-scrolling
branch
from
September 21, 2015 17:21
a78bd7d
to
7eee516
Compare
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
The former is always emitted when the timeline stops (either due to reaching its duration, or due to clutter_timeline_stop() being called); the latter is only emitted in the first case. Changing signals means we can eliminate manual cleanup code after calls to clutter_timeline_stop(). clutter-project#98
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
The motion buffer contains a history of motion events for the current scrolling action, starting from the initial touch event. This is used at the release of the scroll to work out an average origin coordinate, which is then used in the calculation of whether to use kinetic scrolling. Previously, the oldest entry in the motion buffer was removed if the buffer filled up, then the buffer was expanded in size. This meant that for scrolling actions with many motion events, the average taken over the events would quickly creep towards the location of the most recent motion event. This meant that, often, the threshold for starting a kinetic scroll was not reached, and hence kinetic scrolling would not happen. Stop removing the oldest entry from the buffer, and instead grow it indefinitely. This does make the widget prone to consuming all the system’s memory if the user scrolls around for a _very_ long time. This may be better fixed by calculating a moving average instead, and eliminating the motion buffer. clutter-project#98
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
The motion buffer stored each motion event which happened in a sequence (for example, a single swipe) on the kinetic scroll view, in order that the average event coordinates and time could be computed at the end of the sequence (on releasing the touchscreen). This could lead to problems with very long event sequences — potentially allocating a huge event array and leading to allocation failure. Avoid that by computing the average of the events as they are received. clutter-project#98
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
GTimeVal is a pain to use, microsecond precision is not necessary for tracking motion events (millisecond precision is fine), and the use of glong means the potential for overflow when summing several timestamps on 32-bit platforms is quite high. Instead, use gint64 and g_get_real_time(). clutter-project#98
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
This makes debugging problems in the MxKineticScrollView a lot easier. Note that _KINETIC_DEBUG must be defined to enable debug messages. clutter-project#98
pwithnall
force-pushed
the
kinetic-scrolling
branch
from
September 21, 2015 17:22
7eee516
to
7084cb0
Compare
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
GTimeVal is a pain to use and the use of glong means the potential for overflow when summing several timestamps on 32-bit platforms is quite high. Instead, use gint64 and g_get_real_time(). clutter-project#98
pwithnall
pushed a commit
to pwithnall/mx
that referenced
this pull request
Sep 21, 2015
This makes debugging problems in the MxKineticScrollView a lot easier. Note that _KINETIC_DEBUG must be defined to enable debug messages. clutter-project#98
The former is always emitted when the timeline stops (either due to reaching its duration, or due to clutter_timeline_stop() being called); the latter is only emitted in the first case. Changing signals means we can eliminate manual cleanup code after calls to clutter_timeline_stop(). clutter-project#98
The motion buffer contains a history of motion events for the current scrolling action, starting from the initial touch event. This is used at the release of the scroll to work out an average origin coordinate, which is then used in the calculation of whether to use kinetic scrolling. Previously, the oldest entry in the motion buffer was removed if the buffer filled up, then the buffer was expanded in size. This meant that for scrolling actions with many motion events, the average taken over the events would quickly creep towards the location of the most recent motion event. This meant that, often, the threshold for starting a kinetic scroll was not reached, and hence kinetic scrolling would not happen. Stop removing the oldest entry from the buffer, and instead grow it indefinitely. This does make the widget prone to consuming all the system’s memory if the user scrolls around for a _very_ long time. This may be better fixed by calculating a moving average instead, and eliminating the motion buffer. clutter-project#98
The motion buffer stored each motion event which happened in a sequence (for example, a single swipe) on the kinetic scroll view, in order that the average event coordinates and time could be computed at the end of the sequence (on releasing the touchscreen). This could lead to problems with very long event sequences — potentially allocating a huge event array and leading to allocation failure. Avoid that by computing the average of the events as they are received. clutter-project#98
GTimeVal is a pain to use and the use of glong means the potential for overflow when summing several timestamps on 32-bit platforms is quite high. Instead, use gint64 and g_get_real_time(). clutter-project#98
This makes debugging problems in the MxKineticScrollView a lot easier. Note that _KINETIC_DEBUG must be defined to enable debug messages. clutter-project#98
pwithnall
force-pushed
the
kinetic-scrolling
branch
from
September 22, 2015 08:46
7084cb0
to
60fc20e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Various fixes made in the course of investigating a problem where kinetic scrolling would not work due to the frame in which the touchscreen was released taking 400ms to complete — this was because the scroll view had many rows, and the
MxStylable::style-changed
signal propagation to all of them was taking forever.Anyway, these fixes should improve the behaviour of the motion buffer. Instead of dropping the earliest buffer entries when the buffer fills up, it eliminates the buffer entirely and calculates a moving average of the motion events. There are also some debug message improvements.