Correctly detect more cases of non-embedded Arial Black fonts (issue 7835) #7839
Conversation
…7835) This patch adds support for non-embedded Arial Black fonts, that use a `Arial-Black...` format for the font names. Also, this patch changes `canvas.js` such that we always render Arial Black fonts with the maximum weight, which actually improves a number of existing test-cases. This should thus explain the test "failures", which are clear improvements compared with e.g. Adobe Reader. Fixes 7835.
|
/botio test |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/ab4f4045ed65d8a/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/2f633eb06433d96/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/ab4f4045ed65d8a/output.txt Total script time: 25.92 mins
|
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/2f633eb06433d96/output.txt Total script time: 26.97 mins
Image differences available at: http://107.21.233.14:8877/2f633eb06433d96/reftest-analyzer.html#web=eq.log |
| var bold = fontObj.black ? (fontObj.bold ? '900' : 'bold') : | ||
| (fontObj.bold ? 'bold' : 'normal'); | ||
|
|
||
| var bold = fontObj.black ? '900' : (fontObj.bold ? 'bold' : 'normal'); |
There was a problem hiding this comment.
Is Arial-Black == Arial-Black-Bold?
There was a problem hiding this comment.
In practice this assumption seems to give better, i.e. closer to Adobe Reader, rendering in PDF.js for me locally on Windows (even if it doesn't manifest itself on the bots).
It actually seems that Adobe Reader might simply treat (non-embedded) ArialBlack-Bold the same as ArialBlack.
There was a problem hiding this comment.
Having looked at this once more, ordinary ArialBlack should use a font weight that maps to black to render correctly (see https://developer.mozilla.org/en-US/docs/Web/CSS/font-weight#Common_weight_name_mapping).
However, ArialBlack,Bold should ideally map to an even heavier font weight, but given that no such one exists (in CSS), we will thus simply fallback to normal ArialBlack instead.
To me, this seems to give the best possible rendering (at least on Windows) for these non-embedded fonts; otherwise normal ArialBlack actually renders too light.
Edit: Perhaps I should add a comment about the specific handling of font.black here?
|
Thanks for the review! /botio makeref |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.22.172.223:8877/f6e68b70fcc2540/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_makeref from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/ff37c2cb8bf70e2/output.txt |
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/f6e68b70fcc2540/output.txt Total script time: 26.02 mins
|
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/ff37c2cb8bf70e2/output.txt Total script time: 26.60 mins
|
Correctly detect more cases of non-embedded Arial Black fonts (issue 7835)
This patch adds support for non-embedded Arial Black fonts, that use a
Arial-Black...format for the font names.Also, this patch changes
canvas.jssuch that we always render Arial Black fonts with the maximum weight, which actually improves a number of existing test-cases. This should thus explain the test "failures", which are clear improvements compared with e.g. Adobe Reader.Fixes #7835.