Remove the unused bbox setter in the FontFaceObject class (PR 20427 follow-up)#20869
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #20869 +/- ##
==========================================
- Coverage 62.54% 62.53% -0.01%
==========================================
Files 173 173
Lines 121238 121234 -4
==========================================
- Hits 75829 75818 -11
- Misses 45409 45416 +7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Snuffleupagus
force-pushed
the
FontFaceObject-rm-bbox-setter
branch
from
7e45b3c to
e5a37de
Compare
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/53acd96a796dfac/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/53acd96a796dfac/output.txt Total script time: 1.20 mins Published |
|
/botio browsertest |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/59e959a87ac54a5/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_browsertest from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/3e6f904d9017178/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/59e959a87ac54a5/output.txt Total script time: 18.88 mins
Image differences available at: http://54.241.84.105:8877/59e959a87ac54a5/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/3e6f904d9017178/output.txt Total script time: 25.52 mins
Image differences available at: http://54.193.163.58:8877/3e6f904d9017178/reftest-analyzer.html#web=eq.log |
…27 follow-up) The commit message for the patch in PR 20427 is pretty non-descriptive, being only a single line, however there's a bit more context in mozilla#20427 (comment) but unfortunately the details there don't really make sense. Note that the PR only changed main-thread code, but all the links are to worker-thread code!? The `FontFaceObject` class is only used on the main-thread, and when encountering a broken font we fallback to the built-in font renderer; see https://github.com/mozilla/pdf.js/blob/820b70eb25b1c6bf74f916e002d11afc49e929cf/src/display/font_loader.js#L135-L143 Hence the `FontFaceObject` class *only* needs a way to set the `disableFontFace` property, however nowhere on the main-thread do we ever update the `bbox` of a font.
Snuffleupagus
force-pushed
the
FontFaceObject-rm-bbox-setter
branch
from
e5a37de to
f1e1973
Compare
|
Rebased to |
4 participants
The commit message for the patch in PR #20427 is pretty non-descriptive, being only a single line, however there's a bit more context in #20427 (comment) but unfortunately the details there don't really make sense.
Note that the PR only changed main-thread code, but all the links are to worker-thread code!?
The
FontFaceObjectclass is only used on the main-thread, and when encountering a broken font we fallback to the built-in font renderer; seepdf.js/src/display/font_loader.js
Lines 135 to 143 in 820b70e
FontFaceObjectclass only needs a way to set thedisableFontFaceproperty, however nowhere on the main-thread do we ever update thebboxof a font.