Closed
Bug 1382232
Opened 8 years ago
Closed 4 years ago
Cairo APIs still used from content process (independent of printing)
Categories
(Core :: Graphics, enhancement, P3)
Tracking
()
People
(Reporter: Alex_Gaynor, Unassigned)
References
Details
(Keywords: feature, Whiteboard: [gfx-noted])
Filing this as suggested by :jimm.
Right now it looks like in a few cases, the cairo APIs are still used in the content process on Windows, whereas the expectation was that "cairo should only be in use for compositing, drawing in content should all be through skia."
I came across this in researching win32k syscalls, so this isn't a comprehensive look at all cairo usage, rather it's the usage that happened to trigger win32k syscalls.
The three APIs I see being used are:
_cairo_win32_surface_create_for_dc, _moz_cairo_scaled_font_create, _moz_cairo_surface_destroy
Here are example stacks traces for each of them (truncated to 10 frames, let me know if more would be helpful):
win32u!NtGdiCreateCompatibleDC
gdi32full!CreateCompatibleDC+0x13
xul!_create_dc_and_bitmap+0x187
xul!_cairo_win32_surface_create_for_dc+0x72
xul!gfxWindowsSurface::gfxWindowsSurface+0x77
xul!gfxWindowsNativeDrawing::BeginNativeDrawing+0x4b0
xul!nsNativeThemeWin::DrawWidgetBackground+0x29b
xul!nsDisplayThemedBackground::PaintInternal+0xb0
xul!nsDisplayThemedBackground::Paint+0x13
xul!mozilla::FrameLayerBuilder::PaintItems+0x46f
xul!mozilla::FrameLayerBuilder::DrawPaintedLayer+0x824
xul!mozilla::layers::ClientPaintedLayer::PaintThebes+0x163
win32u!NtUserSystemParametersInfo
USER32!RealSystemParametersInfoA+0xf2
UxTheme!ClassicSystemParametersInfoA+0x29
UxTheme!_InternalSystemParametersInfo+0x29
UxTheme!ThemeSystemParametersInfoA+0xcb
USER32!SystemParametersInfoA+0x9b
xul!_moz_cairo_win32_get_system_text_quality+0x17
xul!_cairo_dwrite_font_face_scaled_font_create+0x148
xul!_moz_cairo_scaled_font_create+0x3c5
xul!gfxDWriteFont::GetCairoScaledFont+0xf5
xul!gfxDWriteFont::SetupCairoFont+0x13
xul!gfxFont::GetRoundOffsetsToPixels+0x4c
xul!gfxFont::SplitAndInitTextRun<unsigned
xul!gfxFontGroup::InitScriptRun<unsigned
xul!gfxFontGroup::InitTextRun<unsigned
xul!gfxFontGroup::MakeTextRun+0x138
win32u!NtGdiSetIcmMode
gdi32full!IcmSelectColorTransform+0x75
gdi32full!IcmDeleteLocalDC+0x2f
GDI32!InternalDeleteDC+0x26d
GDI32!DeleteDC+0x13
xul!_cairo_win32_surface_finish+0x52
xul!_moz_cairo_surface_finish+0x76
xul!_moz_cairo_surface_destroy+0x2c
xul!gfxASurface::Release+0x27
xul!mozilla::RefPtrTraits<gfxWindowsSurface>::Release+0x5
xul!RefPtr<gfxWindowsSurface>::ConstRemovingRefPtrTraits<gfxWindowsSurface>::Release+0x5
xul!RefPtr<gfxWindowsSurface>::{dtor}+0x11
xul!gfxWindowsNativeDrawing::~gfxWindowsNativeDrawing+0x34
xul!nsNativeThemeWin::DrawWidgetBackground+0xd6e
xul!nsDisplayThemedBackground::PaintInternal+0xb0
This is run from a VM, so possibly relevant bits of about:support:
Compositing: Basic
WebGL 1 Driver Renderer: Google Inc. -- ANGLE
WebGL 1 Driver Version: OpenGL ES 2.0 (ANGEL 2.1.0...)
WebGL 2 Driver Renderer: WebGL Creation Failed: * Refused to create WebGL2 context because of blacklist entry: FEATURE_FAILURE_UNKNOWN_DEVICE_VENDOR
Direct2D: Blocked for your graphics card because of unresolved driver issues.
DirectWrite: True
We may want to spin off a few different bugs for this; the theme related ones are the nasty ones, so those in particular can use a separate bug. Some of the other ones (not all in the comment above) should be doable, some of the DWrite may be tricky, and we'll find out from :jfkthame what to do with the font related ones.
Flags: needinfo?(jfkthame)
Priority: -- → P3
Whiteboard: [gfx-noted
Reporter | ||
Comment 2•8 years ago
|
||
The theme related ones may just be specific instances of bug 1381938 -- I'm still learning the gfx code :-) They jumped out to :jimm because of the cairo usage.
Comment 3•8 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #1)
> we'll find out from :jfkthame what to do
> with the font related ones.
Might be better to ping :lsalzman, as I'm not really up-to-date with the state of the skia font APIs, which are presumably what we'll be wanting to use (via moz2d?) instead of cairo here.
Basically, the shaping code needs to ask whether glyph positions will be snapped to device pixels when rendered, or if fractional-pixel positioning will be preserved. This typically depends on a bunch of factors such as the presence of transforms and the type of surface we're rendering to, in addition to font hinting/rendering-mode options in effect.
This is needed because if we do all the shaping computations with "ideal" fractional glyph metrics, and then each glyph position gets individually snapped (rounded) to the pixel grid, the result is often ugly erratic glyph spacing within a string.
So can GetRoundOffsetsToPixels use moz2d apis to determine these things, instead of cairo as it currently does?
Flags: needinfo?(jfkthame) → needinfo?(lsalzman)
Comment 4•8 years ago
|
||
Another stack is:
5 - win32u!NtGdiHfontCreate
win32u!NtGdiHfontCreate
gdi32full!CreateFontIndirectExW+0x73
gdi32full!CreateFontIndirectWImpl+0x6e
xul!AutoSelectFont::{ctor}+0x10
xul!gfxDWriteFontEntry::CopyFontTable+0x106
xul!gfxDWriteFontEntry::IsCJKFont+0x58
Jonathan do you have thoughts on how we can figure out this without using GDI?
Flags: needinfo?(jfkthame)
Comment 5•8 years ago
|
||
(In reply to Jonathan Kew (:jfkthame) from comment #3)
> (In reply to Milan Sreckovic [:milan] from comment #1)
> > we'll find out from :jfkthame what to do
> > with the font related ones.
>
> Might be better to ping :lsalzman, as I'm not really up-to-date with the
> state of the skia font APIs, which are presumably what we'll be wanting to
> use (via moz2d?) instead of cairo here.
>
> Basically, the shaping code needs to ask whether glyph positions will be
> snapped to device pixels when rendered, or if fractional-pixel positioning
> will be preserved. This typically depends on a bunch of factors such as the
> presence of transforms and the type of surface we're rendering to, in
> addition to font hinting/rendering-mode options in effect.
>
> This is needed because if we do all the shaping computations with "ideal"
> fractional glyph metrics, and then each glyph position gets individually
> snapped (rounded) to the pixel grid, the result is often ugly erratic glyph
> spacing within a string.
>
> So can GetRoundOffsetsToPixels use moz2d apis to determine these things,
> instead of cairo as it currently does?
GetRoundOffsetsToPixels is definitely silly looking. In principle we should be able to do this without going through the intermediate Cairo scaled font for most font types (maybe Fontconfig being the only really tricky case, but probably solvable).
Flags: needinfo?(lsalzman)
Comment 6•8 years ago
|
||
(In reply to Jeff Muizelaar [:jrmuizel] from comment #4)
> Another stack is:
>
> 5 - win32u!NtGdiHfontCreate
> win32u!NtGdiHfontCreate
> gdi32full!CreateFontIndirectExW+0x73
> gdi32full!CreateFontIndirectWImpl+0x6e
> xul!AutoSelectFont::{ctor}+0x10
> xul!gfxDWriteFontEntry::CopyFontTable+0x106
> xul!gfxDWriteFontEntry::IsCJKFont+0x58
>
> Jonathan do you have thoughts on how we can figure out this without using
> GDI?
By setting "gfx.font_rendering.directwrite.use_gdi_table_loading" to false.
(See bug 602792 for background on where that came from.... though I don't know whether the DWrite issues being addressed there are still relevant on current systems.)
Flags: needinfo?(jfkthame)
![]() |
||
Comment 7•8 years ago
|
||
Another gdi use gfxWindowsSurface, there's lot of gdi going on in this class.
win32u!NtGdiCreateDIBSection
win32u!NtGdiCreateDIBSection
gdi32full!CreateDIBSectionInternal+0x377
gdi32full!CreateDIBSection+0x20
GDI32!CreateDIBSectionStub+0x55
xul!_create_dc_and_bitmap+0x1b8
xul!_cairo_win32_surface_create_for_dc+0x72
xul!gfxWindowsSurface::gfxWindowsSurface+0x77
![]() |
||
Comment 8•8 years ago
|
||
For Cairo, right now we are seeing:
* font access
* drawing surfaces (particularly in theming which is covered by another bug)
* printing
![]() |
||
Comment 9•8 years ago
|
||
Lets let this bug sit for a bit while we clean up known areas where cairo gets invoked (like theming and printing) in other bugs.
Updated•8 years ago
|
status-firefox57:
--- → wontfix
![]() |
||
Updated•8 years ago
|
status-firefox57:
wontfix → ---
Jim, did you really want to suggest this should get uplifted to 57?
status-firefox56:
--- → wontfix
status-firefox57:
--- → wontfix
status-firefox58:
--- → fix-optional
status-firefox-esr52:
--- → wontfix
Flags: needinfo?(jmathies)
Whiteboard: [gfx-noted → [gfx-noted]
![]() |
||
Comment 11•8 years ago
|
||
(In reply to Milan Sreckovic [:milan] from comment #10)
> Jim, did you really want to suggest this should get uplifted to 57?
Nope, I clear all status flags on feature bugs since there's no point in having them. Does the graphics team use those for something?
Flags: needinfo?(jmathies) → needinfo?(milan)
Shows up in 57 tracked queries, but I added "feature" keyword now to make sure it doesn't.
Flags: needinfo?(milan)
Keywords: feature
Comment 13•7 years ago
|
||
status-firefox59:
--- → ?
![]() |
||
Updated•6 years ago
|
Summary: Cairo APIs still used from content process → Cairo APIs still used from content process (independent of printing)
Comment 14•4 years ago
|
||
I have a suspicion that these have already been dealt with or will go with non-native theming.
Comment 15•4 years ago
|
||
Two of the stacks here are definitely to do with native theming and the other one starting from MakeTextRun
has already been removed.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Updated•4 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•