Fix text spacing with vertical fonts (#6387) #6391
Conversation
| "md5": "08c39ac6d0aab1596e6e59793eaf3ee4", | ||
| "rounds": 1, | ||
| "type": "eq" | ||
| }, |
There was a problem hiding this comment.
If this patch fixes the textLayer as well, how about also adding a text test, since we currently don't have that many of those, e.g.
{ "id": "issue6387-text",
"file": "pdfs/issue6387.pdf",
"md5": "08c39ac6d0aab1596e6e59793eaf3ee4",
"rounds": 1,
"type": "text"
},There was a problem hiding this comment.
is this a second block in addition, or can i mark a test as eq and text?
There was a problem hiding this comment.
You'll have to add another block, since there isn't support for specifying multiple types for one test.
CodingFabian
force-pushed
the
fix-vertical-kernings-6387
branch
2 times, most recently
from
38e1297
to
8a8f6d6
Compare
| "rounds": 1, | ||
| "type": "eq" | ||
| }, | ||
| { "id": "issue6387", |
There was a problem hiding this comment.
The ids must be unique, otherwise the reference image for the second test will replace the first one and thus cause test failures. Please change it to "id": "issue6387-text".
CodingFabian
force-pushed
the
fix-vertical-kernings-6387
branch
from
8a8f6d6
to
7d5e00c
Compare
|
/botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/7518aedf6938602/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/7518aedf6938602/output.txt Total script time: 0.65 mins Published |
|
/botio test |
From: Bot.io (Linux)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.21.233.14:8877/b289d90d976bd43/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @timvandermeij received. Current queue size: 0 Live output at: http://107.22.172.223:8877/3b3566cd159f0a8/output.txt |
From: Bot.io (Windows)FailedFull output at http://107.22.172.223:8877/3b3566cd159f0a8/output.txt Total script time: 18.79 mins
Image differences available at: http://107.22.172.223:8877/3b3566cd159f0a8/reftest-analyzer.html#web=eq.log |
From: Bot.io (Linux)FailedFull output at http://107.21.233.14:8877/b289d90d976bd43/output.txt Total script time: 19.89 mins
Image differences available at: http://107.21.233.14:8877/b289d90d976bd43/reftest-analyzer.html#web=eq.log |
CodingFabian
force-pushed
the
fix-vertical-kernings-6387
branch
from
7d5e00c
to
440774b
Compare
|
note that the test failures are expected, because the previous rendering is incorrect (compared to other pdf viewers, for example the height of the vertical font on page 3 figure 4 should be the same for all except the first column) |
|
@yurydelendik Could you review this PR? |
|
@CodingFabian Could you rebase this PR to resolve the merge conflicts? |
According to the PDF spec 5.3.2, a positive value means in horizontal, that the next glyph is further to the left (so narrower), and in vertical that it is further down (so wider). This change fixes the way PDF.js has interpreted the value.
CodingFabian
force-pushed
the
fix-vertical-kernings-6387
branch
from
440774b
to
2564827
Compare
|
conflict in gitignore :-) rebased. |
|
/botio makeref |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://107.22.172.223:8877/5b25edb822b626e/output.txt |
From: Bot.io (Linux)ReceivedCommand cmd_makeref from @brendandahl received. Current queue size: 0 Live output at: http://107.21.233.14:8877/964b2fb8465bd9e/output.txt |
Fix text spacing with vertical fonts (#6387)
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/964b2fb8465bd9e/output.txt Total script time: 19.55 mins
|
From: Bot.io (Windows)SuccessFull output at http://107.22.172.223:8877/5b25edb822b626e/output.txt Total script time: 19.94 mins
|
…es were what I was missing before
According to the PDF spec 5.3.2, a positive value means in horizontal,
that the next glyph is further to the left (so narrower), and in
vertical that it is further down (so wider).
This change fixes the way PDF.js has interpreted the value.
Fixes #6387.