Closed Bug 1600855 Opened 1 year ago Closed 1 year ago

make SVGTextFrame avoid getting glyph advances that cross text frames, and re-enable letter/word-spacing

Categories

(Core :: SVG, defect, P3)

defect

Tracking

()

RESOLVED FIXED
mozilla73
Tracking Status
firefox73 --- fixed

People

(Reporter: heycam, Assigned: heycam)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files)

Now that we have PropertyProvider objects, which can only provide spacing information within a single text frame, we can't call into gfxTextRun::GetAdvanceWidth with a Range that spans text frames. Consequently, we need to remove CharIterator::{GetGlyphAdvance,GetPartialGlyphAdvance} and replace their uses with code the explicitly sums up the advances for each character in a ligature group / cluster.

Summary: make SVGTextFrame avoid getting glyph advances that cross text frames → make SVGTextFrame avoid getting glyph advances that cross text frames, and re-enable letter/word-spacing
Assignee: nobody → cam
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a9c150ebbf2b
Avoid SVG text calling gfxTextRun::GetAdvanceWidth for ranges that cross text frame boundaries. r=longsonr
https://hg.mozilla.org/integration/autoland/rev/a3473bbf2b42
Re-enable letter-spacing and word-spacing on SVG text. r=longsonr

dev-doc-needed to update the Firefox version where this was re-enabled.

Keywords: dev-doc-needed

Backed out 2 changesets (Bug 1600855) for bustages complaining about SVGTextFrame.cpp

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&fromchange=a3473bbf2b429bab91756a80c6f2c48f8f008b8c&tochange=a01a5184808321f284f20278e1e6e709621a2942&selectedJob=279418069

Backout link: https://hg.mozilla.org/integration/autoland/rev/a01a5184808321f284f20278e1e6e709621a2942

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=279418069&repo=autoland&lineNumber=36831

[task 2019-12-03T21:08:25.846Z] 21:08:25 INFO - make[4]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/layout/svg'
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - /builds/worker/fetches/sccache/sccache /builds/worker/fetches/gcc/bin/g++ -std=gnu++17 -o Unified_cpp_layout_svg0.o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/layout/svg -I/builds/worker/workspace/build/src/obj-firefox/layout/svg -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/widget -I/builds/worker/workspace/build/src/layout/base -I/builds/worker/workspace/build/src/layout/generic -I/builds/worker/workspace/build/src/layout/painting -I/builds/worker/workspace/build/src/layout/style -I/builds/worker/workspace/build/src/layout/xul -I/builds/worker/workspace/build/src/dom/base -I/builds/worker/workspace/build/src/dom/svg -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Wall -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wsign-compare -Wtype-limits -Wunreachable-code -Wwrite-strings -Wno-invalid-offsetof -Wduplicated-cond -Wimplicit-fallthrough -Wno-error=maybe-uninitialized -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=coverage-mismatch -Wno-error=free-nonheap-object -Wformat -Wformat-overflow=2 -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fno-aligned-new -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -freorder-blocks -O2 -fno-omit-frame-pointer -funwind-tables -Werror -MD -MP -MF .deps/Unified_cpp_layout_svg0.o.pp Unified_cpp_layout_svg0.cpp
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - In file included from Unified_cpp_layout_svg0.cpp:74:0:
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - /builds/worker/workspace/build/src/layout/svg/SVGTextFrame.cpp: In member function 'bool mozilla::CharIterator::MatchesFilter() const':
[task 2019-12-03T21:08:25.854Z] 21:08:25 ERROR - /builds/worker/workspace/build/src/layout/svg/SVGTextFrame.cpp:2378:1: error: control reaches end of non-void function [-Werror=return-type]
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - }
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - ^
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - cc1plus: all warnings being treated as errors
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - /builds/worker/workspace/build/src/config/rules.mk:804: recipe for target 'Unified_cpp_layout_svg0.o' failed
[task 2019-12-03T21:08:25.854Z] 21:08:25 ERROR - make[4]: *** [Unified_cpp_layout_svg0.o] Error 1
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - make[4]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/layout/svg'
[task 2019-12-03T21:08:25.854Z] 21:08:25 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'layout/svg/target-objects' failed
[task 2019-12-03T21:08:25.855Z] 21:08:25 ERROR - make[3]: *** [layout/svg/target-objects] Error 2
[task 2019-12-03T21:08:25.855Z] 21:08:25 INFO - make[3]: *** Waiting for unfinished jobs....

Flags: needinfo?(cam)

Thanks, I guess that's just a warning in my local build and I didn't see it.

Flags: needinfo?(cam)
Pushed by cmccormack@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4acdc6bc23e2
Avoid SVG text calling gfxTextRun::GetAdvanceWidth for ranges that cross text frame boundaries. r=longsonr
https://hg.mozilla.org/integration/autoland/rev/31ead0a3bca3
Re-enable letter-spacing and word-spacing on SVG text. r=longsonr
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla73
Regressions: 1601824

I've made a PR to the BCD repo to change this from 72 to 73.

Thanks Rachel!

Regressions: 1678947
You need to log in before you can comment on or make changes to this bug.