From 10cb81e81a3782b56ced3d00d7f5b912841b80a2 Mon Sep 17 00:00:00 2001 From: toofar Date: Sat, 23 Sep 2023 16:15:31 +1200 Subject: Disable accelerated 2d canvas by default. Ideally we would only enable this if we detected Intel graphics. But that kind of feature detection is difficult to do cross platform. It's easy to do with Qt (version.py) ... but it needs a QApplication and we are trying to figure out what CLI args to pass to Qt, so we can't do that. So just disable it for everyone for now. TODO: * tests * change to tri-state option, auto by default and make auto look at the webengine version and on for Qt6 < 6.7.0 --- qutebrowser/config/configdata.yml | 14 ++++++++++++++ qutebrowser/config/qtargs.py | 4 ++++ 2 files changed, 18 insertions(+) diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index e8eba9de3..f9058e875 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -385,6 +385,20 @@ qt.workarounds.locale: However, It is expected that distributions shipping QtWebEngine 5.15.3 follow up with a proper fix soon, so it is disabled by default. +qt.workarounds.disable_accelerated_2d_canvas: + default: true + type: Bool + backend: QtWebEngine + restart: true + desc: >- + Disable accelerated 2d canvas to avoid graphical glitches. + + On some setups graphical issues can occur on sites like Google sheets + and PDF.js. These don't occur when accelerated 2d canvas is turned off, + so we do that by default. + + So far these glitches only occur on some Intel graphics devices. + ## auto_save auto_save.interval: diff --git a/qutebrowser/config/qtargs.py b/qutebrowser/config/qtargs.py index 7513554b3..63c1c6b0a 100644 --- a/qutebrowser/config/qtargs.py +++ b/qutebrowser/config/qtargs.py @@ -324,6 +324,10 @@ _WEBENGINE_SETTINGS: Dict[str, Dict[Any, Optional[str]]] = { 'auto': '--enable-experimental-web-platform-features' if machinery.IS_QT5 else None, }, + 'qt.workarounds.disable_accelerated_2d_canvas': { + True: '--disable-accelerated-2d-canvas', + False: None, + }, } -- cgit v1.2.3-54-g00ecf From 2e961080a85d660148937ee8f0f6b3445a8f2c01 Mon Sep 17 00:00:00 2001 From: toofar Date: Sat, 23 Sep 2023 17:10:54 +1200 Subject: Make accelerated 2d canvas setting tristate with auto I would like to be able to disable this workound for new enough chromium versions (we still need to test that chrome 111 will be fixed, but we can always bump it up later). I also wanted to use the declarative mapping `_WEBENGINE_SETTINGS` instead of adding a new conditional in `_qtwebengine_args`, because that just seems nicer. So I changed `_WEBENGINE_SETTINGS` so that the value could also be a function. The idea is that the function returns on of the other values based on some feature detection. This also required passing down the args being used for feature detection in the calling method already. I feel like that dict is being a bit abused here and if the entries could be turned into objects with a bit more consistency it might be nice. But this isn't too bad, right? Had to change the `test_qt_args` test to be a superset test instead of exact match because the new `--disable-accelerated-2d-canvas` arg was showing up in the results based on whatever Qt version you happen to be running the tests on. --- doc/help/settings.asciidoc | 21 +++++++++++++++++ qutebrowser/config/configdata.yml | 9 +++++-- qutebrowser/config/qtargs.py | 49 ++++++++++++++++++++++++++++++++------- tests/unit/config/test_qtargs.py | 32 ++++++++++++++++++++++++- 4 files changed, 99 insertions(+), 12 deletions(-) diff --git a/doc/help/settings.asciidoc b/doc/help/settings.asciidoc index 9bae037f2..de42839ce 100644 --- a/doc/help/settings.asciidoc +++ b/doc/help/settings.asciidoc @@ -303,6 +303,7 @@ |<>|Force a Qt platformtheme to use. |<>|Force software rendering for QtWebEngine. |<>|Turn on Qt HighDPI scaling. +|<>|Disable accelerated 2d canvas to avoid graphical glitches. |<>|Work around locale parsing issues in QtWebEngine 5.15.3. |<>|Delete the QtWebEngine Service Worker directory on every start. |<>|When/how to show the scrollbar. @@ -4001,6 +4002,26 @@ Type: <> Default: +pass:[false]+ +[[qt.workarounds.disable_accelerated_2d_canvas]] +=== qt.workarounds.disable_accelerated_2d_canvas +Disable accelerated 2d canvas to avoid graphical glitches. +On some setups graphical issues can occur on sites like Google sheets and PDF.js. These don't occur when accelerated 2d canvas is turned off, so we do that by default. +So far these glitches only occur on some Intel graphics devices. + +This setting requires a restart. + +This setting is only available with the QtWebEngine backend. + +Type: <> + +Valid values: + + * +always+: Disable accelerated 2d canvas + * +auto+: Disable on Qt6 < 6.6.0, enable otherwise + * +never+: Enable accelerated 2d canvas + +Default: +pass:[auto]+ + [[qt.workarounds.locale]] === qt.workarounds.locale Work around locale parsing issues in QtWebEngine 5.15.3. diff --git a/qutebrowser/config/configdata.yml b/qutebrowser/config/configdata.yml index f9058e875..e57b25d2a 100644 --- a/qutebrowser/config/configdata.yml +++ b/qutebrowser/config/configdata.yml @@ -386,8 +386,13 @@ qt.workarounds.locale: follow up with a proper fix soon, so it is disabled by default. qt.workarounds.disable_accelerated_2d_canvas: - default: true - type: Bool + type: + name: String + valid_values: + - always: Disable accelerated 2d canvas + - auto: Disable on Qt6 < 6.6.0, enable otherwise + - never: Enable accelerated 2d canvas + default: auto backend: QtWebEngine restart: true desc: >- diff --git a/qutebrowser/config/qtargs.py b/qutebrowser/config/qtargs.py index 63c1c6b0a..4bcadea84 100644 --- a/qutebrowser/config/qtargs.py +++ b/qutebrowser/config/qtargs.py @@ -8,7 +8,7 @@ import os import sys import argparse import pathlib -from typing import Any, Dict, Iterator, List, Optional, Sequence, Tuple +from typing import Any, Dict, Iterator, List, Optional, Sequence, Tuple, Union, Callable from qutebrowser.qt import machinery from qutebrowser.qt.core import QLocale @@ -273,10 +273,21 @@ def _qtwebengine_args( if disabled_features: yield _DISABLE_FEATURES + ','.join(disabled_features) - yield from _qtwebengine_settings_args() - - -_WEBENGINE_SETTINGS: Dict[str, Dict[Any, Optional[str]]] = { + yield from _qtwebengine_settings_args(versions, namespace, special_flags) + + +_SettingValueType = Union[ + str, + Callable[ + [ + version.WebEngineVersions, + argparse.Namespace, + Sequence[str], + ], + str, + ], +] +_WEBENGINE_SETTINGS: Dict[str, Dict[Any, Optional[_SettingValueType]]] = { 'qt.force_software_rendering': { 'software-opengl': None, 'qt-quick': None, @@ -325,16 +336,36 @@ _WEBENGINE_SETTINGS: Dict[str, Dict[Any, Optional[str]]] = { '--enable-experimental-web-platform-features' if machinery.IS_QT5 else None, }, 'qt.workarounds.disable_accelerated_2d_canvas': { - True: '--disable-accelerated-2d-canvas', - False: None, + 'always': '--disable-accelerated-2d-canvas', + 'never': None, + 'auto': lambda versions, namespace, special_flags: 'always' + if machinery.IS_QT6 + and versions.chromium_major + and versions.chromium_major < 111 + else 'never', }, } -def _qtwebengine_settings_args() -> Iterator[str]: +def _qtwebengine_settings_args( + versions: version.WebEngineVersions, + namespace: argparse.Namespace, + special_flags: Sequence[str], +) -> Iterator[str]: for setting, args in sorted(_WEBENGINE_SETTINGS.items()): arg = args[config.instance.get(setting)] - if arg is not None: + if callable(arg): + new_value = arg(versions, namespace, special_flags) + assert ( + new_value in args + ), f"qt.settings feature detection returned an unrecognized value: {new_value} for {setting}" + result = args[new_value] + if result is not None: + assert isinstance( + result, str + ), f"qt.settings feature detection returned an invalid type: {type(result)} for {setting}" + yield result + elif arg is not None: yield arg diff --git a/tests/unit/config/test_qtargs.py b/tests/unit/config/test_qtargs.py index 1cb149430..18edb745f 100644 --- a/tests/unit/config/test_qtargs.py +++ b/tests/unit/config/test_qtargs.py @@ -77,7 +77,7 @@ class TestQtArgs: def test_qt_args(self, monkeypatch, config_stub, args, expected, parser): """Test commandline with no Qt arguments given.""" parsed = parser.parse_args(args) - assert qtargs.qt_args(parsed) == expected + assert qtargs.qt_args(parsed) >= expected def test_qt_both(self, config_stub, parser): """Test commandline with a Qt argument and flag.""" @@ -154,6 +154,36 @@ class TestWebEngineArgs: assert '--disable-in-process-stack-traces' in args assert '--enable-in-process-stack-traces' not in args + @pytest.mark.parametrize( + 'qt_version, qt6, value, has_arg', + [ + ('5.15.2', False, 'auto', False), + ('6.5.3', True, 'auto', True), + ('6.6.0', True, 'auto', False), + ('6.5.3', True, 'always', True), + ('6.5.3', True, 'never', False), + ('6.6.0', True, 'always', True), + ], + ) + def test_accelerated_2d_canvas( + self, + parser, + version_patcher, + config_stub, + monkeypatch, + qt_version, + qt6, + value, + has_arg, + ): + version_patcher(qt_version) + config_stub.val.qt.workarounds.disable_accelerated_2d_canvas = value + monkeypatch.setattr(machinery, 'IS_QT6', qt6) + + parsed = parser.parse_args([]) + args = qtargs.qt_args(parsed) + assert ('--disable-accelerated-2d-canvas' in args) == has_arg + @pytest.mark.parametrize('flags, args', [ ([], []), (['--debug-flag', 'chromium'], ['--enable-logging', '--v=1']), -- cgit v1.2.3-54-g00ecf From 9a50f2875b361bdf9cd718e550270036ff6048a7 Mon Sep 17 00:00:00 2001 From: toofar Date: Sat, 23 Sep 2023 17:24:28 +1200 Subject: update changelog --- doc/changelog.asciidoc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/changelog.asciidoc b/doc/changelog.asciidoc index c5abc1aef..461693700 100644 --- a/doc/changelog.asciidoc +++ b/doc/changelog.asciidoc @@ -47,6 +47,9 @@ Fixed https://chromereleases.googleblog.com/2023/09/stable-channel-update-for-desktop_11.html[CVE-2023-4863], a critical heap buffer overflow in WebP, for which "Google is aware that an exploit [...] exists in the wild." +- Graphical glitches in Google sheets and PDF.js via a new setting + `qt.workarounds.disable_accelerated_2d_canvas` to disable the accelerated 2D + canvas feature which defaults to enabled on affected Qt versions. (#7489) [[v3.0.0]] v3.0.0 (2023-08-18) -- cgit v1.2.3-54-g00ecf From 4927534a6898fb3d6bea7f7c2b4ee0b42650a0c1 Mon Sep 17 00:00:00 2001 From: toofar Date: Mon, 25 Sep 2023 18:59:03 +1300 Subject: Reduce args passed to `_qtwebengine_settings_args` I added all three args to this function in an effort to allow for generic feature detection, because other places further up in the code where using all these variables for that. But I didn't look at how the stuff I was passing in could be used. I can see `special_flags` has essentially already been consumed. `namespace` is used for all kinds of stuff, there could theoretically be a pretty simple mapping between the CLI arg and a setting webengine setting but the only examples of that so for are the special flags ones and debug flags, which are already generic in their own way. So if we've gone this long without it we probably don't need it. --- qutebrowser/config/qtargs.py | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/qutebrowser/config/qtargs.py b/qutebrowser/config/qtargs.py index 4bcadea84..934953d0a 100644 --- a/qutebrowser/config/qtargs.py +++ b/qutebrowser/config/qtargs.py @@ -273,7 +273,7 @@ def _qtwebengine_args( if disabled_features: yield _DISABLE_FEATURES + ','.join(disabled_features) - yield from _qtwebengine_settings_args(versions, namespace, special_flags) + yield from _qtwebengine_settings_args(versions) _SettingValueType = Union[ @@ -281,8 +281,6 @@ _SettingValueType = Union[ Callable[ [ version.WebEngineVersions, - argparse.Namespace, - Sequence[str], ], str, ], @@ -338,7 +336,7 @@ _WEBENGINE_SETTINGS: Dict[str, Dict[Any, Optional[_SettingValueType]]] = { 'qt.workarounds.disable_accelerated_2d_canvas': { 'always': '--disable-accelerated-2d-canvas', 'never': None, - 'auto': lambda versions, namespace, special_flags: 'always' + 'auto': lambda versions: 'always' if machinery.IS_QT6 and versions.chromium_major and versions.chromium_major < 111 @@ -347,15 +345,11 @@ _WEBENGINE_SETTINGS: Dict[str, Dict[Any, Optional[_SettingValueType]]] = { } -def _qtwebengine_settings_args( - versions: version.WebEngineVersions, - namespace: argparse.Namespace, - special_flags: Sequence[str], -) -> Iterator[str]: +def _qtwebengine_settings_args(versions: version.WebEngineVersions) -> Iterator[str]: for setting, args in sorted(_WEBENGINE_SETTINGS.items()): arg = args[config.instance.get(setting)] if callable(arg): - new_value = arg(versions, namespace, special_flags) + new_value = arg(versions) assert ( new_value in args ), f"qt.settings feature detection returned an unrecognized value: {new_value} for {setting}" -- cgit v1.2.3-54-g00ecf From fde4dd84345dd5b42c8aa178a56da952a6ba0cfd Mon Sep 17 00:00:00 2001 From: toofar Date: Mon, 25 Sep 2023 19:06:03 +1300 Subject: Put qt.arg test back to being exact match I previously changed the assertion to be a subset match, to deal with a qt arg being added based on the Qt version the tests were being run on. I didn't notice this fixture that can set setting to avoid that dynamic-ness instead. --- tests/unit/config/test_qtargs.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/config/test_qtargs.py b/tests/unit/config/test_qtargs.py index 18edb745f..419faad12 100644 --- a/tests/unit/config/test_qtargs.py +++ b/tests/unit/config/test_qtargs.py @@ -51,6 +51,7 @@ def reduce_args(config_stub, version_patcher, monkeypatch): config_stub.val.content.headers.referer = 'always' config_stub.val.scrolling.bar = 'never' config_stub.val.qt.chromium.experimental_web_platform_features = 'never' + config_stub.val.qt.workarounds.disable_accelerated_2d_canvas = 'never' monkeypatch.setattr(qtargs.utils, 'is_mac', False) # Avoid WebRTC pipewire feature monkeypatch.setattr(qtargs.utils, 'is_linux', False) @@ -77,7 +78,7 @@ class TestQtArgs: def test_qt_args(self, monkeypatch, config_stub, args, expected, parser): """Test commandline with no Qt arguments given.""" parsed = parser.parse_args(args) - assert qtargs.qt_args(parsed) >= expected + assert qtargs.qt_args(parsed) == expected def test_qt_both(self, config_stub, parser): """Test commandline with a Qt argument and flag.""" -- cgit v1.2.3-54-g00ecf