Closed Bug 1087048 Opened 8 years ago Closed 8 years ago

Dispatch Touch Resamples after a Vsync Aligned Composite finishes.

Categories

(Core Graveyard :: Widget: Gonk, defect)

26 Branch
x86
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: mchang, Assigned: mchang)

References

Details

Attachments

(1 file, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1083530 +++

Until we can get bug 930939, we should dispatch touch events after composites. At the moment, we have a race condition between touch events and composites, especially if we dispatch touches resamples and composites at vsync times. This causes very janky behavior, much jankier than having just vsync aligned composites or vsync aligned touch resampling. In practice, without vsync alignment, there is usually a touch | composite | touch | composite ordering. 

This patch changes it so that touch resamples are dispatched AFTER vsync aligned composites so we have a strict ordering. Once bug 930939 comes around, we can switch and touches can come BEFORE composites. Note*, this patch is fixed so that it builds today, but the commented out portion to use a mozilla::TimeStamp will be the real code once bug 1083530 lands.
Attachment #8509099 - Flags: review?(bgirard)
Comment on attachment 8509099 [details] [diff] [review]
Dispatch Touch Events After Composites

Review of attachment 8509099 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/layers/ipc/CompositorParent.cpp
@@ +310,5 @@
> +CompositorVsyncObserver::DispatchTouchEvents(TimeStamp aVsyncTimestamp)
> +{
> +#ifdef MOZ_WIDGET_GONK
> +  if (gfxPrefs::TouchResampling()) {
> +    //GeckoTouchDispatcher::NotifyVsync(aVsyncTimestamp);

Is this a TODO? If so it should have a comment.
(In reply to Benoit Girard (:BenWa) from comment #1)
> Comment on attachment 8509099 [details] [diff] [review]
> Dispatch Touch Events After Composites
> 
> Review of attachment 8509099 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: gfx/layers/ipc/CompositorParent.cpp
> @@ +310,5 @@
> > +CompositorVsyncObserver::DispatchTouchEvents(TimeStamp aVsyncTimestamp)
> > +{
> > +#ifdef MOZ_WIDGET_GONK
> > +  if (gfxPrefs::TouchResampling()) {
> > +    //GeckoTouchDispatcher::NotifyVsync(aVsyncTimestamp);
> 
> Is this a TODO? If so it should have a comment.

I won't land this until bug 1083530 lands, at which point this will become the real code. Just trying to pipeline the reviews. If you prefer to wait until bug 1083530 lands, that will be ok too.
Attachment #8509099 - Flags: review?(bgirard) → review+
Carrying r+. Rebased on master and on patches on bugs 1083530 and 1085512.
Attachment #8509099 - Attachment is obsolete: true
Attachment #8511183 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/02e93e0c09de
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Blocks: 1094059
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.