865546 - Large scaled images in SVG's cause choppy scrolling

Status: VERIFIED FIXED

(Core :: Graphics, defect)

Description

[Ben Aiken]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.65 Safari/537.31

Steps to reproduce:

Scrolling around on a page with a very large image (6000px x 6000px, 54dpi) that is scaled down causes choppy scrolling. Occurs on Firefox v18+ on OSX. Does not seem to occur in versions 17 and prior.


Actual results:

The browser window stutters immensely.


Expected results:

There shouldn't be chopping scrolling. Seems to have been introduced in v18 of Firefox for OSX. Confirmed that it happens with v18, v19, v20, v21b4. Confirmed that does NOT happen in v16, v17.

Comment 1

[Ben Aiken]

Also worth noting that we are intending to push a new release for our web application that affects thousands of people, but this release is being held up due to the fact that the new images we need to use cause issues with the latest versions of Firefox.

Comment 2

[daniel.cane]

Bump - this is very big deal effecting our entire user base of over 15,000 medical staff. You'll see some others from our company jump in and vote -- that's not fake folks -- that's urgency. Thanks in advance.

Comment 3

[Loic]

Provide a testcase or image with such a behavior.

Comment 4

[Ben Aiken]

Attachment: Zipped up package with SVG, JPG and page to demonstrate

Comment 5

[Ben Aiken]

The easiest way to reproduce this (and our use case) is when the image is embedded within an SVG. I am attaching a small set of files with a page that demonstrates the issue. The issue is significantly worse in v20.0 of Firefox from what I can tell.

Comment 6

[Ben Aiken]

When loading the same page in the attached package in Safari [Version 6.0.2 (8536.26.17)], there isn't any demonstrable chop.

Comment 7

[Robert Longson [:longsonr]]

If this didn't happen in 17 it would be helpful if you could find a regression range. Instructions for how to do that are here: http://mozilla.github.io/mozregression/

Comment 8

[Ben Aiken]

Comment on attachment 741841 [details]
Zipped up package with SVG, JPG and page to demonstrate

This attachment is obsolete. Need to update with a new one. My apologies.

Comment 9

[Ben Aiken]

Attachment: Corrected attachment Zip

Comment 10

[Ben Aiken]

Great tool (mozregression). Here are the results - hopefully this should really help narrow it down:

Last good nightly: 2013-01-07
First bad nightly: 2013-01-08

Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=605ae260b7c8&tochange=dccab70d8554

Comment 11

[Robert Longson [:longsonr]]

Is it still a problem with the latest nightly? Bug 504071 is about the only image thing in that range and I think that was recently disabled for SVG images by bug 861805.

Comment 12

[Ben Aiken]

Yep, unfortunately still seeing it. I tested with the following nightly version: 23.0a1 (2013-04-25).

What about bug 798839? It's related to gradients but I'm wondering if maybe some refactoring occurred related to the SVG piece?

Comment 13

[Ben Aiken]

Or perhaps bug 825834? Seems like a few SVG-related commits happened as part of this changeset: http://hg.mozilla.org/mozilla-central/rev/dccab70d8554

Comment 14

[Robert Longson [:longsonr]]

bug 798839 is code that's disabled and you're presumably not using a <view> element in your SVG as they are rarely used. Do correct me if I'm wrong though.

Comment 15

[Ben Aiken]

Hmm okay. I believe you're correct unless the way we're using SVG's inherently uses an SVGViewElement.

Have you or anyone else had a chance to look at the sample code attached and reproduce the issue? I had a few other of our developers confirm the same findings that I did using mozregression with the sample provided.

Comment 16

[Robert Longson [:longsonr]]

You could try profiling the 

Last good nightly: 2013-01-07
First bad nightly: 2013-01-08

builds. Perhaps something will jump out. https://developer.mozilla.org/en-US/docs/Performance/Profiling_with_the_Built-in_Profiler

Comment 17

[Ben Aiken]

Attachment: 2013-01-08 nightly

Profiler results from 2013-01-08 nightly.

Comment 18

[Ben Aiken]

Attachment: 2013-01-07 nightly

Profiler results from the 2013-01-07 nightly.

Comment 19

[Ben Aiken]

Profiler results from the two nightlies have been attached. There's a noticeable increase resources being used inside the image::imgFrame::Draw layer. I have highlighted the two comparable areas on the attached images.

Comment 20

[Ben Aiken]

Did the attached profiling images help in determining what the issue is?

Comment 21

[Luis Moss]

Any progress on finding the cause of the increased resources?

Comment 22

[Karuna]

Can we please fix this ASAP. Thanks,

Comment 23

[Jonathan Watt [:jwatt]]

Alice, can you help identify what caused this? Nothing is jumping out from comment 10.

Comment 24

[George Holler]

I am seeing this too. Do we know what code introduced it?

Comment 25

[Alice0775 White]

My pc is windows7, And I notice the choppy scrolling.

Steps to Reproduce:
1. Open attachment 741870 [details] ffTest.html
2. Reload skip cache (Ctrl+F5)
3. Scroll mouse wheel to down immediately after completion of the page load
4. Repeat Step.2-3 if necessary 
 
Actual Results:
Smooth scroll fails.

Expected Results:
Smoothly scrolled to down

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/48abe27b73b7
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20130106 Firefox/20.0 ID:20130106204602
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/546f9f3999d3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20130106 Firefox/20.0 ID:20130106214802
Pshlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=48abe27b73b7&tochange=546f9f3999d3

Suspected: Bug 504071

Comment 26

[daniel.cane]

/begging

We are literally "screwed" without a fix for this. We dropped support for IE and are "all in" for Firefox. We have over 15,000 end users. We were named to Forbes "America's Most Promising Companies." Not that all companies dont have potential, but this silly little bug is really putting an entire company of 135 people in jeopardy.  

Please. Please. Help.

Dan aka "The CEO"

Comment 27

[Jimmy Z]

Please fix this.

Comment 28

[Jordan Miller]

This is mission critical for our app which is involved in patient care.  Our physicians will not be able to care for their patients effectively without this fix.  Please help

Comment 29

[Jonathan Watt [:jwatt]]

Attachment: JPG for testcase

Comment 30

[Jonathan Watt [:jwatt]]

Attachment: SVG for testcase

Comment 31

[Jonathan Watt [:jwatt]]

Attachment: testcase

Comment 32

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Note that this testcase could be written without SVG and would probably perform better on all browsers.

Comment 33

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Propagate FLAG_CLAMP through RasterImage::DrawWithPreDownscaleIfNeeded

Comment 34

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Thanks Ben Aiken for the profiling!

Comment 35

[Joe Drew (not getting mail)]

Comment on attachment 747324 [details] [diff] [review]
Propagate FLAG_CLAMP through RasterImage::DrawWithPreDownscaleIfNeeded

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

::: image/src/RasterImage.cpp
@@ +3090,5 @@
>                        mSize.height - framerect.YMost(),
>                        framerect.x);
>  
> +  frame->Draw(aContext, aFilter, userSpaceToImageSpace, aFill, padding, subimage,
> +              aFlags & FLAG_CLAMP);

Couldn't we just pass aFlags unmodified through here? (We only look at FLAG_CLAMP now, but in the future we could look at others.)

Comment 36

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Sure. I'll do that.

Comment 37

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix v2

Comment 38

[Ben Aiken]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #34)
> Thanks Ben Aiken for the profiling!

You bet! Please let me know if there's anything else we can do to help with this.

Comment 39

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/831491563b7d

Comment 40

[daniel.cane]

Thanks for working on this team. It means a great deal.

Comment 41

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/831491563b7d

Comment 42

[Freddy Daoud]

Thank you so much for resolving this issue, much appreciated!

Comment 43

[Loic]

Is the fix going to backport in beta Firefox 21 or is it too late?

Comment 44

[Jonathan Watt [:jwatt]]

It's definitely not going to be in 21, but we may get it into 22.

Comment 45

[Ed Morley [:emorley]]

I've retriggered Nightlies (and cancelled the existing ones) to pick up this change. In a few hours (slightly longer for Windows), the 2013-05-10 Nightly will be available, which will contain this fix. Download it at:
http://nightly.mozilla.org/
(See the about panel to confirm which date nightly you have).

Comment 46

[daniel.cane]

Looking forward to testing this, and we very much appreciate all the hard work in getting this resolved. Since this is not going to get picked up in 21, what's the super high level timeline for 22? The entire team here, and all of our clients, are grateful for getting this resolved.

Comment 47

[Jonathan Watt [:jwatt]]

Roughly June 25th for v22.

Comment 48

[Ryan VanderMeulen [:RyanVM]]

Firefox releases ship every 6 weeks. Fx22 (assuming this patch is approved to land there) is scheduled to ship 2013-06-25.
https://wiki.mozilla.org/RapidRelease/Calendar

Comment 49

[Timothy Nikkel (:tnikkel)]

Instead of waiting for this to get fixed in the stable version of Firefox you may want to work around the issue, comment 32 suggests it shouldn't be too hard.

Comment 50

[Ben Aiken]

(In reply to Timothy Nikkel (:tn) from comment #49)
> Instead of waiting for this to get fixed in the stable version of Firefox
> you may want to work around the issue, comment 32 suggests it shouldn't be
> too hard.

We're using SVG's to overlay additional information on the image, but the problem seems to be related to any large scaled images. In our situation, our large scaled images are JPG's embedded within an SVG. The less the image scales, the less the chop happens. Our images are approximately 6000x6000 pixels to give you an idea of the amount of scaling that has to happen. 

We can actually work around the problem by drastically scaling down our images. However, this isn't by any means an ideal solution for us. Detailed views of the body matter very much in the medical field and scaling down these images really kills the ability for us to show those details.

That being said, this latest change helps the problem, but it actually doesn't fix it for us. There's still a drastically noticeable difference between the performance from v17 vs. the latest nightly. We're still exploring other options as well because we just absolutely need to be able to get around this and still can't launch with the latest nightly's performance.

We really can't stress enough how much we appreciate the efforts and attention that we've been able to garner for this issue, but I don't think we're in a FIXED state yet.

I will work on getting more profiling information for the team to see what else might stand out.

Comment 51

[Alex Keybl [:akeybl]]

No need to track, please just nominate for uplift approval once resolved.

Comment 52

[Loic]

(In reply to Ben Aiken from comment #50)
> There's still a drastically noticeable difference between the performance from v17 > vs. the latest nightly.

The regression appeared in FF20 nightlies, so asking your clients to use Firefox 17 ESR (which is recommended in corporate/university environment) will fix the issue.
See http://www.mozilla.org/en-US/firefox/organizations/

Comment 53

[Robert O'Callahan (:roc) (email my personal email if necessary)]

We shouldn't reopen this bug. A bug was fixed here.

Please file a new bug which is a clone of this bug. And I think reprofiling would be very helpful.

Comment 54

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Also, is Mac the only platform you care about?

Comment 55

[Ben Aiken]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #53)
> We shouldn't reopen this bug. A bug was fixed here.
> 
> Please file a new bug which is a clone of this bug. And I think reprofiling
> would be very helpful.

Will do shortly. Thank you sir.

Also regarding the Mac platform question - nope, we care about both. Our clients are a mix of both Windows/OSX (and possibly even Linux).

Comment 56

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Did you file that new bug?

Comment 57

[Loic]

I guess it's bug 871729.

Comment 58

[Ben Aiken]

Sorry no, not yet. Bug 871729 is separate from this issue. I will be re-profiling for this issue later today and opening up a clone bug for this issue as requested. Thanks for the reminder!

Comment 59

[Ben Aiken]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #56)
> Did you file that new bug?

Bug 873551 has been created. I am going to note in that ticket, but I'm being blocked from profiling newer builds 2013-01-18 and on seem to have broken profiling, at least for OSX. Confirmed this with another team member.

Comment 60

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:23.0) Gecko/20100101 Firefox/23.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:23.0) Gecko/20100101 Firefox/23.0

Verified as fixed on Firefox 23 beta 9 (buildID: 20130725195523) and latest Nightly (buildID: 20130725171558).