[PATCH 0/5] The right click of the button deselects the marker

[PATCH 0/5] The right click of the button deselects the marker

[Yordan Karadzhov]

This series of patches implements the feature requested in
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327


Yordan Karadzhov (5):
  kernel-shark: Customize the marker switching button
  kernel-shark: Define a method in View for selecting the Passive marker
  kernel-shark: Define KS_NO_ROW_SELECTED as a negative row index (-1)
  kernel-shark-qt: Do marker-deselect by right mouse click of the button
  kernel-shark: Rename the method for deselecting the Active marker.

 kernel-shark/src/KsDualMarker.cpp  | 21 ++++++++++++++
 kernel-shark/src/KsDualMarker.hpp  | 46 ++++++++++++++++++++++++++++--
 kernel-shark/src/KsMainWindow.cpp  | 38 +++++++++++++++++++++---
 kernel-shark/src/KsMainWindow.hpp  |  6 +++-
 kernel-shark/src/KsModels.cpp      |  8 +++---
 kernel-shark/src/KsModels.hpp      |  3 ++
 kernel-shark/src/KsTraceViewer.cpp | 22 ++++++++++++--
 kernel-shark/src/KsTraceViewer.hpp |  2 ++
 8 files changed, 132 insertions(+), 14 deletions(-)

-- 
2.17.1

[PATCH 1/5] kernel-shark: Customize the marker switching button

[Yordan Karadzhov]

The handler for "mouse press events" in the customized button is
reimplemented in order to provide the emission of a "deselect"
signal in the case of right mouse click. Two additional signals are
defined in KsDualMarkerSM. Those signals are used to re-emit the
"deselect" signals of the two marker buttons. Later (in another patch)
the signals will be connected to methods where the actual deselection
of the markers will take place.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsDualMarker.cpp | 21 ++++++++++++++
 kernel-shark/src/KsDualMarker.hpp | 46 +++++++++++++++++++++++++++++--
 2 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/kernel-shark/src/KsDualMarker.cpp b/kernel-shark/src/KsDualMarker.cpp
index dbeb923..43bc177 100644
--- a/kernel-shark/src/KsDualMarker.cpp
+++ b/kernel-shark/src/KsDualMarker.cpp
@@ -12,6 +12,21 @@
 #include "KsDualMarker.hpp"
 #include "KsGLWidget.hpp"
 
+/**
+ * Reimplemented handler for mouse press events. Right mouse click events will
+ * deselect the corresponding marker.
+ */
+void KsMarkerButton::mousePressEvent(QMouseEvent *e)
+{
+	if(e->button() == Qt::RightButton) {
+		emit deselect();
+
+		return;
+	}
+
+	QPushButton::mousePressEvent(e);
+}
+
 /**
  * @brief Create KsGraphMark object.
  *
@@ -178,6 +193,9 @@ KsDualMarkerSM::KsDualMarkerSM(QWidget *parent)
 	connect(&_buttonB,	&QPushButton::clicked,
 		this,		&KsDualMarkerSM::_doStateB);
 
+	connect(&_buttonB,	&KsMarkerButton::deselect,
+		this,		&KsDualMarkerSM::deselectB);
+
 	/* Define transitions from State B to State A. */
 	_stateB->addTransition(this,	&KsDualMarkerSM::machineToA, _stateA);
 
@@ -192,6 +210,9 @@ KsDualMarkerSM::KsDualMarkerSM(QWidget *parent)
 	connect(&_buttonA,	&QPushButton::clicked,
 		this,		&KsDualMarkerSM::_doStateA);
 
+	connect(&_buttonA,	&KsMarkerButton::deselect,
+		this,		&KsDualMarkerSM::deselectA);
+
 	_machine.addState(_stateA);
 	_machine.addState(_stateB);
 	_machine.setInitialState(_stateA);
diff --git a/kernel-shark/src/KsDualMarker.hpp b/kernel-shark/src/KsDualMarker.hpp
index 73d4f8a..597bddb 100644
--- a/kernel-shark/src/KsDualMarker.hpp
+++ b/kernel-shark/src/KsDualMarker.hpp
@@ -19,6 +19,36 @@
 #include "KsUtils.hpp"
 #include "KsPlotTools.hpp"
 
+/**
+ * The Marker Button class provides a button that deselect the corresponding
+ * marker in the case of a Right  mouse click.
+ */
+class KsMarkerButton : public QPushButton
+{
+	Q_OBJECT
+public:
+	/**
+	 * @brief Create a default button.
+	 */
+	explicit KsMarkerButton(QWidget *parent = nullptr)
+	: QPushButton(parent) {}
+
+	/**
+	 * @brief Create a button with text.
+	 */
+	explicit KsMarkerButton(const QString &text, QWidget *parent = nullptr)
+	: QPushButton(text, parent) {}
+
+	void mousePressEvent(QMouseEvent *e);
+
+signals:
+	/**
+	 * This signal is emitted when the button is click by the Right mouse
+	 * button.
+	 */
+	void deselect();
+};
+
 class KsGLWidget;
 
 /** The KsGraphMark represents a marker for KernelShark GUI. */
@@ -161,10 +191,22 @@ signals:
 	 */
 	void machineToB();
 
+	/**
+	 * This signal is used to re-emitted the deselect signal of the
+	 * Marker A button.
+	 */
+	void deselectA();
+
+	/**
+	 * This signal is used to re-emitted the deselect signal of the
+	 * Marker B button.
+	 */
+	void deselectB();
+
 private:
-	QPushButton	 _buttonA;
+	KsMarkerButton	 _buttonA;
 
-	QPushButton	 _buttonB;
+	KsMarkerButton	 _buttonB;
 
 	QLabel		 _labelMA, _labelMB, _labelDelta;
 
-- 
2.17.1

[PATCH 2/5] kernel-shark: Define a method in View for selecting the Passive marker

[Yordan Karadzhov]

The same method can be used to deselect the Passive marker.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsTraceViewer.cpp | 16 ++++++++++++++++
 kernel-shark/src/KsTraceViewer.hpp |  2 ++
 2 files changed, 18 insertions(+)

diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-shark/src/KsTraceViewer.cpp
index 369c78e..61d7386 100644
--- a/kernel-shark/src/KsTraceViewer.cpp
+++ b/kernel-shark/src/KsTraceViewer.cpp
@@ -704,6 +704,22 @@ void KsTraceViewer::_searchItemsMapReduce(int column,
 		lamSearchReduce(_matchList, m.get());
 }
 
+/**
+ * @brief Color (select) the given row in the table, by using the color of the
+ * 	  Passive marker.
+ *
+ * @param row: The row index. If the Passive marker is selected and the input
+ *	        value is negative, the Passive marker will be deselected.
+ */
+void KsTraceViewer::passiveMarkerSelectRow(int row)
+{
+	DualMarkerState state = _mState->getState();
+
+	_view.setVisible(false);
+	_model.selectRow(!state, row);
+	_view.setVisible(true);
+}
+
 /**
  * Get the currently selected row. If no row is selected the function
  * returns -1.
diff --git a/kernel-shark/src/KsTraceViewer.hpp b/kernel-shark/src/KsTraceViewer.hpp
index f59f5df..cf529ba 100644
--- a/kernel-shark/src/KsTraceViewer.hpp
+++ b/kernel-shark/src/KsTraceViewer.hpp
@@ -68,6 +68,8 @@ public:
 
 	void clearSelection();
 
+	void passiveMarkerSelectRow(int row);
+
 	int selectedRow();
 
 	void update(KsDataStore *data);
-- 
2.17.1

[PATCH 3/5] kernel-shark: Define KS_NO_ROW_SELECTED as a negative row index (-1)

[Yordan Karadzhov]

Currently the model of the table treats any negative value of the Passive
Marker index as "No Row is Selected". In multiple places in the code the
value of "-1" is used in order to deselecting the marker. Defining a
dedicated constant (KS_NO_ROW_SELECTED), to be used for deselecting the
Passive Marker will make the code more readable and robust.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsModels.cpp      | 8 ++++----
 kernel-shark/src/KsModels.hpp      | 3 +++
 kernel-shark/src/KsTraceViewer.cpp | 6 +++---
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel-shark/src/KsModels.cpp b/kernel-shark/src/KsModels.cpp
index 1b68143..b89fee8 100644
--- a/kernel-shark/src/KsModels.cpp
+++ b/kernel-shark/src/KsModels.cpp
@@ -199,8 +199,8 @@ KsViewModel::KsViewModel(QObject *parent)
   _nRows(0),
   _header({"#", "CPU", "Time Stamp", "Task", "PID",
 	   "Latency", "Event", "Info"}),
-  _markA(-1),
-  _markB(-1)
+  _markA(KS_NO_ROW_SELECTED),
+  _markB(KS_NO_ROW_SELECTED)
 {}
 
 /**
@@ -311,10 +311,10 @@ void KsViewModel::selectRow(DualMarkerState state, int row)
 {
 	if (state == DualMarkerState::A) {
 		_markA = row;
-		_markB = -1;
+		_markB = KS_NO_ROW_SELECTED;
 	} else {
 		_markB = row;
-		_markA = -1;
+		_markA = KS_NO_ROW_SELECTED;
 	}
 }
 
diff --git a/kernel-shark/src/KsModels.hpp b/kernel-shark/src/KsModels.hpp
index 808c574..3faaf4a 100644
--- a/kernel-shark/src/KsModels.hpp
+++ b/kernel-shark/src/KsModels.hpp
@@ -28,6 +28,9 @@
 #include "libkshark-model.h"
 #include "KsSearchFSM.hpp"
 
+/** A negative row index, to be used for deselecting the Passive Marker. */
+#define KS_NO_ROW_SELECTED -1
+
 enum class DualMarkerState;
 
 class KsDataStore;
diff --git a/kernel-shark/src/KsTraceViewer.cpp b/kernel-shark/src/KsTraceViewer.cpp
index 61d7386..2881bd0 100644
--- a/kernel-shark/src/KsTraceViewer.cpp
+++ b/kernel-shark/src/KsTraceViewer.cpp
@@ -474,7 +474,7 @@ void KsTraceViewer::markSwitch()
 		 * The passive marker is not set.
 		 * Make sure that the model colors nothing.
 		 */
-		_model.selectRow(!state, -1);
+		_model.selectRow(!state, KS_NO_ROW_SELECTED);
 	}
 
 	/*
@@ -722,12 +722,12 @@ void KsTraceViewer::passiveMarkerSelectRow(int row)
 
 /**
  * Get the currently selected row. If no row is selected the function
- * returns -1.
+ * returns KS_NO_ROW_SELECTED (-1).
  */
 int KsTraceViewer::selectedRow()
 {
 	QItemSelectionModel *sm = _view.selectionModel();
-	int dataRow = -1;
+	int dataRow = KS_NO_ROW_SELECTED;
 
 	if (sm->hasSelection()) {
 		/* Only one row at the time can be selected. */
-- 
2.17.1

[PATCH 4/5] kernel-shark-qt: Do marker-deselect by right mouse click of the button

[Yordan Karadzhov]

Methods for deselecting Marker A and B are added to KsMainWindow. Those
methods are connected to the "deselect" signals of the two marker buttons.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 30 ++++++++++++++++++++++++++++++
 kernel-shark/src/KsMainWindow.hpp |  4 ++++
 2 files changed, 34 insertions(+)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 06770d9..443b7b0 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -119,6 +119,12 @@ KsMainWindow::KsMainWindow(QWidget *parent)
 	connect(&_deselectShortcut,	&QShortcut::activated,
 		this,			&KsMainWindow::_deselect);
 
+	connect(&_mState,	&KsDualMarkerSM::deselectA,
+		this,		&KsMainWindow::_deselectA);
+
+	connect(&_mState,	&KsDualMarkerSM::deselectB,
+		this,		&KsMainWindow::_deselectB);
+
 	_resizeEmpty();
 }
 
@@ -1137,3 +1143,27 @@ void KsMainWindow::_deselect()
 	_mState.updateLabels();
 	_graph.glPtr()->model()->update();
 }
+
+void KsMainWindow::_deselectA()
+{
+	if (_mState.getState() == DualMarkerState::A)
+		_view.clearSelection();
+	else
+		_view.passiveMarkerSelectRow(KS_NO_ROW_SELECTED);
+
+	_mState.markerA().remove();
+	_mState.updateLabels();
+	_graph.glPtr()->model()->update();
+}
+
+void KsMainWindow::_deselectB()
+{
+	if (_mState.getState() == DualMarkerState::B)
+		_view.clearSelection();
+	else
+		_view.passiveMarkerSelectRow(KS_NO_ROW_SELECTED);
+
+	_mState.markerB().remove();
+	_mState.updateLabels();
+	_graph.glPtr()->model()->update();
+}
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index 44f7dd7..abfe196 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -218,6 +218,10 @@ private:
 
 	void _deselect();
 
+	void _deselectA();
+
+	void _deselectB();
+
 	void _updateFilterMenu();
 
 	void _filterSyncCBoxUpdate(kshark_context *kshark_ctx);
-- 
2.17.1

[PATCH 5/5] kernel-shark: Rename the method for deselecting the Active marker.

[Yordan Karadzhov]

This method deselects the Active marker regardless of its state (A or B).
The method is renamed in order to avoid confusion with the methods for
deselecting markers A and B.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327
Signed-off-by: Yordan Karadzhov <ykaradzhov@vmware.com>
---
 kernel-shark/src/KsMainWindow.cpp | 8 ++++----
 kernel-shark/src/KsMainWindow.hpp | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel-shark/src/KsMainWindow.cpp b/kernel-shark/src/KsMainWindow.cpp
index 443b7b0..362b955 100644
--- a/kernel-shark/src/KsMainWindow.cpp
+++ b/kernel-shark/src/KsMainWindow.cpp
@@ -101,10 +101,10 @@ KsMainWindow::KsMainWindow(QWidget *parent)
 		&_view,		&KsTraceViewer::showRow);
 
 	connect(&_graph,	&KsTraceGraph::deselect,
-		this,		&KsMainWindow::_deselect);
+		this,		&KsMainWindow::_deselectActive);
 
 	connect(&_view,		&KsTraceViewer::deselect,
-		this,		&KsMainWindow::_deselect);
+		this,		&KsMainWindow::_deselectActive);
 
 	connect(&_data,		&KsDataStore::updateWidgets,
 		&_view,		&KsTraceViewer::update);
@@ -117,7 +117,7 @@ KsMainWindow::KsMainWindow(QWidget *parent)
 
 	_deselectShortcut.setKey(Qt::CTRL + Qt::Key_D);
 	connect(&_deselectShortcut,	&QShortcut::activated,
-		this,			&KsMainWindow::_deselect);
+		this,			&KsMainWindow::_deselectActive);
 
 	connect(&_mState,	&KsDualMarkerSM::deselectA,
 		this,		&KsMainWindow::_deselectA);
@@ -1136,7 +1136,7 @@ void KsMainWindow::_splitterMoved(int pos, int index)
 	_session.saveSplitterSize(_splitter);
 }
 
-void KsMainWindow::_deselect()
+void KsMainWindow::_deselectActive()
 {
 	_view.clearSelection();
 	_mState.activeMarker().remove();
diff --git a/kernel-shark/src/KsMainWindow.hpp b/kernel-shark/src/KsMainWindow.hpp
index abfe196..969b603 100644
--- a/kernel-shark/src/KsMainWindow.hpp
+++ b/kernel-shark/src/KsMainWindow.hpp
@@ -216,7 +216,7 @@ private:
 	void _error(const QString &text, const QString &errCode,
 		    bool resize, bool unloadPlugins);
 
-	void _deselect();
+	void _deselectActive();
 
 	void _deselectA();
 
-- 
2.17.1

Re: [PATCH 0/5] The right click of the button deselects the marker

[Steven Rostedt]

On Tue, 12 Feb 2019 19:03:57 +0200
Yordan Karadzhov <ykaradzhov@vmware.com> wrote:

> This series of patches implements the feature requested in
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=202327
> 
> 
> Yordan Karadzhov (5):
>   kernel-shark: Customize the marker switching button
>   kernel-shark: Define a method in View for selecting the Passive marker
>   kernel-shark: Define KS_NO_ROW_SELECTED as a negative row index (-1)
>   kernel-shark-qt: Do marker-deselect by right mouse click of the button
>   kernel-shark: Rename the method for deselecting the Active marker.
> 
>  kernel-shark/src/KsDualMarker.cpp  | 21 ++++++++++++++
>  kernel-shark/src/KsDualMarker.hpp  | 46 ++++++++++++++++++++++++++++--
>  kernel-shark/src/KsMainWindow.cpp  | 38 +++++++++++++++++++++---
>  kernel-shark/src/KsMainWindow.hpp  |  6 +++-
>  kernel-shark/src/KsModels.cpp      |  8 +++---
>  kernel-shark/src/KsModels.hpp      |  3 ++
>  kernel-shark/src/KsTraceViewer.cpp | 22 ++++++++++++--
>  kernel-shark/src/KsTraceViewer.hpp |  2 ++
>  8 files changed, 132 insertions(+), 14 deletions(-)
> 

Applied. Thanks a lot Yordan!

-- Steve