1382232 - Cairo APIs still used from content process (independent of printing)

Status: RESOLVED FIXED

(Core :: Graphics, enhancement, P3)

Description

[Alex Gaynor [:Alex_Gaynor]]

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

Comment 1

[Milan Sreckovic [:milan] (needinfo for best results)]

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.

Comment 2

[Alex Gaynor [:Alex_Gaynor]]

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

[Jonathan Kew (:jfkthame)]

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

Comment 4

[Jeff Muizelaar [:jrmuizel]]

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?

Comment 5

[Lee Salzman [:lsalzman]]

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

Comment 6

[Jonathan Kew (:jfkthame)]

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

Comment 7

[Jim Mathies [:jimm]]

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

[Jim Mathies [:jimm]]

For Cairo, right now we are seeing:

* font access
* drawing surfaces (particularly in theming which is covered by another bug)
* printing

Comment 9

[Jim Mathies [:jimm]]

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.

Comment 10

[Milan Sreckovic [:milan] (needinfo for best results)]

Jim, did you really want to suggest this should get uplifted to 57?

Comment 11

[Jim Mathies [:jimm]]

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

Comment 12

[Milan Sreckovic [:milan] (needinfo for best results)]

Shows up in 57 tracked queries, but I added "feature" keyword now to make sure it doesn't.

Comment 13

[Firefox Bug Husbandry Bot]

https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Move_fix-optionals

Comment 14

[Bob Owen (:bobowen)]

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

Comment 15

[Bob Owen (:bobowen)]

Two of the stacks here are definitely to do with native theming and the other one starting from MakeTextRun has already been removed.