1872545 - Make COLR-font palette caching more effective

Status: RESOLVED FIXED

(Core :: Graphics: Text, enhancement)

Description

[Jonathan Kew [:jfkthame]]

In bug 1816902, we created a simple MRU cache for COLR font palettes so as to avoid constantly re-creating them when a number of emoji characters occured within a single textrun.

However, this cache only operates at the level of rendering an individual textrun. In a case like bug 1870986, there are hundreds/thousands of individual emoji glyphs being painted, but they're each painted as a separate canvas2d fillText call, so they're processed as separate textruns and don't benefit from the palette cache in TextRunDrawParams. As a result, palette resolution shows up as a substantial part of the painting time.

We can improve this, therefore, if we hoist the palette cache out of TextRunDrawParams and instead attach it to the context (nsPresContext in the case of HTML text, or CanvasRenderingContext2D in the case of canvas text), and pass a reference down to the actual glyph-painting code. This way, the palette can be resolved once and then used to paint all the emoji glyphs in the example, even though they're rendered via individual single-character paint calls.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1872545 - Hoist color-font palette cache out of TextRunDrawParams to the nsPresContext or CanvasRenderingContext2D, for greater effectiveness. r=#gfx-reviewers,lsalzman

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e959df5e9484
Hoist color-font palette cache out of TextRunDrawParams to the nsPresContext or CanvasRenderingContext2D, for greater effectiveness. r=gfx-reviewers,lsalzman

Comment 3

[Narcis Beleuzu [:NarcisB]]

Backed out for bustages on COLRFonts.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/d70b7a3c970b26399d589b1ca8973b8f49c99520
Log link: https://treeherder.mozilla.org/logviewer?job_id=441886978&repo=autoland&lineNumber=80788

Comment 4

[Jonathan Kew [:jfkthame]]

Re-landing with #includes adjusted, which should fix the fuzzer bustage.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/395cc57551ec
Hoist color-font palette cache out of TextRunDrawParams to the nsPresContext or CanvasRenderingContext2D, for greater effectiveness. r=gfx-reviewers,lsalzman

Comment 6

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/395cc57551ec

Comment 7

[Cristian Tuns]

Backed out for causing build bustages in rust.mk

• Backout link
• Push with failures
• Failure Log
• Failure line: gmake[4]: *** [/builds/worker/checkouts/gecko/config/makefiles/rust.mk:537: force-cargo-test-run] Error 101

Comment 8

[Cristian Tuns]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/114b1b228dac

Comment 9

[Jonathan Kew [:jfkthame]]

Ugh, it seems like bindgen is getting confused by something in the gfx includes, but it's not obvious to me exactly what's causing the breakage.

After some experimenting it looks like we can avoid the issue if we split the PaletteCache out from COLRFonts into a separate .cpp and .h, and avoid exposing gfxFontEntry to bindgen. Just waiting for a try build to confirm this builds everywhere, then I'll re-land the updated patch.

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21beaf6ba627
Hoist color-font palette cache out of TextRunDrawParams to the nsPresContext or CanvasRenderingContext2D, for greater effectiveness. r=gfx-reviewers,lsalzman

Comment 11

[Iulian Moraru]

Backed out for causing windows build bustages on FontPaletteCache.cpp.
So far, this only happened on Windows 2012 x64 debug build-win64-non-unified/plain Bp-nu.

• Push with failures

• Failure log

• Backout link

[task 2024-01-03T23:18:05.921Z] 23:18:05     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/gfx/thebes'
[task 2024-01-03T23:18:05.924Z] 23:18:05     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang-cl -fms-compatibility-version=19.29 -Xclang -std=c++17 -Xclang -ivfsoverlay -Xclang /builds/worker/fetches/vs/overlay.yaml -FoFontPaletteCache.obj -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -guard:cf -DNDEBUG=1 -DTRIMMED=1 -DUNICODE -D_UNICODE -D_CRT_RAND_S -DCERT_CHAIN_PARA_HAS_EXTRA_FIELDS -D_SECURE_ATL -DCHROMIUM_BUILD -DU_STATIC_IMPLEMENTATION -DWIN32 -D_WIN32 -D_WINDOWS -DWIN32_LEAN_AND_MEAN -DMOZ_ENABLE_D3D10_LAYER -DGRAPHITE2_STATIC -DWINAPI_NO_BUNDLED_LIBRARIES -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -DSTATIC_EXPORTABLE_JS_API -I/builds/worker/checkouts/gecko/gfx/thebes -I/builds/worker/workspace/obj-build/gfx/thebes -I/builds/worker/workspace/obj-build/ipc/ipdl/_ipdlheaders -I/builds/worker/checkouts/gecko/ipc/chromium/src -I/builds/worker/workspace/obj-build/security/rlbox -I/builds/worker/checkouts/gecko/dom/base -I/builds/worker/checkouts/gecko/dom/media/platforms/apple -I/builds/worker/checkouts/gecko/dom/xml -I/builds/worker/checkouts/gecko/gfx/cairo/cairo/src -I/builds/worker/checkouts/gecko/widget/gtk -I/builds/worker/checkouts/gecko/gfx/skia -I/builds/worker/checkouts/gecko/gfx/skia/skia -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -MD -FI /builds/worker/workspace/obj-build/mozilla-config.h -DMOZILLA_CLIENT -fcrash-diagnostics-dir=/builds/worker/artifacts -TP -Zc:sizedDealloc- -D_HAS_EXCEPTIONS=0 -Gy -Zc:inline -Gw -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -Xclang -load -Xclang /builds/worker/workspace/obj-build/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O2 -Oy- -Werror -W3 -Wbitfield-enum-conversion -Wdeprecated-this-capture -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wc++2a-compat -Wenum-compare-conditional -Wenum-float-conversion -Wvolatile -Wno-error=deprecated -Wno-error=deprecated-anon-enum-enum-conversion -Wno-error=deprecated-enum-enum-conversion -Wno-error=deprecated-pragma -Wno-error=deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-microsoft-enum-value -Wno-microsoft-include -Wno-invalid-noreturn -Wno-inconsistent-missing-override -Wno-implicit-exception-spec-mismatch -Wno-microsoft-exception-spec -Wno-unused-local-typedef -Wno-ignored-attributes -Wno-used-but-marked-unused -Wno-psabi -Wthread-safety -Wno-error=builtin-macro-redefined -Wno-vla-cxx-extension -Wno-unknown-warning-option -Werror=switch -fno-strict-aliasing -Xclang -ffp-contract=off  -Xclang -MP -Xclang -dependency-file -Xclang .deps/FontPaletteCache.obj.pp -Xclang -MT -Xclang FontPaletteCache.obj   /builds/worker/checkouts/gecko/gfx/thebes/FontPaletteCache.cpp
[task 2024-01-03T23:18:05.924Z] 23:18:05    ERROR -  /builds/worker/checkouts/gecko/gfx/thebes/FontPaletteCache.cpp(15,10): error: use of undeclared identifier 'gfx'
[task 2024-01-03T23:18:05.925Z] 23:18:05     INFO -     15 | nsTArray<gfx::sRGBColor>* mozilla::gfx::PaletteCache::GetPaletteFor(
[task 2024-01-03T23:18:05.925Z] 23:18:05     INFO -        |          ^
[task 2024-01-03T23:18:05.926Z] 23:18:05    ERROR -  /builds/worker/checkouts/gecko/gfx/thebes/FontPaletteCache.cpp(28,10): error: cannot initialize return object of type 'int *' with an rvalue of type 'Pointer' (aka 'nsTArray<mozilla::gfx::sRGBColor> *')
[task 2024-01-03T23:18:05.926Z] 23:18:05     INFO -     28 |   return entry.Data().mPalette.get();
[task 2024-01-03T23:18:05.927Z] 23:18:05     INFO -        |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2024-01-03T23:18:05.927Z] 23:18:05     INFO -  2 errors generated.
[task 2024-01-03T23:18:05.927Z] 23:18:05    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:690: FontPaletteCache.obj] Error 1
[task 2024-01-03T23:18:05.928Z] 23:18:05     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/gfx/thebes'

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/740351987a4e
Hoist color-font palette cache out of TextRunDrawParams to the nsPresContext or CanvasRenderingContext2D, for greater effectiveness. r=gfx-reviewers,lsalzman

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/740351987a4e