[api-minor] Re-factor the disableFontFace fallback value, and skip initializing factories with useWorkerFetch set
#13525
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
At this point in time, the `apiCompatibilityParams` is essentially unused with the sole exception of the `disableFontFace` handling for Node.js environments. Given that `isNodeJS` is a constant now (originally it was a function), we can simply set the correct fallback value for `disableFontFace` directly in the API and clean-up the code a bit here.
…he `useWorkerFetch` API option is set Given that there's no fallback on the worker-thread, it shouldn't be necessary to initialize `CMapReaderFactory`/`StandardFontDataFactory` when `useWorkerFetch = true` is set. Slightly unrelated, but this patch also ensures that the `useSystemFonts` default value only does the `isNodeJS` check in builds where that's actually necessary.
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://54.67.70.0:8877/5460c3e33cd1556/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 1 Live output at: http://3.101.106.178:8877/f58de05258e0f56/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/5460c3e33cd1556/output.txt Total script time: 25.90 mins
Image differences available at: http://54.67.70.0:8877/5460c3e33cd1556/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://3.101.106.178:8877/f58de05258e0f56/output.txt Total script time: 29.67 mins
Image differences available at: http://3.101.106.178:8877/f58de05258e0f56/reftest-analyzer.html#web=eq.log |
timvandermeij
approved these changes
Jun 9, 2021
|
Thank you for cleaning this up! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[api-minor] Set the
disableFontFacefallback value directly in the APIAt this point in time, the
apiCompatibilityParamsis essentially unused with the sole exception of thedisableFontFacehandling for Node.js environments.Given that
isNodeJSis a constant now (originally it was a function), we can simply set the correct fallback value fordisableFontFacedirectly in the API and clean-up the code a bit here.Don't initialize
CMapReaderFactory/StandardFontDataFactorywhen theuseWorkerFetchAPI option is setGiven that there's no fallback on the worker-thread, it shouldn't be necessary to initialize
CMapReaderFactory/StandardFontDataFactorywhenuseWorkerFetch = trueis set.Slightly unrelated, but this patch also ensures that the
useSystemFontsdefault value only does theisNodeJScheck in builds where that's actually necessary.