-
Notifications
You must be signed in to change notification settings - Fork 10.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't downscale by steps images which will end up with small dimensions (bug 1445735) #16482
base: master
Are you sure you want to change the base?
Conversation
Maybe add https://bugzilla.mozilla.org/attachment.cgi?id=8958936 as a linked |
b208794
to
efef06d
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/5a6c47b79ac7b9a/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/8d8638c98cec67e/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/5a6c47b79ac7b9a/output.txt Total script time: 27.90 mins
Image differences available at: http://54.241.84.105:8877/5a6c47b79ac7b9a/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/8d8638c98cec67e/output.txt Total script time: 35.48 mins
Image differences available at: http://54.193.163.58:8877/8d8638c98cec67e/reftest-analyzer.html#web=eq.log |
efef06d
to
afa67a2
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/9e028dbde979716/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 1 Live output at: http://54.193.163.58:8877/042adaa0245dd21/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/9e028dbde979716/output.txt Total script time: 27.70 mins
Image differences available at: http://54.241.84.105:8877/9e028dbde979716/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/042adaa0245dd21/output.txt Total script time: 33.87 mins
Image differences available at: http://54.193.163.58:8877/042adaa0245dd21/reftest-analyzer.html#web=eq.log |
I'd say that the differences are acceptable. @Snuffleupagus, wdyt ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following look like, somewhat clear, regressions:
fit11-talk
pr8808-page1
, which is now too "dark" when compared with e.g. Adobe Readerissue1658-page10
src/display/canvas.js
Outdated
@@ -1188,6 +1188,16 @@ class CanvasGraphics { | |||
1 | |||
); | |||
|
|||
if (width / widthScale <= 5 || height / heightScale <= 5) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we perhaps limit this to cases where both dimensions are small, hence replace the OR with AND above?
The "missing" tip of the curly braces on the images are only one pixel which was gray and is now white.
I opened the pdf in nightly and in Acrobat on mac at 100% and I can't see any differences (that said I'm not that young... I'm likely not a reference).
At normal scales there is no problem but I agree at small scales (when the text is really unreadable) some lines become invisible and it's the same in Acrobat. My feeling is that the images we've in tests aren't representative of what the users have in real life... (on linux, for example, we use a headless mode). It's just a feeling, hence I can be completely wrong here. |
I think that it'd be interesting to see what using AND does to the test-results, since then we'd only consider "truly" small images. Otherwise narrow images are affected as well, which might not actually be what you want here? |
It could perhaps be connected to the |
I'll test tomorrow with one of my old screen with a devicePixelRatio equal to 1 to see if there's something noticeable. |
0b33db6
to
500a08e
Compare
/botio browsertest |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @calixteman received. Current queue size: 0 Live output at: http://54.241.84.105:8877/64fc9166533d5fc/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_browsertest from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/d5d30514e668dcd/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/64fc9166533d5fc/output.txt Total script time: 23.09 mins
Image differences available at: http://54.241.84.105:8877/64fc9166533d5fc/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/d5d30514e668dcd/output.txt Total script time: 23.42 mins
Image differences available at: http://54.193.163.58:8877/d5d30514e668dcd/reftest-analyzer.html#web=eq.log |
500a08e
to
aaeb341
Compare
aaeb341
to
0fd2f12
Compare
What became of this testing, since I'm still not sure that this patch is necessarily always better (e.g. in the mentioned case)? |
I wrote something but I obviously forgot to click on the "Comment" button... Anyway on some pdfs the lines weren't correctly rendered when compared with Acrobat rendering (the black was too gray). |
/botio browsertest |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @calixteman received. Current queue size: 1 Live output at: http://54.241.84.105:8877/68cdedbe61a9838/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_browsertest from @calixteman received. Current queue size: 1 Live output at: http://54.193.163.58:8877/14edc2a90279730/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/68cdedbe61a9838/output.txt Total script time: 20.16 mins
Image differences available at: http://54.241.84.105:8877/68cdedbe61a9838/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/14edc2a90279730/output.txt Total script time: 23.70 mins
Image differences available at: http://54.193.163.58:8877/14edc2a90279730/reftest-analyzer.html#web=eq.log |
No description provided.