600476 - Google maps auto scrolling while drawing a route is jerky and slow

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Timothy Nikkel (:tnikkel)]

Goto http://gmap-pedometer.com/. Switch to Hybrid and zoom in as far as you can
and still have the roads overlaid on the images. Click Start recording. Double
click on a road somewhere. Double click somewhere else on a road. Keep drawing
a path so that the map has to scroll. This used to feel nice and smooth on a particular low end computer I have. But it now feels jerky and slow.

I bisected this and found 97188fb7b44a to be nice and smooth, changeset b5f727a62c7c shows the jerky behaviour.

(This has the same steps to reproduce as bug 593927 but is a different issue as the regression range is different.)

Comment 1

[Markus Stange [:mstange]]

What happens when you comment out the call to PreparePatternForUntiledDrawing in gfxSurfaceDrawable::Draw? Does that bring back the old performance?

That's one of the places that wasn't pure refactoring - in the untiled drawing case we now call it unconditionally, where before changeset b5f727a62c7c we only called it when there were non-integer translations (it was called SetExtendAndFilterOnPattern then).

Comment 2

[Timothy Nikkel (:tnikkel)]

Yes, commenting out that call fixes the problem!

Comment 3

[Markus Stange [:mstange]]

Great!

Comment 4

[Markus Stange [:mstange]]

Attachment: only change extend and filter for non-integer translations

This patch restores the condition around PreparePatternForUntiledDrawing (aka SetExtendAndFilterOnPattern) that http://hg.mozilla.org/mozilla-central/rev/b5f727a62c7c removed. But I'm not sure if it makes sense for the general surface drawable case, or what justifies this optimization in the first place.

Comment 5

[Joe Drew (not getting mail)]

Comment on attachment 492451 [details] [diff] [review]
only change extend and filter for non-integer translations

If the translations are integer, we assume no resampling will occur, according to http://hg.mozilla.org/mozilla-central/rev/b5f727a62c7c#l1.144 . I think it's fine to do this in the general case too, but I will get roc to take a look at it to make sure (since he wrote the code initially).

Comment 6

[Markus Stange [:mstange]]

http://hg.mozilla.org/mozilla-central/rev/64188998417c

Comment 8

[Bas Schouten (:bas.schouten)]

This killed D2D performance by using EXTEND_NONE where we know this to be really slow! We should probably still use EXTEND_PAD, and then keep the filter the same.

Comment 9

[Ed Morley [:emorley]]

(In reply to comment #8)
> This killed D2D performance by using EXTEND_NONE where we know this to be
> really slow! We should probably still use EXTEND_PAD, and then keep the filter
> the same.

Would this affect memory usage? If it might, I will need to remember to rerun testcase in bug 616280 with the new patch on top of the older rev again.

Comment 10

[Bas Schouten (:bas.schouten)]

(In reply to comment #9)
> (In reply to comment #8)
> > This killed D2D performance by using EXTEND_NONE where we know this to be
> > really slow! We should probably still use EXTEND_PAD, and then keep the filter
> > the same.
> 
> Would this affect memory usage? If it might, I will need to remember to rerun
> testcase in bug 616280 with the new patch on top of the older rev again.

It will in some cases.