Closed Bug 757680 Opened 8 years ago Closed 7 years ago

Too few touch events dispatched for drawing application

Categories

(Firefox for Android :: General, defect)

14 Branch
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 19
Tracking Status
blocking-fennec1.0 --- -
fennec + ---

People

(Reporter: birtles, Assigned: kats)

References

()

Details

Attachments

(4 files)

We have a drawing application that uses touch events for drawing line art using SVG (see URL).

The touch event handler adds points to an SVG polyline (the smoothing of the polyline into a path takes place on touchend).

On a Toshiba Regza AT200 running Android 3.2 the drawing is smooth when running Fennec 10. However, on Fennec 14 far fewer touch events appear to be dispatched and the intermediate polyline is very jagged and the resulting path is very inaccurate.

If I get a chance I'll try to produce a reduced version of this test.

For now, the source in question is here:
https://github.com/mozilla/parapara/blob/fa8904d949b7cd94fdf56803648971cdc4f47422/editor/lib/js/parapara.js#L321

We've been using this tool with Fennec 10 for a while  (and its been working great, thanks!) and we plan to use it as part of the Mozilla Webmaker Summer Code Party (starting 23 June) and Osaka Firefox Developer Conference (30 June) so I'll do my best to contribute to fixing this soon.
OS: Windows 7 → Android
Hardware: x86_64 → All
CC's wesj and kats since they were so helpful earlier on and I didn't get to post the bug before they disappeared.
Attached file Reduced test case
This is a somewhat reduced version of the application. The difference is not as pronounced as the actual application (which suggests that it may, in part, be related to the complexity of the touch event handlers) but still very noticeable.

Try drawing some curvy lines very quickly first in Fennec 10 on a tablet, then Fennec 14. I see much more jagged lines in Fennec 14 suggesting fewer touch events are being delivered.
What device were you testing on? On the Galaxy Nexus the test case works great.

http://people.mozilla.org/~kgupta/tmp/circle.png
Toshiba Regza AT200 running Android 3.2
(In reply to Kartikaya Gupta (:kats) from comment #3)
> On the Galaxy Nexus the test case works great.

How does the URL on the bug look? That is, the fully-fledged app?
Not bad, it's a little more jagged than the test version but still quite good.

https://people.mozilla.com/~kgupta/tmp/parapara.png
I also tried it on the Galaxy Tab 10.1, which seems very similar in specifications to the Toshiba Regza, and it was noticeably worse there. This is what I ended up with when I tried drawing circles quickly:

https://people.mozilla.com/~kgupta/tmp/parapara-tab.jpg
The simplified test case works much better on the Galaxy Tab.

https://people.mozilla.com/~kgupta/tmp/circle-tab.jpg

Since the Galaxy Tab hardware should be generating events at the same rate for the two pages, I suspect it's just the extra JS computation that's causing the jaggedness on the full version.

The stock browser on the Galaxy Tab does better on both pages than we do; either they are not coalescing touch events, or they are executing scripts faster, or both.
Here's what I'm seeing on the Regza:

Fennec 10, reduced test case:
http://people.mozilla.org/~bbirtles/touch-testing/reduced-f10.jpg

Fennec 14, reduced test case:
http://people.mozilla.org/~bbirtles/touch-testing/reduced-f14.jpg

Fennec 10, full app:
http://people.mozilla.org/~bbirtles/touch-testing/full-f10.jpg

Fennec 14, full app:
http://people.mozilla.org/~bbirtles/touch-testing/full-f14.jpg

As you can see from the last shot, despite drawing the same pattern the result looks completely different. The app is unuseable in this case.

What the next step for tracking down the cause here?
I'd be surprised if the problem is our JS. We're usually on par, if not faster than stock, and should be the same (if not faster) in Native vs. XUL. My first instinct would be to look at the coalescing and see what's going in vs coming out, but kats might have some better ideas.
So event coalescing is definitely what's going on here. I turned on logging for when it happens and a lot of events are getting coalesced. Turning off event coalescing makes the circles much more circular, although there's definitely some noticeable processing lag.

Next step is to see what we're doing that causes the events to get backed up in the queue and coalesced.
It seems like one of the first draw events we run takes a bunch of time (I'm seeing ~180ms in the trace I generated) and during that time 8 touch-move events get coalesced. Later there's another ~180ms draw event that takes out a couple more touch-move events.

I don't see anything particularly abnormal happening here; things are working more or less as we designed them except that the draws are taking a really long time and that degrades the number of touch events the page receives. On most pages this is the right tradeoff, but obviously for a drawing app this is not good.

Next step is probably to profile the draw and see why it's taking so long.
Personally, I'd like to make this a priority before we ship. Drawing apps are common on mobile devices, and I'd like to be good at them.
tracking-fennec: --- → ?
(In reply to Kartikaya Gupta (:kats) from comment #12)
> Next step is probably to profile the draw and see why it's taking so long.

That would be good. Although there are things that can be done to speed up the app in question and SVG in general, I wonder how much they'll help with the delay from the initial draw event.
This is a variant on the original test case.

The original test case uses setAttribute to build up the list of points. As the line gets longer, so does the string and the amount of processing required to add each point. The alternative is to use the SVG DOM and just append points directly to the line. Prior to bug 629200 (which landed in mozilla13), however, the SVG DOM was very slow for this.

This test case makes use of the SVG DOM. It will be somewhat slower in Fennec 10 than the previous test case (due to bug 629200). For Fennec 14, however, it should be slightly faster. In my testing, however, it doesn't make any noticeable difference. I'm including it here simply to eliminate one possible cause of delay, namely, poor usage of the DOM. (Note that even if this were to help, there are still many apps that rely on setAttribute, such as SVG Edit.)
Doh. Meant to flip the blocking flag.
blocking-fennec1.0: --- → ?
tracking-fennec: ? → 15+
blocking-fennec1.0: ? → -
At the Firefox Devcon in Osaka at the end of June (June 30 to be precise) we'd like to get everyone in the audience to fire up Fennec on their smartphones and participate in a group drawing session (using the Parapara animation workshop concept).

This bug is currently blocking us from doing that (unless we tell everyone to install Fennec 10). If there are any workarounds we can implement in JS for this then I'd love to hear them! Thanks!

(And if we can fix this by June 30 I'll hand-deliver as many random Japanese sweet treats as I can carry to the Toronto office at the gfx/layout work week in July. :)
(In reply to Brian Birtles (:birtles) from comment #17)
> At the Firefox Devcon in Osaka at the end of June (June 30 to be precise)
> we'd like to get everyone in the audience to fire up Fennec on their
> smartphones and participate in a group drawing session (using the Parapara
> animation workshop concept).

Will they be using phones or tablets? Note that currently Firefox Beta from the Google Play store will install the XUL version on tablets, and this problem might only affect tablets. (Well, it probably affects lower-end phones as well but it didn't affect the Galaxy Nexus). It would be good if we could get an idea of how much of an improvement is needed to satisfy your requirements on the target devices. (I don't really have a good metric for this, but maybe we could create a test page where you have to draw a line as quickly as you can, and the page ensures some minimum number of touch events are received to pass the test).

That being said, I was thinking about this and I wanted to try disabling coalescing of touch events in cases where Gecko is processing events other than touch events. This would result in us only delivering fewer touch events to pages that take a long time to process those touch events, and not penalize pages in cases where our drawing is slow. I'll give that a whirl today.
(In reply to Kartikaya Gupta (:kats) from comment #18)
> ... note that currently Firefox Beta from
> the Google Play store will install the XUL version on tablets, and this
> problem might only affect tablets

Drive by comment; currently tablets are blocked for Firefox Beta on Google Play (bug 759445).
I tried un-coalescing as I described in comment 18 but it ends up still being very laggy. We definitely need to figure out what we're drawing that's taking so long and how we can make it faster.
Since the app is adding SVG elements to the DOM, would that cause Fennec to start a screenshot? If so, maybe we are doing too much work.

A logcat would show the screenshot message, if it's happening.
I don't see us taking an unreasonable number of screenshots. Most of the slow events are of type 7 (DRAW events).
Here is one profile I pulled off while drawing random shapes in the full app (the one in the bug URL):

http://people.mozilla.com/~bgirard/cleopatra/?report=AMIfv961WOKuR9fBMI6Oy7p9FlDyTcuDWXIVE-66OBzH0QgBcmQWTyUZ1YERcmRiKbfh9GKcnH3Rvb-5Oprfzu2ub84duU6Tul5nkut-VomcMmqSJTdQ_CX3wUjvh43tgOrFetjDEO5JCZeh5H8944ciS22dO5sP1A

It's spending 32% of the time painting, mostly drawing the root layer. I don't really know what that means (cc'ing :jrmuizel for help here) but if I had to guess I would say that we're not doing partial invalidation of the SVG and we're redrawing the whole thing every time?
(Also note my profile is from a debug build, so should be taken with appropriate grains of salt)
(In reply to Kartikaya Gupta (:kats) from comment #24)
> (Also note my profile is from a debug build, so should be taken with
> appropriate grains of salt)

The symbols are wrong in this profile and debug build profiles are not always that easy to reason about...
(In reply to Kartikaya Gupta (:kats) from comment #18)
> (In reply to Brian Birtles (:birtles) from comment #17)
> > At the Firefox Devcon in Osaka at the end of June (June 30 to be precise)
> > we'd like to get everyone in the audience to fire up Fennec on their
> > smartphones and participate in a group drawing session (using the Parapara
> > animation workshop concept).
> 
> Will they be using phones or tablets?

Phones primarily. Also, I'm seeing the same symptoms on my phone (HTC J, HTC Sense 4.0).

> It would be good if
> we could get an idea of how much of an improvement is needed to satisfy your
> requirements on the target devices. (I don't really have a good metric for
> this, but maybe we could create a test page where you have to draw a line as
> quickly as you can, and the page ensures some minimum number of touch events
> are received to pass the test).

Yes, this is hard to spec. We really want the audience to load up the app on their own phones so the target devices are really anything :)

From what I'm seeing on my phone, I think I'd need to see at least double the number of events on my phone to get acceptably smooth drawing, triple would be ideal.

(In reply to Kartikaya Gupta (:kats) from comment #20)
> I tried un-coalescing as I described in comment 18 but it ends up still
> being very laggy. We definitely need to figure out what we're drawing that's
> taking so long and how we can make it faster.

Oh, that's disappointing. :( The performance is fine in Fennec 10 so it doesn't seem like a device limitation.
Brian - How does this work in Fx17? Any better?
tracking-fennec: 15+ → +
(In reply to Mark Finkle (:mfinkle) from comment #27)
> Brian - How does this work in Fx17? Any better?

I'm afraid not. I just tested with Fx10, Fx13, Fx 15, and Fx17. Performs smoothly in Fx10 and Fx13 and is jagged in Fx15 and Fx17. There's no obvious difference between Fx15 and Fx17.
cc'ing dholbert to see if his svg skillz can help.

I'm tempted to remove the touchmove coalescing. Its way too late for Brian's use, but we could pref it for testing at least...
(In reply to Wesley Johnston (:wesj) from comment #29)
> I'm tempted to remove the touchmove coalescing. Its way too late for Brian's
> use, but we could pref it for testing at least...

I'd like to at least run a test with touchmove coalescing turned off to see if that solves it. We'll probably be using this app often in the future so I'm still interested in finding a solution.
Attached patch PatchSplinter Review
This adds a pref for touch event coalescing.
Attachment #650367 - Flags: review?(bugmail.mozilla)
Comment on attachment 650367 [details] [diff] [review]
Patch

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

::: widget/android/nsAppShell.cpp
@@ +262,5 @@
>      if (NS_FAILED(rv)) {
>          rv = Preferences::GetString(PREFNAME_UA_LOCALE, &locale);
>      }
>  
> +    mAllowCoalescingTouches = Preferences::GetBool(PREFNAME_COALESCE_TOUCHES, true);

Move this line down a couple of lines, so it goes after the bridge->SetSelectedLocale call and before the return rv;

@@ +307,5 @@
>          }
>  
>          bridge->SetSelectedLocale(locale);
> +
> +        mAllowCoalescingTouches = Preferences::GetBool(PREFNAME_COALESCE_TOUCHES, true);

Not part of this bug, but this duplicated code for reading prefs should be refactored...
Attachment #650367 - Flags: review?(bugmail.mozilla) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ba91b3e9f38

I defaulted this coalesce Brian, but you can change it on about:config

dom.events.touch.coalescing.enabled (I added it to our defaults so it should be easy to find with a search). I'll try running with it for a weekend and if things seems ok maybe we can turn it off?
Whiteboard: [leave open]
Ping. Brian, did you find that turning off the coalescing helped things?
Flags: needinfo?(birtles)
(In reply to Kartikaya Gupta (:kats) from comment #35)
> Ping. Brian, did you find that turning off the coalescing helped things?

Thanks, sorry I hadn't commented on this yet. Yes, it helps tremendously. Performance is much better. Thank you very much!
Flags: needinfo?(birtles) needinfo?(birtles) → needinfo+
Great. :wesj, did you try running with the coalescing off? I'll give it a shot on the lowest-end phone that I have and see if it's noticeably worse. Either way we should close this bug out once we decide whether to leave it on or off by default.
I tried it on a Galaxy Q on a variety of sites and didn't see any observable slowdowns. It might still impact sites that run a lot of script for each touch event but I'd like to turn it off by default and then see if people complain.
Assignee: nobody → bugmail.mozilla
Attachment #673220 - Flags: review?(wjohnston)
Comment on attachment 673220 [details] [diff] [review]
Turn off coalescing by default

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

I was thinking the same thing yesterday.
Attachment #673220 - Flags: review?(wjohnston) → review+
https://hg.mozilla.org/mozilla-central/rev/0cc577753007
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
You need to log in before you can comment on or make changes to this bug.