-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Add an option to enable/disable hardware acceleration (bug 1902012) #18238
Conversation
/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/9f3ddb12d4070bc/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/caa82771b7db49d/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/9f3ddb12d4070bc/output.txt Total script time: 28.87 mins
Image differences available at: http://54.241.84.105:8877/9f3ddb12d4070bc/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/caa82771b7db49d/output.txt Total script time: 41.61 mins
Image differences available at: http://54.193.163.58:8877/caa82771b7db49d/reftest-analyzer.html#web=eq.log |
/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/95003cd5491e9e1/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/c03d7a10b723af1/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/95003cd5491e9e1/output.txt Total script time: 29.04 mins
Image differences available at: http://54.241.84.105:8877/95003cd5491e9e1/reftest-analyzer.html#web=eq.log |
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.
r=me, with the final comments addressed; thank you!
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/c03d7a10b723af1/output.txt Total script time: 44.19 mins
Image differences available at: http://54.193.163.58:8877/c03d7a10b723af1/reftest-analyzer.html#web=eq.log |
/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/26467fb15f49b35/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @calixteman received. Current queue size: 0 Live output at: http://54.193.163.58:8877/e164b4315fc63ec/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/26467fb15f49b35/output.txt Total script time: 28.62 mins
Image differences available at: http://54.241.84.105:8877/26467fb15f49b35/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/e164b4315fc63ec/output.txt Total script time: 43.71 mins
Image differences available at: http://54.193.163.58:8877/e164b4315fc63ec/reftest-analyzer.html#web=eq.log |
Why isn't the option exposed in Chrome's options? |
While disabling HWA unconditionally will certainly fix some issues associated with https://bugzilla.mozilla.org/show_bug.cgi?id=1902012, I wonder whether doing this unconditionally by default is the correct approach to take. Do we have any data that shows that turning off HWA unconditionally is going to have a larger positive impact than the performance loss on systems where HWA outperforms software rendering? Browsers usually have hardware blocklists to selectively disable HWA on systems that are known to have issues with HWA. Additionally, users can override the browser defaults, This patch does not only add an option to disable HWA, it already disables HWA in situations where the PDF.js code is expected to read back data, by specifying |
@Rob--W, I wasn't so happy to disable hwa for the exact reasons you gave (and tbh I was pretty reluctant). |
If there are doubts about disabling unconditionally, could we change the default to true (so that the use of HWA is controlled by the browser/user)? The mere existence of the setting now enables others to opt out if they want to deploy PDF.js in an environment where HWA is an issue. PDF.js + HWA has been out in the wild for many years at this point, whereas HWA off is a new change here. I think that it would be lower risk to default to true (since it is the same as before), and then experiment with turning it off in cohorts. If that shows a noticeable performance difference, then the default can change. |
IIRC canvas acceleration has only been available recently in Firefox, so effectively it's only a short time HWA has been really enabled in Firefox. |
It's indeed relatively new, but not trivial either (1.5 years). HWA canvas has been enabled on Linux+macOS since Firefox 110 (https://bugzilla.mozilla.org/show_bug.cgi?id=1806058), on Android since Firefox 113 (https://bugzilla.mozilla.org/show_bug.cgi?id=1825182). Interestingly, canvas HWA seems not enabled on Windows: https://searchfox.org/mozilla-central/rev/94f839e924ba6c69f3e0d062d4c6cc4fee7cad5b/modules/libpref/init/StaticPrefList.yaml#5706-5715 In Chrome, HWA canvas has been around for 12 years, judging by the announcement at https://developer.chrome.com/blog/taking-advantage-of-gpu-acceleration-in-the-2d-canvas/ If there is a desire to turn off HWA by default, I'd like it to be enabled by default in the Chrome extension (I can submit a patch), unless there is data that shows that HWA off-by-default offers a better experience.
Is this only in Firefox, or does it also cover other browser engines such as Chromium? |
At least I can tell you that I've been having issues when HWA was turned OFF in pdf.js running in Chrome as listed in issue 18242. That's why I asked about making it a user controlled option in Chrome. After enabling it, the weird issues have gone away (for the meantime). |
This commit introduces a new option `softwareRender` to allow users to manually configure disabling GPU instead of guessing the browser. This cleans up the temporary fix introduced in the previous commit, which we have decided to keep as the PDF.js team has addressed the issue and has chosen to do the same via a preference (mozilla/pdf.js#18238 on GitHub).
No description provided.