Closed Bug 612042 Opened 9 years ago Closed 9 years ago
Hardware acceleration causes responsiveness to lag
On this page, FF 4b7's responsiveness suffers significantly when hardware acceleration is enabled. I can reproduce it reliably in a virgin profile, and at least one other user confirms it. In detail: 1) Via the Options GUI, enable Hardware acceleration (Tools | options | advanced | general ...) 2) Load this page; don't scroll yet. http://www.baseball-reference.com/boxes/PIT/PIT196010130.shtml 3) Note FF's CPU and memory usage 4) The middle of this page has a graph titled "Win Probability Chart'. Watch FF's CPU and memory, and scroll down until the chart is visible. 5) If your computer works like mine, you'll notice the following: a) Scrolling: Hangs for ~1 sec just before the graph is visible, then proceeds in slow jumps. b) CPU usage: Jumps ~10-15% momentarily c) Memory: ~100 MB increase in Working Set Memory, Memory (Private Working Set), and Commit Size. 6) With the chart still visible, switch to another tab and switch back. Note the 2-3 second lag in displaying the problematic tab. 7) Watching FF's memory usage, scroll the chart out of view. After a few seconds it drops ~100 MB. Note that scrolling also works normally when the graph isn't visible. Everything behaves normally with hardware accelaration disabled (via the Options GUI) and in IE. I don't know how previous betas performed on this page. I did try it in a virgin profile. MY SYSTEM * Win7 x64 * Firefox 4b7 * Intel Core2 Duo 2.8 GHz, 8 GB RAM * Graphics, courtesy about:support: Adapter Description: NVIDIA Quadro NVS 160M Vendor ID: 10de Device ID: 06eb Adapter RAM: 256 Adapter Drivers: nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um Driver Version: 22.214.171.12421 Driver Date: 6-16-2009 Direct2D Enabled: true DirectWrite Enabled: true GPU Accelerated Windows: 2/2 Direct3D 10 * Some prefs -- I have no idea what's relevant, so here's a bunch: gfx.3d_video.enabled;false gfx.color_management.display_profile; gfx.color_management.mode;2 gfx.color_management.rendering_intent;0 gfx.direct2d.disabled;false gfx.direct2d.force-enabled;false gfx.downloadable_fonts.enabled;true gfx.downloadable_fonts.sanitize;true gfx.downloadable_fonts.sanitize.preserve_otl_tables;true gfx.font_rendering.cleartype.always_use_for_content;false gfx.font_rendering.cleartype.use_for_downloadable_fonts;true gfx.font_rendering.directwrite.enabled;false gfx.font_rendering.harfbuzz.level;1 gfx.use_text_smoothing_setting;false layers.accelerate-all;true layers.accelerate-none;false layers.prefer-d3d9;false layers.prefer-opengl;false (I really hope this isn't a dupe. Without knowing the underlying problem, I didn't know what to search for.) Thanks!
I'm hoping that this is somehow going to be fixed by your border fixes, Bas, but if this still sucks after that, we should at least investigate for some beta.
Assignee: nobody → bas.schouten
blocking2.0: ? → betaN+
My border drawing code did not fix this. I'm seeing the following behavior: GDI: Generally slow, a little bit slower at the graph, but not terribly so. D2D: Generally fast, but -terribly- slow at the graph. It might still be border rendering but a path my code doesn't cover. Or it could be something else in drawing. I'm investigating this. On a sidenote, I personally think that's a silly way to draw a graph!
Status: NEW → ASSIGNED
The problem here consists of two things: We're using EXTEND_NONE to draw the background images. This seems to be because gfxSurfaceDrawable does not usually seem to set any extend mode on the pattern, causing it to default to NONE. We're not re-using the image surfaces here as far as I can see, causing all these to be re-uploaded to the GPU each frame, which makes it somewhat slower than it could be as well. I haven't looked into this in detail yet. The attached patch uses EXTEND_PAD for the default gfxSurfaceDrawable::Draw case. I'm not 100% sure this is correct, I've fired it at try to see if it passes all tests.
Are we taking the temporary surface path in imgFrame::SurfaceForDrawing?
(In reply to comment #4) > Are we taking the temporary surface path in imgFrame::SurfaceForDrawing? I don't think so since we know doTile is not true because otherwise it would use EXTEND_REPEAT. But I'm waiting for my build to finish. It seems that we're definitely spending most time uploading images though. Which we only do if we can't find user data on the source surface with a texture stored on it.
(In reply to comment #5) > I don't think so since we know doTile is not true because otherwise it would > use EXTEND_REPEAT. No, we don't know that. Sometimes we manually repeat the image to a temporary surface and then draw that (to avoid sampling outside the desired rectangle in tile space).
So, the reason we're not caching here is there's a background image used that is 1000x20000 pixels. For these images we don't currently cache, this is also the reason for the high memory usage, 1000x20000x4B=80MB. My suggestion is to fix the EXTEND_NONE as per IRC discussion and then consider this resolved. Performance is not super but I think it's acceptable (with the patch) for a website using an 80MB background image.
After looking at the code and thinking about this, I believe this to be the cleanest code change. Otherwise we'd need to duplicate the linux detection code in a Filter and ExtendMode function.
Hrm, bug seems to have been posted before the EXTEND_NONE change though. Let's hope this is improved sufficiently by a combination of my border drawing fixes and this fix.
Comment on attachment 496408 [details] [diff] [review] Use SetupPattern for all images but FAST filter for integertranslations Two issues: 1. You want to set it to FILTER_FAST when we have only integer translations. Currently you set it backwardsly. 2. A a comment above the NonIntegerTranslation block saying "If we only have integer translations, we presume no special filtering needs to happen, and so we explicitly use FILTER_FAST."
Attachment #496408 - Flags: review?(joe) → review-
You're absolutely right. Addressed.
Comment on attachment 496548 [details] [diff] [review] Use SetupPattern for all images but FAST filter for integertranslations v2 Hm, now I don't like the double-negatives! How about we, as a separate patch, add gfxMatrix::HasOnlyIntegerTranslation(), which returns !HasNonIntegerTranslation(), then use that?
Attachment #496548 - Flags: review?(joe) → review-
As per request this adds the HasOnlyIntegerTranslation function.
Attachment #496746 - Flags: review?(joe)
Uses HasOnlyIntegerTranslation function as per request.
Attachment #496746 - Flags: review?(joe) → review+
Comment on attachment 496747 [details] [diff] [review] Use SetupPattern for all images but FAST filter for integertranslations v3 hooray
Attachment #496747 - Flags: review?(joe) → review+
http://hg.mozilla.org/mozilla-central/rev/d0acc22d830e http://hg.mozilla.org/mozilla-central/rev/25d77f95d92c Let's see how the bug behaves once these are in.
Nah! Let's open a new bug if there are still problems.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
The only 'problems' I see after applying this patch is slow scrolling when the "Win Probability Chart" is in view. CPU goes up to about 3%. WSS jumps about 100MB. I don't see the other problems mentioned. I am running a high end machine. H/W Acceleration is on. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101212 Firefox/4.0b8pre - Build ID: 20101212030333
(In reply to comment #18) > The only 'problems' I see after applying this patch is slow scrolling when the > "Win Probability Chart" is in view. CPU goes up to about 3%. WSS jumps about > 100MB. Its definitely an interactive chart that will display popups depend on the bar you hover over. Maybe its a bunch of JS (without looking at the code)
You need to log in before you can comment on or make changes to this bug.