Closed
Bug 1083530
Opened 9 years ago
Closed 9 years ago
Refactor GeckoTouchDispatcher to use mozilla::TimeStamp instead of nsecs_t for vsync times
Categories
(Core Graveyard :: Widget: Gonk, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla36
People
(Reporter: mchang, Assigned: mchang)
References
Details
Attachments
(2 files, 5 obsolete files)
1014 bytes,
patch
|
mchang
:
review+
|
Details | Diff | Splinter Review |
23.14 KB,
patch
|
mwu
:
review+
|
Details | Diff | Splinter Review |
+++ This bug was initially created as a clone of Bug #1069037 +++ For Project Silk, we'd like to use 1 timestamp and pass it around to the three components required to listen to vsync: The compositor, touch input, and refresh driver. The compositor and refresh driver both use the mozilla::TimeStamp as scheduling mechanisms for animations and requestAnimationFrames. Instead of having the VsyncFramework pass two timestamps, the android input vsync timestamp and the mozilla::TimeStamp around, let's simplify the framework so we can just use the mozilla::TImeStamp. The only user of the android nsecs_t timestamp is the GeckoTouchDispatcher. Change it to use mozilla::TimeStamps.
Assignee | ||
Comment 1•9 years ago
|
||
Mostly a global rename from int64_t to mozilla::Timestamp. No logic changes in resampling, that will come in another bug. I sync android nsecs_t time and mozilla::TimeStamp in both the HwcComposer and GeckoInputDispatcher so that we can have mozilla::Timestamps that are aligned with andriod nsecs_t times.
Attachment #8505834 -
Flags: review?(mwu)
Assignee | ||
Comment 2•9 years ago
|
||
Rebased now that bug 1069037 landed. No logic changes, just mostly renaming nsecs_t to TimeStamps.
Attachment #8505834 -
Attachment is obsolete: true
Attachment #8505834 -
Flags: review?(mwu)
Attachment #8507992 -
Flags: review?(mwu)
Assignee | ||
Comment 3•9 years ago
|
||
Rebased on master. Also fixed a small bug in the VsyncDispatcher to ensure that we correctly notify the GeckoTouchDispatcher.
Attachment #8507992 -
Attachment is obsolete: true
Attachment #8507992 -
Flags: review?(mwu)
Attachment #8511149 -
Flags: review?(mwu)
Comment 4•9 years ago
|
||
Comment on attachment 8511149 [details] [diff] [review] Use mozilla::TimeStamp in GeckoTouchDispatcher instead of nsecs_t Review of attachment 8511149 [details] [diff] [review]: ----------------------------------------------------------------- I'm OK with switching to TimeStamp, but don't pretend that the moz timestamp and system timestamp are different things. You can do a sanity check to make sure they're close enough, but don't try to translate one to another - just convert the system time into a moz timestamp.
Attachment #8511149 -
Flags: review?(mwu)
Assignee | ||
Comment 5•9 years ago
|
||
Hi Roc, :mwu thinks it would be better to enable mozilla::TimeStamp to be created from the android system time since they both use clock_gettime and are actually the same time systems. This patch enables the creation of a mozilla::TimeStamp from an android system time on b2g. The TimeStamp.h comment says this type of behavior is explicitly disabled (http://dxr.mozilla.org/mozilla-central/source/xpcom/ds/TimeStamp.h#363), but can we make an exception for this since they are both the same units? Thanks!
Attachment #8512872 -
Flags: review?(roc)
Attachment #8512872 -
Flags: review?(roc) → review+
Comment 6•9 years ago
|
||
Comment on attachment 8512872 [details] [diff] [review] Create mozilla::TimeStamp constructor with system time argument Review of attachment 8512872 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/ds/TimeStamp.h @@ +389,5 @@ > > +#ifdef MOZ_WIDGET_GONK > + TimeStamp(int64_t aAndroidTime) : mValue(aAndroidTime) > + { > + MOZ_RELEASE_ASSERT(sizeof(aAndroidTime) == sizeof(TimeStampValue)); I think you want MOZ_STATIC_ASSERT here.
Assignee | ||
Comment 7•9 years ago
|
||
Fixed to create mozilla::TimeStamps based off android system times.
Attachment #8511149 -
Attachment is obsolete: true
Attachment #8512994 -
Flags: review?(mwu)
Assignee | ||
Updated•9 years ago
|
Attachment #8512994 -
Attachment description: Create mozilla::TimeStamp constructor with system time argument → Refactor GeckoTouchDispatcher to use mozilla::TimeStamp instead of nsecs_t for vsync times
Assignee | ||
Comment 8•9 years ago
|
||
Carrying r+. Use static_assert, MOZ_STATIC_ASSERT only works in C code.
Attachment #8512872 -
Attachment is obsolete: true
Attachment #8512995 -
Flags: review+
Comment 9•9 years ago
|
||
Comment on attachment 8512994 [details] [diff] [review] Refactor GeckoTouchDispatcher to use mozilla::TimeStamp instead of nsecs_t for vsync times Review of attachment 8512994 [details] [diff] [review]: ----------------------------------------------------------------- ::: widget/gonk/GeckoTouchDispatcher.cpp @@ +320,5 @@ > } > > // This has to be signed int since it is negative > + int64_t frameDiff = (currentTouchTime - aSampleTime).ToMicroseconds(); > + ResampleTouch(aOutTouch, currentTouch, prevTouch, frameDiff, mTouchTimeDiff.ToMicroseconds(), false); Why not also switch ResampleTouch and Interpolate to using TouchDuration? Then we don't need to convert here. ::: widget/gonk/HwcComposer2D.cpp @@ +74,5 @@ > > namespace mozilla { > > #if ANDROID_VERSION >= 17 > nsecs_t sAndroidInitTime = 0; This can be removed now, right? @@ +233,5 @@ > } > } > } > > +nsecs_t sLastVsyncTime; Make this a member of HwcComposer2D.
Attachment #8512994 -
Flags: review?(mwu)
Assignee | ||
Comment 10•9 years ago
|
||
Updated with comment 9.
Attachment #8512994 -
Attachment is obsolete: true
Attachment #8513030 -
Flags: review?(mwu)
Updated•9 years ago
|
Attachment #8513030 -
Flags: review?(mwu) → review+
Assignee | ||
Comment 11•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/907d4f91d07c https://hg.mozilla.org/integration/b2g-inbound/rev/a9748c48d9bd
Comment 12•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/907d4f91d07c https://hg.mozilla.org/mozilla-central/rev/a9748c48d9bd
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Updated•5 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•