[PATCH 0/3] Fixes needed befor KS 1.0

[PATCH 0/3] Fixes needed befor KS 1.0

[Yordan Karadzhov (VMware)]

Yordan Karadzhov (VMware) (3):
  kernel-shark: The graph widget must follow the active marker
  kernel-shark: Always clear the marker after resizing the table.
  kernel-shark: Don't try to update the markers if no data is loaded

 kernel-shark/src/KsMainWindow.cpp  | 18 ++++++++++++------
 kernel-shark/src/KsMainWindow.hpp  |  2 ++
 kernel-shark/src/KsTraceViewer.cpp | 17 +++++++++++++----
 3 files changed, 27 insertions(+), 10 deletions(-)

-- 
2.20.1

[PATCH 1/3] kernel-shark: The graph widget must follow the active marker

[Yordan Karadzhov (VMware)]

The "Graph follows" checkbox controls if the Graph widget follows or not
the change of the Active marker made from the View widget (the text data
table).
In the same time, when the user clicks on the checkbox switching it from
Unchecked to Checked, a signal is send to the Graph widget to make sure
that it will visualize the current position of the Active marker. When
sending this signal, we currently use the iterator of the search results
list, which is wrong because of two reasons. First, the  search results
list can be empty, which will trigger a segmentation fault, as reported
by Valentin Schneider. But even more important is that nothing guarantees
that when the checkbox is clacked, the marker and the iterator both point
to the same trace entry. Note that the iteration over the search results
is only one of the possible ways to change the marker.

Reported-By: Valentin Schneider <valentin.schneider@arm.com>
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204139
Tested-by: Valentin Schneider <valentin.schneider@arm.com>
Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsTraceViewer.cpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-shark/src/KsTraceViewer.cpp
index 05977c3..89e5dba 100644
--- a/kernel-shark/src/KsTraceViewer.cpp
+++ b/kernel-shark/src/KsTraceViewer.cpp
@@ -285,10 +285,11 @@ void KsTraceViewer::_searchEditText(const QString &text)
 
 void KsTraceViewer::_graphFollowsChanged(int state)
 {
-	_graphFollows = (bool) state;
+	int row = selectedRow();
 
-	if (_graphFollows && _searchDone())
-		emit select(*_it); // Send a signal to the Graph widget.
+	_graphFollows = (bool) state;
+	if (_graphFollows && row != KS_NO_ROW_SELECTED)
+		emit select(row); // Send a signal to the Graph widget.
 }
 
 void KsTraceViewer::_search()
-- 
2.20.1

[PATCH 2/3] kernel-shark: Always clear the marker after resizing the table.

[Yordan Karadzhov (VMware)]

It looks more like a Qt bug, but sometimes the automatic resize of
the table widget done in KsTraceViewer::_resizeToContents() has the
parasitic effect to select the first row of the table (making the row
green). If this is happening clear the selection by hand.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsTraceViewer.cpp | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-shark/src/KsTraceViewer.cpp
index 89e5dba..c176cef 100644
--- a/kernel-shark/src/KsTraceViewer.cpp
+++ b/kernel-shark/src/KsTraceViewer.cpp
@@ -563,12 +563,20 @@ void KsTraceViewer::keyReleaseEvent(QKeyEvent *event)
 
 void KsTraceViewer::_resizeToContents()
 {
-	int rows, columnSize;
+	int rows, columnSize, markRow = selectedRow();
 
 	_view.setVisible(false);
 	_view.resizeColumnsToContents();
 	_view.setVisible(true);
 
+	/*
+	 * It looks like a Qt bug, but sometimes when no row is selected in
+	 * the table, the automatic resize of the widget (the lines above) has
+	 * the parasitic effect to select the first row of the table.
+	 */
+	if (markRow == KS_NO_ROW_SELECTED)
+		_view.clearSelection();
+
 	/*
 	 * Because of some unknown reason the first column doesn't get
 	 * resized properly by the code above. We will resize this
-- 
2.20.1

[PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded

[Yordan Karadzhov (VMware)]

This change aims to avoid showing the labels of the time axis (zeros)
when no data is loaded.

Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
---
 kernel-shark/src/KsMainWindow.cpp | 18 ++++++++++++------
 kernel-shark/src/KsMainWindow.hpp |  2 ++
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 8826cf5..c8d9d25 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -1210,12 +1210,20 @@ void KsMainWindow::_splitterMoved(int pos, int index)
 	_session.saveSplitterSize(_splitter);
 }
 
+void KsMainWindow::_updateMarkData()
+{
+	if (_data.size() < 1)
+		return;
+
+	_mState.updateLabels();
+	_graph.glPtr()->model()->update();
+}
+
 void KsMainWindow::_deselectActive()
 {
 	_view.clearSelection();
 	_mState.activeMarker().remove();
-	_mState.updateLabels();
-	_graph.glPtr()->model()->update();
+	_updateMarkData();
 }
 
 void KsMainWindow::_deselectA()
@@ -1226,8 +1234,7 @@ void KsMainWindow::_deselectA()
 		_view.passiveMarkerSelectRow(KS_NO_ROW_SELECTED);
 
 	_mState.markerA().remove();
-	_mState.updateLabels();
-	_graph.glPtr()->model()->update();
+	_updateMarkData();
 }
 
 void KsMainWindow::_deselectB()
@@ -1238,6 +1245,5 @@ void KsMainWindow::_deselectB()
 		_view.passiveMarkerSelectRow(KS_NO_ROW_SELECTED);
 
 	_mState.markerB().remove();
-	_mState.updateLabels();
-	_graph.glPtr()->model()->update();
+	_updateMarkData();
 }
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index 62e66a0..7e2e839 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -232,6 +232,8 @@ private:
 	void _error(const QString &text, const QString &errCode,
 		    bool resize, bool unloadPlugins);
 
+	void _updateMarkData();
+
 	void _deselectActive();
 
 	void _deselectA();
-- 
2.20.1

Re: [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded

[Steven Rostedt]

On Mon, 15 Jul 2019 16:20:42 +0300
"Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:

> This change aims to avoid showing the labels of the time axis (zeros)
> when no data is loaded.

What is this actually fixing? I don't see a difference with adding this
patch and without it.

-- Steve

> 
> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>

Re: [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded

[Yordan Karadzhov (VMware)]

On 17.07.19 г. 22:30 ч., Steven Rostedt wrote:
> On Mon, 15 Jul 2019 16:20:42 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> This change aims to avoid showing the labels of the time axis (zeros)
>> when no data is loaded.
> 
> What is this actually fixing? I don't see a difference with adding this
> patch and without it.
> 

Before applying the patch, try to open the GUI without a .dat file. Then 
try to change the marker by pressing one of the marker's buttons. This 
will try to update the labels of the time axis. But because no data is 
loaded and nothing is plotted you will see only zeros at the place where 
the time axis is supposed to be plotted.

Thanks!
Yordan


> -- Steve
> 
>>
>> Signed-off-by: Yordan Karadzhov (VMware) <y.karadz@gmail.com>
>>

[PATCH 0/3] Fixes needed befor KS 1.0

[Yordan Karadzhov (VMware)]

Adderssing the issues reported by "howaboutsynergy" here:

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204277

Yordan Karadzhov (VMware) (3):
  kernel-shark: Make KsEventsCheckBoxWidget::removeSystem more robust
  kernel-shark: Better error message for the constructor of
    KsCaptureControl
  kernel-shark: Disable Capture if trace-cmd can't function

 kernel-shark/src/KsCaptureDialog.cpp | 25 ++++++++++++++++++++++---
 kernel-shark/src/KsWidgetsLib.cpp    |  9 ++++++---
 2 files changed, 28 insertions(+), 6 deletions(-)

-- 
2.20.1