Bug#1031953: marked as done (libkf5screen8: Second monitor issues with NVIDIA)

To: Steven Robbins <steve@sumost.ca>
Subject: Bug#1031953: marked as done (libkf5screen8: Second monitor issues with NVIDIA)
From: "Debian Bug Tracking System" <owner@bugs.debian.org>
Date: Wed, 01 Mar 2023 23:57:04 +0000
Message-id: <[🔎] handler.1031953.D1031953.1677714759111949.ackdone@bugs.debian.org>
Reply-to: 1031953@bugs.debian.org
References: <12169228.O9o76ZdvQC@riemann> <167736118136.10924.2081807220900663033.reportbug@riemann.sumost.ca>

Your message dated Wed, 01 Mar 2023 17:45:18 -0600
with message-id <12169228.O9o76ZdvQC@riemann>
and subject line Re: Bug#1031953: libkf5screen8: Second monitor issues with NVIDIA
has caused the Debian Bug report #1031953,
regarding libkf5screen8: Second monitor issues with NVIDIA
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
1031953: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1031953
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems

--- Begin Message ---

To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: libkf5screen8: Second monitor issues with NVIDIA
From: "Steve M. Robbins" <smr@debian.org>
Date: Sat, 25 Feb 2023 15:39:41 -0600
Message-id: <167736118136.10924.2081807220900663033.reportbug@riemann.sumost.ca>

Package: libkf5screen8
Version: 4:5.27.0-1
Severity: normal
Tags: patch upstream

This is to note that the upstream bug titled "On X11 with proprietary NVIDIA
GPU drivers, external monitor disabled after reboot or wake-from-sleep" applies
to Debian package 4.27.0-1.

See https://bugs.kde.org/show_bug.cgi?id=460341

Today a fix has finally been found.  There are at least a few reports that it
solves the issue -- I've tested it myself and it does solve the symptom I've
been having since October 2022.

The patch is:
https://invent.kde.org/plasma/libkscreen/-/commit/1d237c29655c7e3fb15fb9b71e5f167bd207593f

I'd love to see it applied before the release.  Happy to do an upload myself if
that's preferred.

-Steve


-- System Information:
Debian Release: bookworm/sid
  APT prefers unstable
  APT policy: (700, 'unstable'), (500, 'testing')
Architecture: amd64 (x86_64)
Foreign Architectures: i386

Kernel: Linux 6.1.0-5-amd64 (SMP w/16 CPU threads; PREEMPT)
Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE
Locale: LANG=en_CA.UTF-8, LC_CTYPE=en_CA.UTF-8 (charmap=UTF-8), LANGUAGE=en_CA:en
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages libkf5screen8 depends on:
ii  libc6              2.36-8
ii  libkf5screen-data  4:5.27.0-1.1
ii  libqt5core5a       5.15.8+dfsg-2
ii  libqt5dbus5        5.15.8+dfsg-2
ii  libqt5gui5         5.15.8+dfsg-2
ii  libqt5x11extras5   5.15.8-2
ii  libstdc++6         12.2.0-14

Versions of packages libkf5screen8 recommends:
ii  hwdata  0.367-1

libkf5screen8 suggests no packages.

-- no debconf information

diff --git a/backends/xrandr/xrandrconfig.cpp b/backends/xrandr/xrandrconfig.cpp
index 4ab5b3c4e1f49752c80eb6fca4f39691954024f9..50910d44f766b755208c28bfdccee4821e9ed7e3 100644
--- a/backends/xrandr/xrandrconfig.cpp
+++ b/backends/xrandr/xrandrconfig.cpp
@@ -128,11 +128,16 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
     const QSize newScreenSize = screenSize(config);
     const QSize currentScreenSize = m_screen->currentSize();
 
-    // Previously we initially set such screen size, that it can take the current
-    // as well  as the new configuration, then we apply the output changes,
-    // and finally then (if necessary) we reduce the screen size to
-    // fix the new configuration precisely.Now we initially disable the output,
-    // then set the target screen size, and finally we apply the output changes.
+    // When the current screen configuration is bigger than the new size (like
+    // when rotating an output), the XSetScreenSize can fail or apply the smaller
+    // size only partially, because we apply the size (we have to) before the
+    // output changes. To prevent all kinds of weird screen sizes from happening,
+    // we initially set such screen size, that it can take the current as well
+    // as the new configuration, then we apply the output changes, and finally then
+    // (if necessary) we reduce the screen size to fix the new configuration precisely.
+    const QSize intermediateScreenSize =
+        QSize(qMax(newScreenSize.width(), currentScreenSize.width()), qMax(newScreenSize.height(), currentScreenSize.height()));
+
     int neededCrtcs = 0;
 
     // pairs of before/after
@@ -246,6 +251,7 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
     qCDebug(KSCREEN_XRANDR) << "\tChange Screen Size:" << (newScreenSize != currentScreenSize);
     if (newScreenSize != currentScreenSize) {
         qCDebug(KSCREEN_XRANDR) << "\t\tOld:" << currentScreenSize << "\n"
+                                << "\t\tIntermediate:" << intermediateScreenSize << "\n"
                                 << "\t\tNew:" << newScreenSize;
     }
 
@@ -280,14 +286,24 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
         disableOutput(output);
     }
 
-    if (currentScreenSize != newScreenSize) {
-        for (const KScreen::OutputPtr &output : toChange) {
-            disableOutput(output);
-        }
+    if (intermediateScreenSize != currentScreenSize) {
+        setScreenSize(intermediateScreenSize);
     }
 
     bool forceScreenSizeUpdate = false;
 
+    for (const KScreen::OutputPtr &output : toChange) {
+        if (!changeOutput(output)) {
+            /* If we disabled the output before changing it and XRandR failed
+             * to re-enable it, then update screen size too */
+            if (toDisable.contains(output->id())) {
+                output->setEnabled(false);
+                qCDebug(KSCREEN_XRANDR) << "Output failed to change: " << output->name();
+                forceScreenSizeUpdate = true;
+            }
+        }
+    }
+
     for (const KScreen::OutputPtr &output : toEnable) {
         if (!enableOutput(output)) {
             qCDebug(KSCREEN_XRANDR) << "Output failed to be Enabled: " << output->name();
@@ -302,7 +318,7 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
         setOutputPriority(outputId, priority);
     }
 
-    if (forceScreenSizeUpdate || currentScreenSize != newScreenSize) {
+    if (forceScreenSizeUpdate || intermediateScreenSize != newScreenSize) {
         QSize newSize = newScreenSize;
         if (forceScreenSizeUpdate) {
             newSize = screenSize(config);
@@ -310,17 +326,6 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
         }
         setScreenSize(newSize);
     }
-
-    for (const KScreen::OutputPtr &output : toChange) {
-        if (!changeOutput(output)) {
-            /* If we disabled the output before changing it and XRandR failed
-             * to re-enable it, then update screen size too */
-            if (toDisable.contains(output->id())) {
-                output->setEnabled(false);
-                qCDebug(KSCREEN_XRANDR) << "Output failed to change: " << output->name();
-            }
-        }
-    }
 }
 
 void XRandRConfig::printConfig(const ConfigPtr &config) const

diff --git a/backends/xrandr/xrandrconfig.cpp b/backends/xrandr/xrandrconfig.cpp
index 4ab5b3c4e1f49752c80eb6fca4f39691954024f9..50910d44f766b755208c28bfdccee4821e9ed7e3 100644
--- a/backends/xrandr/xrandrconfig.cpp
+++ b/backends/xrandr/xrandrconfig.cpp
@@ -128,11 +128,16 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
     const QSize newScreenSize = screenSize(config);
     const QSize currentScreenSize = m_screen->currentSize();
 
-    // Previously we initially set such screen size, that it can take the current
-    // as well  as the new configuration, then we apply the output changes,
-    // and finally then (if necessary) we reduce the screen size to
-    // fix the new configuration precisely.Now we initially disable the output,
-    // then set the target screen size, and finally we apply the output changes.
+    // When the current screen configuration is bigger than the new size (like
+    // when rotating an output), the XSetScreenSize can fail or apply the smaller
+    // size only partially, because we apply the size (we have to) before the
+    // output changes. To prevent all kinds of weird screen sizes from happening,
+    // we initially set such screen size, that it can take the current as well
+    // as the new configuration, then we apply the output changes, and finally then
+    // (if necessary) we reduce the screen size to fix the new configuration precisely.
+    const QSize intermediateScreenSize =
+        QSize(qMax(newScreenSize.width(), currentScreenSize.width()), qMax(newScreenSize.height(), currentScreenSize.height()));
+
     int neededCrtcs = 0;
 
     // pairs of before/after
@@ -246,6 +251,7 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
     qCDebug(KSCREEN_XRANDR) << "\tChange Screen Size:" << (newScreenSize != currentScreenSize);
     if (newScreenSize != currentScreenSize) {
         qCDebug(KSCREEN_XRANDR) << "\t\tOld:" << currentScreenSize << "\n"
+                                << "\t\tIntermediate:" << intermediateScreenSize << "\n"
                                 << "\t\tNew:" << newScreenSize;
     }
 
@@ -280,14 +286,24 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
         disableOutput(output);
     }
 
-    if (currentScreenSize != newScreenSize) {
-        for (const KScreen::OutputPtr &output : toChange) {
-            disableOutput(output);
-        }
+    if (intermediateScreenSize != currentScreenSize) {
+        setScreenSize(intermediateScreenSize);
     }
 
     bool forceScreenSizeUpdate = false;
 
+    for (const KScreen::OutputPtr &output : toChange) {
+        if (!changeOutput(output)) {
+            /* If we disabled the output before changing it and XRandR failed
+             * to re-enable it, then update screen size too */
+            if (toDisable.contains(output->id())) {
+                output->setEnabled(false);
+                qCDebug(KSCREEN_XRANDR) << "Output failed to change: " << output->name();
+                forceScreenSizeUpdate = true;
+            }
+        }
+    }
+
     for (const KScreen::OutputPtr &output : toEnable) {
         if (!enableOutput(output)) {
             qCDebug(KSCREEN_XRANDR) << "Output failed to be Enabled: " << output->name();
@@ -302,7 +318,7 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
         setOutputPriority(outputId, priority);
     }
 
-    if (forceScreenSizeUpdate || currentScreenSize != newScreenSize) {
+    if (forceScreenSizeUpdate || intermediateScreenSize != newScreenSize) {
         QSize newSize = newScreenSize;
         if (forceScreenSizeUpdate) {
             newSize = screenSize(config);
@@ -310,17 +326,6 @@ void XRandRConfig::applyKScreenConfig(const KScreen::ConfigPtr &config)
         }
         setScreenSize(newSize);
     }
-
-    for (const KScreen::OutputPtr &output : toChange) {
-        if (!changeOutput(output)) {
-            /* If we disabled the output before changing it and XRandR failed
-             * to re-enable it, then update screen size too */
-            if (toDisable.contains(output->id())) {
-                output->setEnabled(false);
-                qCDebug(KSCREEN_XRANDR) << "Output failed to change: " << output->name();
-            }
-        }
-    }
 }
 
 void XRandRConfig::printConfig(const ConfigPtr &config) const

--- End Message ---

--- Begin Message ---

To: Aurélien COUDERC <libre@coucouf.fr>, 1031953-done@bugs.debian.org

Subject: Re: Bug#1031953: libkf5screen8: Second monitor issues with NVIDIA

From: Steven Robbins <steve@sumost.ca>

Date: Wed, 01 Mar 2023 17:45:18 -0600

Message-id: <12169228.O9o76ZdvQC@riemann>

In-reply-to: <2671359.mvXUDI8C0e@riemann>

References: <167736118136.10924.2081807220900663033.reportbug@riemann.sumost.ca> <5976371.lOV4Wx5bFT@grummly> <2671359.mvXUDI8C0e@riemann>
On Saturday, February 25, 2023 4:08:54 P.M. CST Steven Robbins wrote:
> On Saturday, February 25, 2023 3:57:45 P.M. CST you wrote:
> > Please don’t. I plan to upload the whole of Plasma 5.27.2 targeting
> > bookworm.
> 
> Awesome, thanks!
> 
> > But please do follow-up on this and if the patch doesn’t make into the
> > 5.27.2 upstream release you’re welcome to report it here so we can apply
> > the fix on top of that release.
> 
> Yep, I'll re-test and update/close the bug once 5.27.2 hits me.

It has, I did, and the bug is fixed.  Thus closing.
-Steve
Attachment: signature.asc
Description: This is a digitally signed message part.

--- End Message ---

Reply to:

Prev by Date: skanpage_22.12.3-1_source.changes ACCEPTED into unstable
Next by Date: Processing of meta-kde_142_source.changes
Previous by thread: skanpage_22.12.3-1_source.changes ACCEPTED into unstable
Next by thread: Processing of meta-kde_142_source.changes
Index(es):
- Date
- Thread