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

Flamegraph is not repainted correctly when zooming out #624

Closed
Waqar144 opened this issue Apr 26, 2024 · 15 comments · Fixed by #683 · May be fixed by #674
Closed

Flamegraph is not repainted correctly when zooming out #624

Waqar144 opened this issue Apr 26, 2024 · 15 comments · Fixed by #683 · May be fixed by #674
Labels

Comments

@Waqar144
Copy link

Describe the bug

See the attached video. It is easier to see the problem than describe it:

2024-04-25.15-23-53.mp4

To Reproduce
Steps to reproduce the behavior:

  1. Open perf.data.perfparser.tar.gz
  2. Zoom into KateApp::init() as much as possible
  3. Zoom out using the "Go back in symbol view history (Alt + Left)" button in the top left corner of the app one step at a time
  4. See dirty regions after 2nd or 3rd step

Expected behavior
Regions are cleared properly

Version Info (please complete the following information):

  • Linux Kernel version: 6.6.19-1
  • perf version: 6.7-1
  • hotspot version (appimage? selfcompiled?): master (self compiled)
  • if self-compiled hotspot, what version of elfutils: 0.191-1
@Waqar144 Waqar144 added the bug label Apr 26, 2024
@lievenhey
Copy link
Contributor

What qt version are you using? I tried with 6.7 and 5.15 but I couldn't reproduce the issue.

@Waqar144
Copy link
Author

Waqar144 commented May 2, 2024

5.15.12

lievenhey added a commit that referenced this issue May 6, 2024
In QT 5.15.12 the background of the flamegraph is not cleared correctly
which creates phantom entries. This problem does not exists in 5.15.10
and after 5.15.13.

fix: #624
@lievenhey
Copy link
Contributor

thanks, this seems to be a problem only in 5.15.12

@Waqar144
Copy link
Author

Seems like this issue is back in 5.15.14

@milianw milianw reopened this Jul 18, 2024
@milianw
Copy link
Member

milianw commented Jul 18, 2024

@lievenhey can you investigate please?

@lievenhey
Copy link
Contributor

I just tried an appimage build with qt 5.15.14 and I don't see this issue.

@jeff-dagenais
Copy link

jeff-dagenais commented Nov 15, 2024

image

This is built and running in a Docker container FROM ubuntu:oracular@sha256:eea047b4b181f2d3aeafbc0ce5294a2bbb3b98153a68b9ed4bc573d871ca9450

@jeff-dagenais
Copy link

jeff-dagenais commented Nov 15, 2024

This replicates 100% for me. Built hotspot eb3b7b2, with KDDockWidget 82e397cb8d71875c01432ff253becece5f0c8955. I'm running this in docker, drawing on both a linux X and Xquartz on macOS.

Happens when one hits back or forward, either with the on screen button, or with keyboard shortcuts.

@milianw do you have any hints about a hack I could do to mitigate this even if dirty, in the short term? Or pointers about where the problem might be. I can do some testing since I replicate effortlessly.

@milianw
Copy link
Member

milianw commented Nov 15, 2024

no, you would have to debug the flamegraph.cpp code to figure out what is going on there - are we sure that the scene is properly cleaned or do we have (somehow?!) old item trees lurking around? is it maybe something with the drawing code in the scene missing an explicit clear in whatever platform you are using? try to debug FlameGraph::setData(FrameGraphicsItem* rootItem) e.g. to verify it clears correctly.

or check and see if you can improve it by messing with this hack:

#if QT_VERSION <= QT_VERSION_CHECK(5, 15, 12) && QT_VERSION > QT_VERSION_CHECK(5, 15, 10)
    // the scene background doesn't get cleared correctly when using qt 5.15.12
    // this doesn't happen in 5.15.10 and 5.15.13
    m_scene->setBackgroundBrush(QBrush());
#endif

or try something like this:

diff --git a/src/flamegraph.cpp b/src/flamegraph.cpp
index c3e5f4b..80670d4 100644
--- a/src/flamegraph.cpp
+++ b/src/flamegraph.cpp
@@ -674,6 +674,11 @@ FlameGraph::FlameGraph(QWidget* parent, Qt::WindowFlags flags)
     connect(Settings::instance(), &Settings::collapseDepthChanged, this, updateHelper);
 
     m_scene->setItemIndexMethod(QGraphicsScene::NoIndex);
+#if QT_VERSION < QT_VERSION_CHECK(6, 0, 0)
+    // the scene background doesn't get cleared correctly when using qt 5.15
+    m_scene->setBackgroundBrush(palette().brush(QPalette::Active, QPalette::Base));
+#endif
+
     m_view->setScene(m_scene);
     m_view->viewport()->installEventFilter(this);
     m_view->viewport()->setMouseTracking(true);
@@ -1189,12 +1194,6 @@ void FlameGraph::selectItem(FrameGraphicsItem* item)
     // then layout all items below the selected on
     layoutItems(item);
 
-#if QT_VERSION <= QT_VERSION_CHECK(5, 15, 12) && QT_VERSION > QT_VERSION_CHECK(5, 15, 10)
-    // the scene background doesn't get cleared correctly when using qt 5.15.12
-    // this doesn't happen in 5.15.10 and 5.15.13
-    m_scene->setBackgroundBrush(QBrush());
-#endif
-
     // Triggers a refresh of the scene's bounding rect without going via the
     // event loop. This makes the centerOn call below work as expected in all cases.
     m_scene->sceneRect();

but generally, without being able to reproduce I don't know how to help you. maybe it's just time to update the appimages to use qt6 and then this will fix it too 🤷

@lievenhey
Copy link
Contributor

lievenhey commented Nov 15, 2024

I just used ubuntu:oracular@sha256:eea047b4b181f2d3aeafbc0ce5294a2bbb3b98153a68b9ed4bc573d871ca9450 to build hotspot with qt5 and ran it inside the container using wayland.
I can't reproduce the issue here.

I should add that it uses software rendering since wayland-egl can't access /dev/dri/renderD128

@lievenhey
Copy link
Contributor

okay, finally managed to reproduce it

lievenhey added a commit that referenced this issue Nov 15, 2024
Apparently when running X11 the flamgraph doesn't clear the background
correctly. This patch resets the background brush which clears the
background correctly.

fixes: #624
lievenhey added a commit that referenced this issue Nov 15, 2024
Apparently when running X11 the flamgraph doesn't clear the background
correctly. This patch resets the background brush which clears the
background correctly.

fixes: #624
@lievenhey
Copy link
Contributor

@jeff-dagenais can you try https://github.com/KDAB/hotspot/actions/runs/11857808074/artifacts/2193338354 ? That at least fixed it on my docker setup.

@milianw
Copy link
Member

milianw commented Nov 15, 2024

@lievenhey can you try with my patch above and see if that fixes the issue in your setup too, now that you can reproduce the issue?

@jeff-dagenais
Copy link

@lievenhey @milianw I applied 1264007#diff-567435b9d6ea19886df5ebde56b6f5299ce0b560b56a24b204e855d291390ffbR1194 to my own source and docker build and it totally fixes it. I don't know what's happening exactly, but if this isn't too costly, it's totally sustainable for my use case.

To give more clues, I do know that a slight resize of the window when it's got garbage at the top will fully clear it the bad background.

Also, when scrolling the flamegraph view up and down, the stale background pixels do move along with the good flamegraph pixels. The bad background clears and becomes normal only where scrolling made the top part disappear under the top border and brought back into view.
image

(nit: See the "Flame &Graph" there...)

@lievenhey
Copy link
Contributor

Your patch also seems to work. I guess this is some undefined behaviour on qt site. Since it doesn't impact performance I am leaning towards removing the #ifdef

@lievenhey lievenhey linked a pull request Nov 18, 2024 that will close this issue
lievenhey added a commit that referenced this issue Nov 18, 2024
Apparently when running X11 the flamgraph doesn't clear the background
correctly. This patch resets the background brush which clears the
background correctly.

fixes: #624
lievenhey added a commit that referenced this issue Nov 18, 2024
Apparently when running X11 the flamgraph doesn't clear the background
correctly. This patch resets the background brush which clears the
background correctly.

fixes: #624
milianw pushed a commit that referenced this issue Nov 18, 2024
Apparently when running X11 the flamgraph doesn't clear the background
correctly. This patch resets the background brush which clears the
background correctly.

fixes: #624
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants