Hang in DrawTargetD2D::FillGlyphs

RESOLVED FIXED in Firefox 15

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Ben Turner (not reading bugmail, use the needinfo flag!), Assigned: bas)

Tracking

unspecified
mozilla16
x86_64
Windows 7
Points:
---

Firefox Tracking Flags

(firefox15+ fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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

5 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

5 years ago
Created attachment 634260 [details] [diff] [review]
Reduce calling frequency of SetTextRenderingParams

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)
(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

5 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.
tracking-firefox15: ? → -
tracking-firefox16: ? → -
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?
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.
Oh, we'd take it, but I'd like to know if it isn't fixed :)
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/bturner@mozilla.com-2dd31542fd6f
tracking-firefox15: - → ---
tracking-firefox16: - → ---
Oops, bugzilla changed the flags...
So... It seems that this changes rendering slightly. It's hard to see it, almost. I'll attach some screenshots.
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+
Though it is also important to know why the rendering changed.
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

5 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.
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

5 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

5 years ago
Created attachment 636098 [details] [diff] [review]
Reduce calling frequency of SetTextRenderingParams v2

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)
Attachment #636098 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/mozilla-central/rev/1e188186d0f8
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16

Updated

5 years ago
tracking-firefox15: ? → +
(Assignee)

Comment 21

5 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 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+
[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 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+
http://hg.mozilla.org/releases/mozilla-beta/rev/1ac588e89dd5
status-firefox15: --- → fixed
You need to log in before you can comment on or make changes to this bug.