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)

Unspecified
Windows
enhancement

Tracking

()

RESOLVED FIXED
Tracking Status
firefox-esr52 --- wontfix
firefox56 --- wontfix
firefox57 --- wontfix
firefox58 --- wontfix
firefox59 --- wontfix

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
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.
(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)
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)
(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)
(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)
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
For Cairo, right now we are seeing: * font access * drawing surfaces (particularly in theming which is covered by another bug) * printing
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.
Blocks: 1383524
Jim, did you really want to suggest this should get uplifted to 57?
Flags: needinfo?(jmathies)
Whiteboard: [gfx-noted → [gfx-noted]
(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
Summary: Cairo APIs still used from content process → Cairo APIs still used from content process (independent of printing)

I have a suspicion that these have already been dealt with or will go with non-native theming.

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
You need to log in before you can comment on or make changes to this bug.