Closed
Bug 1382232
Opened 7 years ago
Closed 3 years ago
Cairo APIs still used from content process (independent of printing)
Categories
(Core :: Graphics, enhancement, P3)
Tracking
()
People
(Reporter: Alex_Gaynor, Unassigned)
References
(Blocks 1 open bug)
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•7 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•7 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•7 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•7 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•7 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•7 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•7 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•7 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•7 years ago
|
status-firefox57:
--- → wontfix
Updated•7 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•7 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•6 years ago
|
||
https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals
status-firefox59:
--- → ?
Updated•5 years ago
|
Summary: Cairo APIs still used from content process → Cairo APIs still used from content process (independent of printing)
Comment 14•3 years ago
|
||
I have a suspicion that these have already been dealt with or will go with non-native theming.
Comment 15•3 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: 3 years ago
Resolution: --- → FIXED
Updated•3 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•