From 68589fe937967b808f786b8fe21b9cb9285b14c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Wed, 6 Oct 2021 20:19:22 +0200 Subject: [PATCH] compositor: Merge the OpenGL 2 and 3 backends This change merges the two OpenGL backends into one making the current default of GLCore the overall default. It becomes the first context to try to create. If it fails, it will automatically fall back to the (previous) OpenGL 2 backend. Reasoning: the differentiation of OpenGL 2 and 3 is a very technical one and hard to understand for users. It is not obvious which one is better or should be used. This results in many user discussions like "Which backend to use?" Back when the OpenGL 3 backend was introduced the dedicated feature made sense. It was a new code base using new driver features. Nowadays the code base in KWin is robust and mature and so are the drivers. A driver advertising support for OpenGL 3 will support OpenGL 3. We don't have to plan for driver breakage in this area any more. Also our code evolved through the context attribute builder which gives us the possibility to more easily fall back in case we cannot create the context. Thus the need to select the backend is not so important as it used to be when the feature got introduced. If a user still wants to force OpenGL2, it is still possible by setting the appropriate environment variables like MESA_GL_VERSION_OVERRIDE. This change brings the improvement that the backend selection is now completely removed from the compositing KCM. --- .../integration/generic_scene_opengl_test.cpp | 15 ---- .../integration/generic_scene_opengl_test.h | 1 - src/kcmkwin/kwincompositing/compositing.ui | 10 --- .../kwincompositing_setting.kcfg | 11 --- src/kcmkwin/kwincompositing/main.cpp | 71 +------------------ src/options.cpp | 11 --- src/options.h | 10 --- .../scenes/opengl/abstract_egl_backend.cpp | 2 +- .../platforms/x11/standalone/glxbackend.cpp | 61 ++++++++-------- 9 files changed, 33 insertions(+), 159 deletions(-) diff --git a/autotests/integration/generic_scene_opengl_test.cpp b/autotests/integration/generic_scene_opengl_test.cpp index cf80c263b5..c15136d621 100644 --- a/autotests/integration/generic_scene_opengl_test.cpp +++ b/autotests/integration/generic_scene_opengl_test.cpp @@ -70,24 +70,9 @@ void GenericSceneOpenGLTest::initTestCase() QCOMPARE(kwinApp()->platform()->selectedCompositor(), KWin::OpenGLCompositing); } -void GenericSceneOpenGLTest::testRestart_data() -{ - QTest::addColumn("core"); - - QTest::newRow("GLCore") << true; - QTest::newRow("Legacy") << false; -} - void GenericSceneOpenGLTest::testRestart() { // simple restart of the OpenGL compositor without any windows being shown - - // setup opengl compositing options - auto compositingGroup = kwinApp()->config()->group("Compositing"); - QFETCH(bool, core); - compositingGroup.writeEntry("GLCore", core); - compositingGroup.sync(); - QSignalSpy sceneCreatedSpy(KWin::Compositor::self(), &Compositor::sceneCreated); QVERIFY(sceneCreatedSpy.isValid()); KWin::Compositor::self()->reinitialize(); diff --git a/autotests/integration/generic_scene_opengl_test.h b/autotests/integration/generic_scene_opengl_test.h index 8e8876e4a4..8c1cf54205 100644 --- a/autotests/integration/generic_scene_opengl_test.h +++ b/autotests/integration/generic_scene_opengl_test.h @@ -21,7 +21,6 @@ protected: private Q_SLOTS: void initTestCase(); void cleanup(); - void testRestart_data(); void testRestart(); private: diff --git a/src/kcmkwin/kwincompositing/compositing.ui b/src/kcmkwin/kwincompositing/compositing.ui index cf70d7ba35..a970f2ccaf 100644 --- a/src/kcmkwin/kwincompositing/compositing.ui +++ b/src/kcmkwin/kwincompositing/compositing.ui @@ -174,16 +174,6 @@ you can reset this protection but be aware that this might result in an immediat - - - - Rendering backend: - - - - - - diff --git a/src/kcmkwin/kwincompositing/kwincompositing_setting.kcfg b/src/kcmkwin/kwincompositing/kwincompositing_setting.kcfg index f6a2af78f3..d4d30e86b3 100644 --- a/src/kcmkwin/kwincompositing/kwincompositing_setting.kcfg +++ b/src/kcmkwin/kwincompositing/kwincompositing_setting.kcfg @@ -48,17 +48,6 @@ - - OpenGL - - - - - - - false - - true diff --git a/src/kcmkwin/kwincompositing/main.cpp b/src/kcmkwin/kwincompositing/main.cpp index 9b915d1899..4ff7b90d51 100644 --- a/src/kcmkwin/kwincompositing/main.cpp +++ b/src/kcmkwin/kwincompositing/main.cpp @@ -35,10 +35,6 @@ class KWinCompositingKCM : public KCModule { Q_OBJECT public: - enum CompositingTypeIndex { - OPENGL31_INDEX = 0, - OPENGL20_INDEX, - }; explicit KWinCompositingKCM(QWidget *parent = nullptr, const QVariantList &args = QVariantList()); @@ -48,7 +44,6 @@ public Q_SLOTS: void defaults() override; private Q_SLOTS: - void onBackendChanged(); void reenableGl(); private: @@ -163,60 +158,28 @@ void KWinCompositingKCM::init() } ); - // compositing type - m_form.backend->addItem(i18n("OpenGL 3.1"), CompositingTypeIndex::OPENGL31_INDEX); - m_form.backend->addItem(i18n("OpenGL 2.0"), CompositingTypeIndex::OPENGL20_INDEX); - - connect(m_form.backend, currentIndexChangedSignal, this, &KWinCompositingKCM::onBackendChanged); - if (m_settings->openGLIsUnsafe()) { m_form.glCrashedWarning->animatedShow(); } } -void KWinCompositingKCM::onBackendChanged() -{ - const int currentType = m_form.backend->currentData().toInt(); - - m_form.kcfg_glTextureFilter->setVisible(currentType == CompositingTypeIndex::OPENGL31_INDEX || - currentType == CompositingTypeIndex::OPENGL20_INDEX); - - updateUnmanagedItemStatus(); -} - void KWinCompositingKCM::updateUnmanagedItemStatus() { - int backend = KWinCompositingSetting::EnumBackend::OpenGL; - bool glCore = true; - const int currentType = m_form.backend->currentData().toInt(); - switch (currentType) { - case CompositingTypeIndex::OPENGL31_INDEX: - // default already set - break; - case CompositingTypeIndex::OPENGL20_INDEX: - glCore = false; - break; - } const auto animationDuration = s_animationMultipliers[m_form.animationDurationFactor->value()]; const bool inPlasma = isRunningPlasma(); - bool changed = glCore != m_settings->glCore(); - changed |= backend != m_settings->backend(); + bool changed = false; if (!inPlasma) { changed |= (animationDuration != m_settings->animationDurationFactor()); } unmanagedWidgetChangeState(changed); - bool defaulted = glCore == m_settings->defaultGlCoreValue(); - defaulted &= backend == m_settings->defaultBackendValue(); + bool defaulted = true; if (!inPlasma) { defaulted &= animationDuration == m_settings->defaultAnimationDurationFactorValue(); } - m_form.backend->setProperty("_kde_highlight_neutral", defaultsIndicatorsVisible() && (backend != m_settings->defaultBackendValue() || glCore != m_settings->defaultGlCoreValue())); - m_form.backend->update(); - unmanagedWidgetDefaultState(defaulted); } @@ -231,20 +194,6 @@ void KWinCompositingKCM::load() const int index = static_cast(std::distance(s_animationMultipliers.begin(), it)); m_form.animationDurationFactor->setValue(index); m_form.animationDurationFactor->setDisabled(m_settings->isAnimationDurationFactorImmutable()); - - m_settings->findItem("Backend")->readConfig(m_settings->config()); - m_settings->findItem("glCore")->readConfig(m_settings->config()); - - if (m_settings->backend() == KWinCompositingSetting::EnumBackend::OpenGL) { - if (m_settings->glCore()) { - m_form.backend->setCurrentIndex(CompositingTypeIndex::OPENGL31_INDEX); - } else { - m_form.backend->setCurrentIndex(CompositingTypeIndex::OPENGL20_INDEX); - } - } - m_form.backend->setDisabled(m_settings->isBackendImmutable()); - - onBackendChanged(); } void KWinCompositingKCM::defaults() @@ -252,7 +201,6 @@ void KWinCompositingKCM::defaults() KCModule::defaults(); // unmanaged widgets - m_form.backend->setCurrentIndex(CompositingTypeIndex::OPENGL20_INDEX); if (!isRunningPlasma()) { // corresponds to 1.0 seconds in s_animationMultipliers m_form.animationDurationFactor->setValue(3); @@ -261,21 +209,6 @@ void KWinCompositingKCM::defaults() void KWinCompositingKCM::save() { - int backend = KWinCompositingSetting::EnumBackend::OpenGL; - bool glCore = true; - const int currentType = m_form.backend->currentData().toInt(); - switch (currentType) { - case CompositingTypeIndex::OPENGL31_INDEX: - // default already set - break; - case CompositingTypeIndex::OPENGL20_INDEX: - backend = KWinCompositingSetting::EnumBackend::OpenGL; - glCore = false; - break; - } - m_settings->setBackend(backend); - m_settings->setGlCore(glCore); - if (!isRunningPlasma()) { const auto animationDuration = s_animationMultipliers[m_form.animationDurationFactor->value()]; m_settings->setAnimationDurationFactor(animationDuration); diff --git a/src/options.cpp b/src/options.cpp index 42fb513f98..ec08efdf1d 100644 --- a/src/options.cpp +++ b/src/options.cpp @@ -62,7 +62,6 @@ Options::Options(QObject *parent) , m_glSmoothScale(Options::defaultGlSmoothScale()) , m_glStrictBinding(Options::defaultGlStrictBinding()) , m_glStrictBindingFollowsDriver(Options::defaultGlStrictBindingFollowsDriver()) - , m_glCoreProfile(Options::defaultGLCoreProfile()) , m_glPreferBufferSwap(Options::defaultGlPreferBufferSwap()) , m_glPlatformInterface(Options::defaultGlPlatformInterface()) , m_windowsBlockCompositing(true) @@ -590,15 +589,6 @@ void Options::setGlStrictBindingFollowsDriver(bool glStrictBindingFollowsDriver) Q_EMIT glStrictBindingFollowsDriverChanged(); } -void Options::setGLCoreProfile(bool value) -{ - if (m_glCoreProfile == value) { - return; - } - m_glCoreProfile = value; - Q_EMIT glCoreProfileChanged(); -} - void Options::setWindowsBlockCompositing(bool value) { if (m_windowsBlockCompositing == value) { @@ -883,7 +873,6 @@ void Options::reloadCompositingSettings(bool force) if (!isGlStrictBindingFollowsDriver()) { setGlStrictBinding(config.readEntry("GLStrictBinding", Options::defaultGlStrictBinding())); } - setGLCoreProfile(config.readEntry("GLCore", Options::defaultGLCoreProfile())); char c = 0; const QString s = config.readEntry("GLPreferBufferSwap", QString(Options::defaultGlPreferBufferSwap())); diff --git a/src/options.h b/src/options.h index 9a0ff0b7e2..f6df87a576 100644 --- a/src/options.h +++ b/src/options.h @@ -193,7 +193,6 @@ class KWIN_EXPORT Options : public QObject * If @c false glStrictBinding is set from a config value and not updated during scene initialization. */ Q_PROPERTY(bool glStrictBindingFollowsDriver READ isGlStrictBindingFollowsDriver WRITE setGlStrictBindingFollowsDriver NOTIFY glStrictBindingFollowsDriverChanged) - Q_PROPERTY(bool glCoreProfile READ glCoreProfile WRITE setGLCoreProfile NOTIFY glCoreProfileChanged) Q_PROPERTY(GlSwapStrategy glPreferBufferSwap READ glPreferBufferSwap WRITE setGlPreferBufferSwap NOTIFY glPreferBufferSwapChanged) Q_PROPERTY(KWin::OpenGLPlatformInterface glPlatformInterface READ glPlatformInterface WRITE setGlPlatformInterface NOTIFY glPlatformInterfaceChanged) Q_PROPERTY(bool windowsBlockCompositing READ windowsBlockCompositing WRITE setWindowsBlockCompositing NOTIFY windowsBlockCompositingChanged) @@ -583,9 +582,6 @@ public: bool isGlStrictBindingFollowsDriver() const { return m_glStrictBindingFollowsDriver; } - bool glCoreProfile() const { - return m_glCoreProfile; - } OpenGLPlatformInterface glPlatformInterface() const { return m_glPlatformInterface; } @@ -666,7 +662,6 @@ public: void setGlSmoothScale(int glSmoothScale); void setGlStrictBinding(bool glStrictBinding); void setGlStrictBindingFollowsDriver(bool glStrictBindingFollowsDriver); - void setGLCoreProfile(bool glCoreProfile); void setGlPreferBufferSwap(char glPreferBufferSwap); void setGlPlatformInterface(OpenGLPlatformInterface interface); void setWindowsBlockCompositing(bool set); @@ -753,9 +748,6 @@ public: static bool defaultGlStrictBindingFollowsDriver() { return true; } - static bool defaultGLCoreProfile() { - return false; - } static GlSwapStrategy defaultGlPreferBufferSwap() { return AutoSwapStrategy; } @@ -839,7 +831,6 @@ Q_SIGNALS: void glSmoothScaleChanged(); void glStrictBindingChanged(); void glStrictBindingFollowsDriverChanged(); - void glCoreProfileChanged(); void glPreferBufferSwapChanged(); void glPlatformInterfaceChanged(); void windowsBlockCompositingChanged(); @@ -885,7 +876,6 @@ private: // Settings that should be auto-detected bool m_glStrictBinding; bool m_glStrictBindingFollowsDriver; - bool m_glCoreProfile; GlSwapStrategy m_glPreferBufferSwap; OpenGLPlatformInterface m_glPlatformInterface; bool m_windowsBlockCompositing; diff --git a/src/platformsupport/scenes/opengl/abstract_egl_backend.cpp b/src/platformsupport/scenes/opengl/abstract_egl_backend.cpp index 7f8a1d33bc..84dd3341f6 100644 --- a/src/platformsupport/scenes/opengl/abstract_egl_backend.cpp +++ b/src/platformsupport/scenes/opengl/abstract_egl_backend.cpp @@ -294,7 +294,7 @@ bool AbstractEglBackend::createContext() gles->setVersion(2); candidates.push_back(std::move(gles)); } else { - if (options->glCoreProfile() && haveCreateContext) { + if (haveCreateContext) { if (haveRobustness && haveContextPriority) { auto robustCorePriority = std::make_unique(); robustCorePriority->setVersion(3, 1); diff --git a/src/plugins/platforms/x11/standalone/glxbackend.cpp b/src/plugins/platforms/x11/standalone/glxbackend.cpp index 881adbdbab..7c15847dd3 100644 --- a/src/plugins/platforms/x11/standalone/glxbackend.cpp +++ b/src/plugins/platforms/x11/standalone/glxbackend.cpp @@ -336,39 +336,38 @@ bool GlxBackend::initRenderingContext() const bool haveVideoMemoryPurge = hasExtension(QByteArrayLiteral("GLX_NV_robustness_video_memory_purge")); std::vector candidates; - if (options->glCoreProfile()) { - if (have_robustness) { - if (haveVideoMemoryPurge) { - GlxContextAttributeBuilder purgeMemoryCore; - purgeMemoryCore.setVersion(3, 1); - purgeMemoryCore.setRobust(true); - purgeMemoryCore.setResetOnVideoMemoryPurge(true); - candidates.emplace_back(std::move(purgeMemoryCore)); - } - GlxContextAttributeBuilder robustCore; - robustCore.setVersion(3, 1); - robustCore.setRobust(true); - candidates.emplace_back(std::move(robustCore)); + // core + if (have_robustness) { + if (haveVideoMemoryPurge) { + GlxContextAttributeBuilder purgeMemoryCore; + purgeMemoryCore.setVersion(3, 1); + purgeMemoryCore.setRobust(true); + purgeMemoryCore.setResetOnVideoMemoryPurge(true); + candidates.emplace_back(std::move(purgeMemoryCore)); } - GlxContextAttributeBuilder core; - core.setVersion(3, 1); - candidates.emplace_back(std::move(core)); - } else { - if (have_robustness) { - if (haveVideoMemoryPurge) { - GlxContextAttributeBuilder purgeMemoryLegacy; - purgeMemoryLegacy.setRobust(true); - purgeMemoryLegacy.setResetOnVideoMemoryPurge(true); - candidates.emplace_back(std::move(purgeMemoryLegacy)); - } - GlxContextAttributeBuilder robustLegacy; - robustLegacy.setRobust(true); - candidates.emplace_back(std::move(robustLegacy)); - } - GlxContextAttributeBuilder legacy; - legacy.setVersion(2, 1); - candidates.emplace_back(std::move(legacy)); + GlxContextAttributeBuilder robustCore; + robustCore.setVersion(3, 1); + robustCore.setRobust(true); + candidates.emplace_back(std::move(robustCore)); } + GlxContextAttributeBuilder core; + core.setVersion(3, 1); + candidates.emplace_back(std::move(core)); + // legacy + if (have_robustness) { + if (haveVideoMemoryPurge) { + GlxContextAttributeBuilder purgeMemoryLegacy; + purgeMemoryLegacy.setRobust(true); + purgeMemoryLegacy.setResetOnVideoMemoryPurge(true); + candidates.emplace_back(std::move(purgeMemoryLegacy)); + } + GlxContextAttributeBuilder robustLegacy; + robustLegacy.setRobust(true); + candidates.emplace_back(std::move(robustLegacy)); + } + GlxContextAttributeBuilder legacy; + legacy.setVersion(2, 1); + candidates.emplace_back(std::move(legacy)); for (auto it = candidates.begin(); it != candidates.end(); it++) { const auto attribs = it->build(); ctx = glXCreateContextAttribsARB(display(), fbconfig, globalShareContext, true, attribs.data());