-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Attempt to fallback to a default font, for non-available ones, in PartialEvaluator.loadFont
#11218
Conversation
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/2ff030e7576aecf/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/4b6eb95bbe54810/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/2ff030e7576aecf/output.txt Total script time: 17.76 mins
Image differences available at: http://54.67.70.0:8877/2ff030e7576aecf/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/4b6eb95bbe54810/output.txt Total script time: 26.25 mins
Image differences available at: http://54.215.176.217:8877/4b6eb95bbe54810/reftest-analyzer.html#web=eq.log |
f924e65
to
683fe57
Compare
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/52474df625220b6/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/af785733b9ed0ab/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/52474df625220b6/output.txt Total script time: 17.76 mins
Image differences available at: http://54.67.70.0:8877/52474df625220b6/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/af785733b9ed0ab/output.txt Total script time: 25.96 mins
Image differences available at: http://54.215.176.217:8877/af785733b9ed0ab/reftest-analyzer.html#web=eq.log |
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/cc57cf2084ad739/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/cc57cf2084ad739/output.txt Total script time: 1.67 mins Published |
@timvandermeij Please note that I'm not entirely sure about the way forward here! |
src/core/evaluator.js
Outdated
warn('fontRes not available'); | ||
return errorFont(); | ||
warn(`fontRes "${fontName}" not available.`); | ||
// Attempting to fallback, see the code-path just below... |
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.
I think we should put this in the warning here/below instead (or both) so it's clear from the console that this is a recovery path. We did this before for some other recovery messages.
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.
Good point; I've removed the (frankly) unnecessary warning duplication, and also added different warning messages depending on whether the fallback/Error paths are taken.
The patch itself looks clear to me and seeing the reference images this does seem to address the issue. I have one comment above. Looking at the other patch after the updates, I don't see too many differences anymore. |
Since my intention wasn't to step all over the other patch, and I threw this one together before seeing the updated version, I'm just not sure how we should proceed here!? However, the other one still has a couple of issues with the implementation (it's not as complete as it ought to be, and it also abuses |
I would say we can continue with this more complete patch, but use https://help.github.com/en/articles/creating-a-commit-with-multiple-authors to credit both authors for the work. |
683fe57
to
1a3ec74
Compare
…rtialEvaluator.loadFont` This handles the two different ways that fonts can be loaded, either by Name (which is the common case) or by Reference. Furthermore, this also takes the `ignoreErrors` option into account when deciding whether to fallback or Error. Finally, by creating a minimal but valid Font dictionary, there's no special-cases necessary in any of the font parsing code. Co-authored-by: huzjakd <[email protected]> Co-Authored-By: Jonas Jenwald <[email protected]>
1a3ec74
to
94171d9
Compare
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/36e499ea441ae23/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/fb378d4ef415a88/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/36e499ea441ae23/output.txt Total script time: 17.78 mins
Image differences available at: http://54.67.70.0:8877/36e499ea441ae23/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/fb378d4ef415a88/output.txt Total script time: 26.35 mins
Image differences available at: http://54.215.176.217:8877/fb378d4ef415a88/reftest-analyzer.html#web=eq.log |
Nice work! /botio makeref |
From: Bot.io (Linux m4)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://54.67.70.0:8877/28b2e0ed4d8a40f/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_makeref from @timvandermeij received. Current queue size: 0 Live output at: http://54.215.176.217:8877/9a94ba642a9779a/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.67.70.0:8877/28b2e0ed4d8a40f/output.txt Total script time: 16.16 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.215.176.217:8877/9a94ba642a9779a/output.txt Total script time: 24.43 mins
|
Fixes #11068
Fixes #11162