Linux-Trace-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] Fixes needed befor KS 1.0
@ 2019-07-15 13:20 Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 1/3] kernel-shark: The graph widget must follow the active marker Yordan Karadzhov (VMware)
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-07-15 13:20 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/3] kernel-shark: The graph widget must follow the active marker
  2019-07-15 13:20 [PATCH 0/3] Fixes needed befor KS 1.0 Yordan Karadzhov (VMware)
@ 2019-07-15 13:20 ` Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 2/3] kernel-shark: Always clear the marker after resizing the table Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded Yordan Karadzhov (VMware)
  2 siblings, 0 replies; 7+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-07-15 13:20 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, Yordan Karadzhov (VMware), Valentin Schneider

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] kernel-shark: Always clear the marker after resizing the table.
  2019-07-15 13:20 [PATCH 0/3] Fixes needed befor KS 1.0 Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 1/3] kernel-shark: The graph widget must follow the active marker Yordan Karadzhov (VMware)
@ 2019-07-15 13:20 ` Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded Yordan Karadzhov (VMware)
  2 siblings, 0 replies; 7+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-07-15 13:20 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded
  2019-07-15 13:20 [PATCH 0/3] Fixes needed befor KS 1.0 Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 1/3] kernel-shark: The graph widget must follow the active marker Yordan Karadzhov (VMware)
  2019-07-15 13:20 ` [PATCH 2/3] kernel-shark: Always clear the marker after resizing the table Yordan Karadzhov (VMware)
@ 2019-07-15 13:20 ` Yordan Karadzhov (VMware)
  2019-07-17 19:30   ` Steven Rostedt
  2 siblings, 1 reply; 7+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-07-15 13:20 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded
  2019-07-15 13:20 ` [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded Yordan Karadzhov (VMware)
@ 2019-07-17 19:30   ` Steven Rostedt
  2019-07-18 13:51     ` Yordan Karadzhov (VMware)
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2019-07-17 19:30 UTC (permalink / raw)
  To: Yordan Karadzhov (VMware); +Cc: linux-trace-devel

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>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded
  2019-07-17 19:30   ` Steven Rostedt
@ 2019-07-18 13:51     ` Yordan Karadzhov (VMware)
  0 siblings, 0 replies; 7+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-07-18 13:51 UTC (permalink / raw)
  To: Steven Rostedt; +Cc: linux-trace-devel



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>
>>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 0/3] Fixes needed befor KS 1.0
@ 2019-07-23 12:52 Yordan Karadzhov (VMware)
  0 siblings, 0 replies; 7+ messages in thread
From: Yordan Karadzhov (VMware) @ 2019-07-23 12:52 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-devel, howaboutsynergy, 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-15 13:20 [PATCH 0/3] Fixes needed befor KS 1.0 Yordan Karadzhov (VMware)
2019-07-15 13:20 ` [PATCH 1/3] kernel-shark: The graph widget must follow the active marker Yordan Karadzhov (VMware)
2019-07-15 13:20 ` [PATCH 2/3] kernel-shark: Always clear the marker after resizing the table Yordan Karadzhov (VMware)
2019-07-15 13:20 ` [PATCH 3/3] kernel-shark: Don't try to update the markers if no data is loaded Yordan Karadzhov (VMware)
2019-07-17 19:30   ` Steven Rostedt
2019-07-18 13:51     ` Yordan Karadzhov (VMware)
2019-07-23 12:52 [PATCH 0/3] Fixes needed befor KS 1.0 Yordan Karadzhov (VMware)

Linux-Trace-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-trace-devel/0 linux-trace-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-trace-devel linux-trace-devel/ https://lore.kernel.org/linux-trace-devel \
		linux-trace-devel@vger.kernel.org linux-trace-devel@archiver.kernel.org
	public-inbox-index linux-trace-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-trace-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox