Skip to content
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

High CPU load in case of QT_SCALE_FACTOR > 1 #1926

Closed
smirnovskysaha opened this issue Aug 18, 2023 · 11 comments · Fixed by #1931
Closed

High CPU load in case of QT_SCALE_FACTOR > 1 #1926

smirnovskysaha opened this issue Aug 18, 2023 · 11 comments · Fixed by #1931

Comments

@smirnovskysaha
Copy link

smirnovskysaha commented Aug 18, 2023

Problem
lxqt-panel loads CPU heavily when QT_SCALE_FACTOR is not equal to 1.

Steps to reproduce
run "QT_SCALE_FACTOR=1.5 lxqt-panel", CPU usage by lxqt-panel becomes about 70%.

Version
current (commit 9918a0e).

Source of the problem and possible fix
I have found that the problem goes away when the TaskBar widget is removed from the panel.
Profiling with gprof shows that LXQtTaskBar::onWindowChanged(), LXQtTaskGroup::regroup(), LXQtTaskGroup::onWindowChanged() are called very often in case of QT_SCALE_FACTOR>1.
I have commented out the line 64
connect(parent, &LXQtTaskBar::refreshIconGeometry, this, &LXQtTaskGroup::refreshIconsGeometry);
in the file plugin-taskbar/lxqttaskgroup.cpp and the problem disappears.

  • Distribution: gentoo
  • Qt Version: 5.15.10
  • liblxqt Version: 1.3.0
@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

Not reproducible here, whether I used QT_SCALE_FACTOR=1.5 lxqt-panel, or I set the scaling to 1.5 in LXQt Session Settings and log out and in. In both cases, the CPU usage was normal (minimal). In the first case, it made no difference whether I launched apps from Panel, or by using shortcuts.

So, there should be another factor. I'll keep this open for a while, hoping that you might find that factor.

@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

Oops! I'd tested with my old laptop, which didn't have the latest git. With my work laptop and the latest git lxqt-panel, I confirm that the CPU usage is high (~20% here).

The cause may be in a recent commit.

@tsujan tsujan added the bug label Aug 18, 2023
@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

The cause is that the X11-based function LXQtTaskButton::refreshIconGeometry doesn't consider the scale factor. More precisely, the scale factor isn't considered in its argument geom.

This is a new behavior (as I mentioned above, it didn't happen in older versions of lxqt-panel). I guess it's caused by a change or bug in KWindowSystem. My concern is that if we "fix" it in lxqt-panel and the change/bug is reversed in KWindowSystem, we'll have the same problem later. On the other hand, it's a serious problem and should be fixed/avoided.

@smirnovskysaha
Copy link
Author

Ok, thank you for your feedback!
As a workaround, I will comment out the connection with LXQtTaskBar::refreshIconGeometry() in LXQtTaskGroup and will monitor behaviour of lxqt-panel. What possible problems might arise in this case?

@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

Sadly, I was right; it's a bug in KWindowSystem :( NETWinInfo::setIconGeometry takes into account the scale factor by using qApp->devicePixelRatio(), but NETWinInfo::iconGeometry doesn't. If we work around this issue and then it's fixed in KWindowSystem, we'll encounter it again.

@palinek, any suggestion? Can we avoid this nasty problem in a persistent way?

@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

What possible problems might arise in this case?

I'm not sure — never dealt with this function before. Perhaps misplaced icons under special circumstances.

EDIT: You could also add a return; to the beginning of the function LXQtTaskButton::refreshIconGeometry instead of commenting out the connection to the signal LXQtTaskBar::refreshIconGeometry.

@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

The KDE doc says that NETWinInfo::setIconGeometry sets the icon geometry for the application window. I don't understand why it's needed. Can't Qt do it (whatever it is), independently of X11?

I tested with 1.2.1 in my old laptop by removing LXQtTaskButton::refreshIconGeometry. With or without it, the taskbar icons weren't updated when I changed the position of Panel (from bottom to left); I had to restart it to have a usable taskbar after changing the position.

@smirnovskysaha
Do you see a problem in the taskbar if you add a return; to the beginning of the function LXQtTaskButton::refreshIconGeometry (instead of commenting out the connection to the signal LXQtTaskBar::refreshIconGeometry)?

@smirnovskysaha
Copy link
Author

smirnovskysaha commented Aug 18, 2023

@tsujan
If I add return to the beginning of the function LXQtTaskButton::refreshIconGeometry then the problem with high CPU load goes away.
Also I have no visible problems in the taskbar.

@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

@smirnovskysaha, thanks!

Also I have no visible problems in the taskbar.

I don't see any either, whether with git or with an older version. The behavior of icons are also the same when I change the panel's height/width or the icon size of the taskbar (suboptimal).

@smirnovskysaha
Copy link
Author

But I should notice that just adding return in LXQtTaskButton::refreshIconGeometry leads anyway to multiple calling of LXQtTaskButton::refreshIconGeometry (according to profiling information).

@tsujan
Copy link
Member

tsujan commented Aug 18, 2023

Yes. Here the signal LXQtTaskBar::refreshIconGeometry is emitted continuously. That may be a design flaw, but it doesn't cause an extra CPU usage when LXQtTaskButton::refreshIconGeometry is removed.

EDIT: I'm using this patch:

diff -ruNp lxqt-panel-orig/plugin-taskbar/lxqttaskbutton.cpp lxqt-panel/plugin-taskbar/lxqttaskbutton.cpp
--- lxqt-panel-orig/plugin-taskbar/lxqttaskbutton.cpp
+++ lxqt-panel/plugin-taskbar/lxqttaskbutton.cpp
@@ -159,35 +159,6 @@ void LXQtTaskButton::updateIcon()
 /************************************************
 
  ************************************************/
-void LXQtTaskButton::refreshIconGeometry(QRect const & geom)
-{
-    xcb_connection_t* x11conn = QX11Info::connection();
-
-    if (!x11conn) {
-        return;
-    }
-
-    NETWinInfo info(x11conn,
-                    windowId(),
-                    (WId) QX11Info::appRootWindow(),
-                    NET::WMIconGeometry,
-                    NET::Properties2());
-    NETRect const curr = info.iconGeometry();
-    if (curr.pos.x != geom.x() || curr.pos.y != geom.y()
-            || curr.size.width != geom.width() || curr.size.height != geom.height())
-    {
-        NETRect nrect;
-        nrect.pos.x = geom.x();
-        nrect.pos.y = geom.y();
-        nrect.size.height = geom.height();
-        nrect.size.width = geom.width();
-        info.setIconGeometry(nrect);
-    }
-}
-
-/************************************************
-
- ************************************************/
 void LXQtTaskButton::changeEvent(QEvent *event)
 {
     if (event->type() == QEvent::StyleChange)
diff -ruNp lxqt-panel-orig/plugin-taskbar/lxqttaskbutton.h lxqt-panel/plugin-taskbar/lxqttaskbutton.h
--- lxqt-panel-orig/plugin-taskbar/lxqttaskbutton.h
+++ lxqt-panel/plugin-taskbar/lxqttaskbutton.h
@@ -79,7 +79,6 @@ public:
 
     LXQtTaskBar * parentTaskBar() const {return mParentTaskBar;}
 
-    void refreshIconGeometry(QRect const & geom);
     static QString mimeDataFormat() { return QLatin1String("lxqt/lxqttaskbutton"); }
     /*! \return true if this button received DragEnter event (and no DragLeave event yet)
      * */
diff -ruNp lxqt-panel-orig/plugin-taskbar/lxqttaskgroup.cpp lxqt-panel/plugin-taskbar/lxqttaskgroup.cpp
--- lxqt-panel-orig/plugin-taskbar/lxqttaskgroup.cpp
+++ lxqt-panel/plugin-taskbar/lxqttaskgroup.cpp
@@ -439,18 +439,8 @@ void LXQtTaskGroup::setPopupVisible(bool
  ************************************************/
 void LXQtTaskGroup::refreshIconsGeometry()
 {
-    QRect rect = geometry();
-    rect.moveTo(mapToGlobal(QPoint(0, 0)));
-
-    if (mSingleButton)
-    {
-        refreshIconGeometry(rect);
-        return;
-    }
-
     for(LXQtTaskButton *but : qAsConst(mButtonHash))
     {
-        but->refreshIconGeometry(rect);
         but->setIconSize(QSize(plugin()->panel()->iconSize(), plugin()->panel()->iconSize()));
     }
 }

tsujan added a commit that referenced this issue Aug 23, 2023
… by removing `LXQtTaskButton::refreshIconGeometry`, for which I found no use. The high CPU usage was a result of a recent change — or perhaps bug — in `KWindowSystem`.

Fixes #1926
tsujan added a commit that referenced this issue Aug 27, 2023
`KWindowSystem` started to take the scale factor into account; we should also do so in task bar.

NOTE: `KWindowSystem` 5.101.0, which is our minimum required version, does it too.

Fixes #1926
tsujan added a commit that referenced this issue Aug 28, 2023
`KWindowSystem` started to take the scale factor into account; we should also do so in task bar.

NOTE: `KWindowSystem` 5.101.0, which is our minimum required version, does it too.

Fixes #1926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants