Closed
Bug 765845
Opened 12 years ago
Closed 12 years ago
Hang in DrawTargetD2D::FillGlyphs
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: bent.mozilla, Assigned: bas.schouten)
Details
Attachments
(1 file, 1 obsolete file)
2.57 KB,
patch
|
jrmuizel
:
review+
lsblakk
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
Whenever I use my laptop on battery Firefox seems to slow to a crawl. I am becoming more and more convinced that the slowdown I see is due to our 3d acceleration, probably because NVIDIA switches to a very slow low-power mode. Recently I started collecting minidumps whenever Firefox enters a hang state (where the window gets greyed out and the title bar says "Not Responding"). I collected about ten different dumps and each one had the following stack:
Main thread:
DWrite.dll!operator!=() + 0x19 bytes
DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::IsEqualKeyImpl() + 0x3a bytes
DWrite.dll!CacheReader::LookupElement() + 0xe6 bytes
DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::FillFromReadOnlyCache() + 0x76 bytes
DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::ServerLookup() + 0xa8 bytes
DWrite.dll!GlyphDataBlock<GlyphBitmapLayout,GlyphBitmapRasterizationParameters,GlyphBitmapRasterizationState>::GetGlyphData() + 0x57 bytes
DWrite.dll!GetGlyphBitmaps() + 0x93 bytes
DWrite.dll!GlyphRunAnalysis::GlyphRunAnalysis() + 0x2fc bytes
DWrite.dll!DWriteFactory::CreateGlyphRunAnalysis() + 0xfb bytes
d2d1.dll!GlyphRunRealizer::Init() + 0xf8 bytes
d2d1.dll!CHwSurfaceRenderTarget::DrawGlyphRunInternal() + 0xbc bytes
d2d1.dll!CCommand_DrawGlyphRun::Execute() + 0x27 bytes
d2d1.dll!CHwSurfaceRenderTarget::ProcessBatch() + 0x4c bytes
d2d1.dll!CBatchSerializer::FlushInternal() + 0x2e bytes
d2d1.dll!CBatchSerializer::FlushInternalAndTryIncreaseStorage() + 0x36 bytes
d2d1.dll!CBatchSerializer::StartCommand<CCommand_SetTextRenderingParams>() + 0xc bytes
d2d1.dll!DrawingContext::SetTextRenderingParams() + 0x34 bytes
d2d1.dll!D2DRenderTargetBase<ID2D1HwndRenderTarget>::SetTextRenderingParams() + 0x4e bytes
> gkmedias.dll!mozilla::gfx::DrawTargetD2D::FillGlyphs(mozilla::gfx::ScaledFont * aFont, const mozilla::gfx::GlyphBuffer & aBuffer, const mozilla::gfx::Pattern & aPattern, const mozilla::gfx::DrawOptions & aOptions, const mozilla::gfx::GlyphRenderingOptions * aRenderOptions) Line 886
xul.dll!gfxContext::AzureState::AzureState(const gfxContext::AzureState & __that) + 0x80 bytes
00388a04()
000000aa()
4a7faa24()
Is there anything that can be done about this?
Here are my graphics stats:
Adapter Description
Intel(R) HD Graphics Family
Vendor ID
0x8086
Device ID
0x0126
Adapter RAM
Unknown
Adapter Drivers
igdumd64 igd10umd64 igd10umd64 igdumdx32 igd10umd32 igd10umd32
Driver Version
8.15.10.2538
Driver Date
9-26-2011
Adapter Description (GPU #2)
NVIDIA Quadro 2000M
Vendor ID (GPU #2)
0x10de
Device ID (GPU #2)
0x0dda
Adapter RAM (GPU #2)
2048
Adapter Drivers (GPU #2)
nvd3dumx,nvwgf2umx,nvwgf2umx nvd3dum,nvwgf2um,nvwgf2um
Driver Version (GPU #2)
8.17.12.7593
Driver Date (GPU #2)
8-12-2011
Direct2D Enabled
true
DirectWrite Enabled
true (6.1.7601.17789)
ClearType Parameters
ClearType parameters not found
WebGL Renderer
Blocked for your graphics card because of unresolved driver issues.
GPU Accelerated Windows1/1
Direct3D 10
AzureBackend
direct2d
Assignee | ||
Comment 1•12 years ago
|
||
Hrm, we will be calling this a little more with Azure content than without. Has this problem started recently? The fix should be easy. But I should fix this on aurora as well.
tracking-firefox15:
--- → ?
tracking-firefox16:
--- → ?
Assignee | ||
Comment 2•12 years ago
|
||
This makes us call TextRenderingParams as often as we do with Cairo. I never really knew this could cause a flush of the command stream so could be expensive.
Assignee: nobody → bas.schouten
Status: NEW → ASSIGNED
Attachment #634260 -
Flags: review?(jmuizelaar)
Reporter | ||
Comment 3•12 years ago
|
||
(In reply to Bas Schouten (:bas) from comment #1)
> Has this problem started recently?
Not sure about this particular stack, but FF has been really slow on battery power for a while now. I try to be plugged in whenever I can because of it so I just don't know.
Comment 4•12 years ago
|
||
It's not clear that this is a recent regression, so no need to track for release. That being said, we would consider an uplift to Aurora 15 once fixed.
Comment 5•12 years ago
|
||
If this patch fixes the bug, then it's a regression from bug 715768, which landed for Firefox 15.
Ben, can you do a build with this patch in it?
Reporter | ||
Comment 6•12 years ago
|
||
Wait, if this reduces the number of times we call this function that clearly causes an expensive blocking operation why wouldn't we take it?
I can do a build sometime soon, I guess.
Comment 7•12 years ago
|
||
Oh, we'd take it, but I'd like to know if it isn't fixed :)
Reporter | ||
Comment 8•12 years ago
|
||
tracking-firefox15:
- → ---
tracking-firefox16:
- → ---
Reporter | ||
Comment 9•12 years ago
|
||
Oops, bugzilla changed the flags...
Reporter | ||
Comment 10•12 years ago
|
||
So... It seems that this changes rendering slightly. It's hard to see it, almost. I'll attach some screenshots.
Comment 13•12 years ago
|
||
Comment on attachment 634260 [details] [diff] [review]
Reduce calling frequency of SetTextRenderingParams
Review of attachment 634260 [details] [diff] [review]:
-----------------------------------------------------------------
::: gfx/2d/DrawTargetD2D.h
@@ +199,5 @@
> mutable RefPtr<ID2D1RenderTarget> mRT;
>
> + // We store this to prevent excessive SetTextRenderingParams calls.
> + RefPtr<IDWriteRenderingParams> mTextRenderingParams;
> +
This should say why we want to avoid excessive SetTextRenderingParams calls, but maybe that is better off at the SetTextRenderingParams call site.
Attachment #634260 -
Flags: review?(jmuizelaar) → review+
Comment 14•12 years ago
|
||
Though it is also important to know why the rendering changed.
Comment 15•12 years ago
|
||
It looks to me like there are two differences in the font rendering: different hinting behavior (maybe the difference between Natural and GDI-like rendering, for example?), and different gamma correction being used for the colored subpixel-AA pixels.
Assignee | ||
Comment 16•12 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #14)
> Though it is also important to know why the rendering changed.
I guess I could see this happening if it returned the TempRT for drawing somewhere... I'll add something for that to the patch.
Reporter | ||
Comment 17•12 years ago
|
||
The verdict is in, though: this patch fixes the hang I was seeing. Now I see some slowdown on battery but all my profiles point to the layout engine.
Assignee | ||
Comment 18•12 years ago
|
||
This is a regression then, let's fix it. I'll be uploading a new patch in a little bit.
tracking-firefox15:
--- → ?
Assignee | ||
Comment 19•12 years ago
|
||
The previous patch was very broken (note how it sets mParams before actually checking if it's equal to params). This patch fixes all correctness issues.
Attachment #634260 -
Attachment is obsolete: true
Attachment #636098 -
Flags: review?(jmuizelaar)
Updated•12 years ago
|
Attachment #636098 -
Flags: review?(jmuizelaar) → review+
Comment 20•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Updated•12 years ago
|
Assignee | ||
Comment 21•12 years ago
|
||
Comment on attachment 636098 [details] [diff] [review]
Reduce calling frequency of SetTextRenderingParams v2
[Approval Request Comment]
Bug caused by (feature/regressing bug #): 715768
User impact if declined: Severe performance slowdown in some cases
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): Very low risk
String or UUID changes made by this patch: None
Attachment #636098 -
Flags: approval-mozilla-aurora?
Comment 22•12 years ago
|
||
Comment on attachment 636098 [details] [diff] [review]
Reduce calling frequency of SetTextRenderingParams v2
[Triage Comment]
New regression in FF15, with a low risk fix. Approved for Aurora 15.
Attachment #636098 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 23•12 years ago
|
||
[Triage Comment]
This missed landing in when 15 was on Aurora so now needs to land to beta. Please update the status flags once this has landed.
Comment 24•12 years ago
|
||
Comment on attachment 636098 [details] [diff] [review]
Reduce calling frequency of SetTextRenderingParams v2
[Triage Comment]
bumping up the approval nomination accordingly.
Attachment #636098 -
Flags: approval-mozilla-aurora+ → approval-mozilla-beta+
Comment 25•12 years ago
|
||
status-firefox15:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•