From 47e9e7bbc3a4e91a960f4cbb1db48c5aff6dcace Mon Sep 17 00:00:00 2001
From: Soeren Apel <soeren@apelpie.net>
Date: Tue, 16 Aug 2016 21:31:59 +0200
Subject: [PATCH] Move signals to views and make Session handle multiple views

---
 pv/mainwindow.cpp       |  12 +++++
 pv/mainwindow.hpp       |   2 +
 pv/popups/channels.cpp  |   4 +-
 pv/session.cpp          | 114 +++++++++++++++++++++++-----------------
 pv/session.hpp          |  19 ++++---
 pv/storesession.cpp     |  19 +++----
 pv/view/decodetrace.cpp |  34 ++++++------
 pv/view/view.cpp        |  30 +++++++----
 pv/view/view.hpp        |  12 +++++
 9 files changed, 149 insertions(+), 97 deletions(-)

diff --git a/pv/mainwindow.cpp b/pv/mainwindow.cpp
index 41c748b7..8ab1a24b 100644
--- a/pv/mainwindow.cpp
+++ b/pv/mainwindow.cpp
@@ -131,6 +131,16 @@ MainWindow::MainWindow(DeviceManager &device_manager,
 		load_init_file(open_file_name, open_file_format);
 }
 
+MainWindow::~MainWindow()
+{
+	for (auto entry : view_docks_) {
+		const std::shared_ptr<QDockWidget> dock = entry.first;
+		dock->setWidget(0);
+		const std::shared_ptr<pv::view::View> view = entry.second;
+		session_.deregister_view(view);
+	}
+}
+
 QAction* MainWindow::action_open() const
 {
 	return action_open_;
@@ -258,6 +268,8 @@ shared_ptr<pv::view::View> MainWindow::add_view(const QString &title,
 			v->enable_coloured_bg(action_view_coloured_bg_->isChecked());
 			action_view_show_cursors_->setChecked(v->cursors_shown());
 		}
+
+		session.register_view(v);
 	}
 
 	return v;
diff --git a/pv/mainwindow.hpp b/pv/mainwindow.hpp
index 91a0bc5d..547089c5 100644
--- a/pv/mainwindow.hpp
+++ b/pv/mainwindow.hpp
@@ -83,6 +83,8 @@ public:
 		std::string open_file_format = std::string(),
 		QWidget *parent = 0);
 
+	~MainWindow();
+
 	QAction* action_open() const;
 	QAction* action_save_as() const;
 	QAction* action_save_selection_as() const;
diff --git a/pv/popups/channels.cpp b/pv/popups/channels.cpp
index f8f94d1a..23204e85 100644
--- a/pv/popups/channels.cpp
+++ b/pv/popups/channels.cpp
@@ -82,8 +82,8 @@ Channels::Channels(Session &session, QWidget *parent) :
 	map<shared_ptr<Channel>, shared_ptr<SignalBase> > signal_map;
 
 	unordered_set< shared_ptr<SignalBase> > sigs;
-	for (const shared_ptr<view::Signal> s : session_.signals())
-		sigs.insert(s->base());
+	for (const shared_ptr<data::SignalBase> b : session_.signalbases())
+		sigs.insert(b);
 
 	for (const shared_ptr<SignalBase> &sig : sigs)
 		signal_map[sig->channel()] = sig;
diff --git a/pv/session.cpp b/pv/session.cpp
index 99dca215..852de298 100644
--- a/pv/session.cpp
+++ b/pv/session.cpp
@@ -48,6 +48,8 @@
 #include "view/analogsignal.hpp"
 #include "view/decodetrace.hpp"
 #include "view/logicsignal.hpp"
+#include "view/signal.hpp"
+#include "view/view.hpp"
 
 #include <cassert>
 #include <mutex>
@@ -142,13 +144,12 @@ void Session::set_device(shared_ptr<devices::Device> device)
 	device_.reset();
 
 	// Remove all stored data
-	signals_.clear();
-	{
-		shared_lock<shared_mutex> lock(signals_mutex_);
-		for (const shared_ptr<data::SignalData> d : all_signal_data_)
-			d->clear();
-	}
+	for (std::shared_ptr<pv::view::View> view : views_)
+		view->clear_signals();
+	for (const shared_ptr<data::SignalData> d : all_signal_data_)
+		d->clear();
 	all_signal_data_.clear();
+	signalbases_.clear();
 	cur_logic_segment_.reset();
 
 	for (auto entry : cur_analog_segments_) {
@@ -224,11 +225,8 @@ void Session::start_capture(function<void (const QString)> error_handler)
 	}
 
 	// Clear signal data
-	{
-		shared_lock<shared_mutex> lock(signals_mutex_);
-		for (const shared_ptr<data::SignalData> d : all_signal_data_)
-			d->clear();
-	}
+	for (const shared_ptr<data::SignalData> d : all_signal_data_)
+		d->clear();
 
 	// Begin the session
 	sampling_thread_ = std::thread(
@@ -245,19 +243,26 @@ void Session::stop_capture()
 		sampling_thread_.join();
 }
 
+void Session::register_view(std::shared_ptr<pv::view::View> view)
+{
+	views_.insert(view);
+}
+
+void Session::deregister_view(std::shared_ptr<pv::view::View> view)
+{
+	views_.erase(view);
+}
+
 double Session::get_samplerate() const
 {
 	double samplerate = 0.0;
 
-	{
-		shared_lock<shared_mutex> lock(signals_mutex_);
-		for (const shared_ptr<pv::data::SignalData> d : all_signal_data_) {
-			assert(d);
-			const vector< shared_ptr<pv::data::Segment> > segments =
-				d->segments();
-			for (const shared_ptr<pv::data::Segment> &s : segments)
-				samplerate = std::max(samplerate, s->samplerate());
-		}
+	for (const shared_ptr<pv::data::SignalData> d : all_signal_data_) {
+		assert(d);
+		const vector< shared_ptr<pv::data::Segment> > segments =
+			d->segments();
+		for (const shared_ptr<pv::data::Segment> &s : segments)
+			samplerate = std::max(samplerate, s->samplerate());
 	}
 	// If there is no sample rate given we use samples as unit
 	if (samplerate == 0.0)
@@ -266,10 +271,10 @@ double Session::get_samplerate() const
 	return samplerate;
 }
 
-const unordered_set< shared_ptr<view::Signal> > Session::signals() const
+const std::unordered_set< std::shared_ptr<data::SignalBase> >
+	Session::signalbases() const
 {
-	shared_lock<shared_mutex> lock(signals_mutex_);
-	return signals_;
+	return signalbases_;
 }
 
 #ifdef ENABLE_DECODE
@@ -279,8 +284,6 @@ bool Session::add_decoder(srd_decoder *const dec)
 	shared_ptr<data::DecoderStack> decoder_stack;
 
 	try {
-		lock_guard<boost::shared_mutex> lock(signals_mutex_);
-
 		// Create the decoder
 		decoder_stack = shared_ptr<data::DecoderStack>(
 			new data::DecoderStack(*this, dec));
@@ -329,7 +332,6 @@ bool Session::add_decoder(srd_decoder *const dec)
 
 vector< shared_ptr<view::DecodeTrace> > Session::get_decode_signals() const
 {
-	shared_lock<shared_mutex> lock(signals_mutex_);
 	return decode_traces_;
 }
 
@@ -361,8 +363,10 @@ void Session::set_capture_state(capture_state state)
 void Session::update_signals()
 {
 	if (!device_) {
-		signals_.clear();
+		signalbases_.clear();
 		logic_data_.reset();
+		for (std::shared_ptr<pv::view::View> view : views_)
+			view->clear_signals();
 		return;
 	}
 
@@ -370,8 +374,10 @@ void Session::update_signals()
 
 	const shared_ptr<sigrok::Device> sr_dev = device_->device();
 	if (!sr_dev) {
-		signals_.clear();
+		signalbases_.clear();
 		logic_data_.reset();
+		for (std::shared_ptr<pv::view::View> view : views_)
+			view->clear_signals();
 		return;
 	}
 
@@ -396,12 +402,10 @@ void Session::update_signals()
 		}
 	}
 
-	// Make the Signals list
-	{
-		unique_lock<shared_mutex> lock(signals_mutex_);
-
-		unordered_set< shared_ptr<view::Signal> > prev_sigs(signals_);
-		signals_.clear();
+	// Make the signals list
+	for (std::shared_ptr<pv::view::View> view : views_) {
+		unordered_set< shared_ptr<view::Signal> > prev_sigs(view->signals());
+		view->clear_signals();
 
 		for (auto channel : sr_dev->channels()) {
 			shared_ptr<data::SignalBase> signalbase;
@@ -417,29 +421,45 @@ void Session::update_signals()
 				// Copy the signal from the old set to the new
 				signal = *iter;
 			} else {
-				// Create a new signal
-				signalbase = shared_ptr<data::SignalBase>(
-					new data::SignalBase(channel));
+				// Find the signalbase for this channel if possible
+				signalbase.reset();
+				for (const shared_ptr<data::SignalBase> b : signalbases_)
+					if (b->channel() == channel)
+						signalbase = b;
 
 				switch(channel->type()->id()) {
 				case SR_CHANNEL_LOGIC:
-					signalbase->set_data(logic_data_);
+					if (!signalbase) {
+						signalbase = shared_ptr<data::SignalBase>(
+							new data::SignalBase(channel));
+						signalbases_.insert(signalbase);
+
+						all_signal_data_.insert(logic_data_);
+						signalbase->set_data(logic_data_);
+					}
+
 					signal = shared_ptr<view::Signal>(
 						new view::LogicSignal(*this,
 							device_, signalbase));
-					all_signal_data_.insert(logic_data_);
-					signalbases_.insert(signalbase);
+					view->add_signal(signal);
 					break;
 
 				case SR_CHANNEL_ANALOG:
 				{
-					shared_ptr<data::Analog> data(new data::Analog());
-					signalbase->set_data(data);
+					if (!signalbase) {
+						signalbase = shared_ptr<data::SignalBase>(
+							new data::SignalBase(channel));
+						signalbases_.insert(signalbase);
+
+						shared_ptr<data::Analog> data(new data::Analog());
+						all_signal_data_.insert(data);
+						signalbase->set_data(data);
+					}
+
 					signal = shared_ptr<view::Signal>(
 						new view::AnalogSignal(
 							*this, signalbase));
-					all_signal_data_.insert(data);
-					signalbases_.insert(signalbase);
+					view->add_signal(signal);
 					break;
 				}
 
@@ -448,9 +468,6 @@ void Session::update_signals()
 					break;
 				}
 			}
-
-			assert(signal);
-			signals_.insert(signal);
 		}
 	}
 
@@ -534,7 +551,6 @@ void Session::feed_in_trigger()
 	uint64_t sample_count = 0;
 
 	{
-		shared_lock<shared_mutex> lock(signals_mutex_);
 		for (const shared_ptr<pv::data::SignalData> d : all_signal_data_) {
 			assert(d);
 			uint64_t temp_count = 0;
@@ -604,7 +620,7 @@ void Session::feed_in_analog(shared_ptr<Analog> analog)
 	const float *data = static_cast<const float *>(analog->data_pointer());
 	bool sweep_beginning = false;
 
-	if (signals_.empty())
+	if (signalbases_.empty())
 		update_signals();
 
 	for (auto channel : channels) {
diff --git a/pv/session.hpp b/pv/session.hpp
index 96da72d3..83a73f4a 100644
--- a/pv/session.hpp
+++ b/pv/session.hpp
@@ -74,8 +74,7 @@ class Device;
 
 namespace view {
 class DecodeTrace;
-class LogicSignal;
-class Signal;
+class View;
 }
 
 class Session : public QObject
@@ -117,8 +116,12 @@ public:
 
 	double get_samplerate() const;
 
-	const std::unordered_set< std::shared_ptr<view::Signal> >
-		signals() const;
+	void register_view(std::shared_ptr<pv::view::View> view);
+
+	void deregister_view(std::shared_ptr<pv::view::View> view);
+
+	const std::unordered_set< std::shared_ptr<data::SignalBase> >
+		signalbases() const;
 
 #ifdef ENABLE_DECODE
 	bool add_decoder(srd_decoder *const dec);
@@ -159,16 +162,16 @@ private:
 	DeviceManager &device_manager_;
 	std::shared_ptr<devices::Device> device_;
 
+	std::unordered_set< std::shared_ptr<pv::view::View> > views_;
+
 	std::vector< std::shared_ptr<view::DecodeTrace> > decode_traces_;
 
 	mutable std::mutex sampling_mutex_; //!< Protects access to capture_state_.
 	capture_state capture_state_;
 
-	mutable boost::shared_mutex signals_mutex_;
-	std::unordered_set< std::shared_ptr<view::Signal> > signals_;
 
-	std::set< std::shared_ptr<data::SignalBase> > signalbases_;
-	std::set< std::shared_ptr<data::SignalData> > all_signal_data_;
+	std::unordered_set< std::shared_ptr<data::SignalBase> > signalbases_;
+	std::unordered_set< std::shared_ptr<data::SignalData> > all_signal_data_;
 
 	mutable std::recursive_mutex data_mutex_;
 	std::shared_ptr<data::Logic> logic_data_;
diff --git a/pv/storesession.cpp b/pv/storesession.cpp
index 00f6af63..16665369 100644
--- a/pv/storesession.cpp
+++ b/pv/storesession.cpp
@@ -38,7 +38,6 @@
 #include <pv/data/logicsegment.hpp>
 #include <pv/data/signalbase.hpp>
 #include <pv/devices/device.hpp>
-#include <pv/view/signal.hpp>
 
 #include <libsigrokcxx/libsigrokcxx.hpp>
 
@@ -63,6 +62,7 @@ using std::vector;
 
 using Glib::VariantBase;
 
+using sigrok::ChannelType;
 using sigrok::ConfigKey;
 using sigrok::Error;
 using sigrok::OutputFormat;
@@ -106,22 +106,20 @@ const QString& StoreSession::error() const
 
 bool StoreSession::start()
 {
-	const unordered_set< shared_ptr<view::Signal> > sigs(session_.signals());
+	const unordered_set< shared_ptr<data::SignalBase> > sigs(session_.signalbases());
 
 	shared_ptr<data::Segment> any_segment;
 	shared_ptr<data::LogicSegment> lsegment;
 	vector< shared_ptr<data::SignalBase> > achannel_list;
 	vector< shared_ptr<data::AnalogSegment> > asegment_list;
 
-	for (shared_ptr<view::Signal> signal : sigs) {
+	for (shared_ptr<data::SignalBase> signal : sigs) {
 		if (!signal->enabled())
 			continue;
 
-		shared_ptr<data::SignalData> data = signal->data();
-
-		if (dynamic_pointer_cast<data::Logic>(data)) {
+		if (signal->type() == ChannelType::LOGIC) {
 			// All logic channels share the same data segments
-			shared_ptr<data::Logic> ldata = dynamic_pointer_cast<data::Logic>(data);
+			shared_ptr<data::Logic> ldata = signal->logic_data();
 
 			const deque< shared_ptr<data::LogicSegment> > &lsegments =
 				ldata->logic_segments();
@@ -135,10 +133,9 @@ bool StoreSession::start()
 			any_segment = lsegment;
 		}
 
-		if (dynamic_pointer_cast<data::Analog>(data)) {
+		if (signal->type() == ChannelType::ANALOG) {
 			// Each analog channel has its own segments
-			shared_ptr<data::Analog> adata =
-				dynamic_pointer_cast<data::Analog>(data);
+			shared_ptr<data::Analog> adata = signal->analog_data();
 
 			const deque< shared_ptr<data::AnalogSegment> > &asegments =
 				adata->analog_segments();
@@ -151,7 +148,7 @@ bool StoreSession::start()
 			asegment_list.push_back(asegments.front());
 			any_segment = asegments.front();
 
-			achannel_list.push_back(signal->base());
+			achannel_list.push_back(signal);
 		}
 	}
 
diff --git a/pv/view/decodetrace.cpp b/pv/view/decodetrace.cpp
index b4201dd9..1f21b024 100644
--- a/pv/view/decodetrace.cpp
+++ b/pv/view/decodetrace.cpp
@@ -50,7 +50,6 @@ extern "C" {
 #include <pv/data/logic.hpp>
 #include <pv/data/logicsegment.hpp>
 #include <pv/data/decode/annotation.hpp>
-#include <pv/view/signal.hpp>
 #include <pv/view/view.hpp>
 #include <pv/view/viewport.hpp>
 #include <pv/widgets/decodergroupbox.hpp>
@@ -846,15 +845,15 @@ QComboBox* DecodeTrace::create_channel_selector(
 {
 	assert(dec);
 
-	const auto sigs(session_.signals());
+	const auto sigs(session_.signalbases());
 
-	vector< shared_ptr<Signal> > sig_list(sigs.begin(), sigs.end());
+	vector< shared_ptr<data::SignalBase> > sig_list(sigs.begin(), sigs.end());
 	std::sort(sig_list.begin(), sig_list.end(),
-		[](const shared_ptr<Signal> &a, const shared_ptr<Signal> b) {
-			return strnatcasecmp(a->base()->name().toStdString(),
-				b->base()->name().toStdString()) < 0; });
+		[](const shared_ptr<data::SignalBase> &a,
+		const shared_ptr<data::SignalBase> &b) {
+			return strnatcasecmp(a->name().toStdString(),
+				b->name().toStdString()) < 0; });
 
-	assert(decoder_stack_);
 	const auto channel_iter = dec->channels().find(pdch);
 
 	QComboBox *selector = new QComboBox(parent);
@@ -864,14 +863,14 @@ QComboBox* DecodeTrace::create_channel_selector(
 	if (channel_iter == dec->channels().end())
 		selector->setCurrentIndex(0);
 
-	for (const shared_ptr<view::Signal> &s : sig_list) {
-		assert(s);
-		if (s->base()->type() == sigrok::ChannelType::LOGIC && s->enabled()) {
-			selector->addItem(s->base()->name(),
-				qVariantFromValue((void*)s->base().get()));
+	for (const shared_ptr<data::SignalBase> &b : sig_list) {
+		assert(b);
+		if (b->type() == sigrok::ChannelType::LOGIC && b->enabled()) {
+			selector->addItem(b->name(),
+				qVariantFromValue((void*)b.get()));
 
 			if (channel_iter != dec->channels().end() &&
-				(*channel_iter).second == s->base())
+				(*channel_iter).second == b)
 				selector->setCurrentIndex(
 					selector->count() - 1);
 		}
@@ -886,7 +885,8 @@ void DecodeTrace::commit_decoder_channels(shared_ptr<data::decode::Decoder> &dec
 
 	map<const srd_channel*, shared_ptr<data::SignalBase> > channel_map;
 
-	const unordered_set< shared_ptr<Signal> > sigs(session_.signals());
+	const unordered_set< shared_ptr<data::SignalBase> >
+		sigs(session_.signalbases());
 
 	for (const ChannelSelector &s : channel_selectors_) {
 		if (s.decoder_ != dec)
@@ -896,9 +896,9 @@ void DecodeTrace::commit_decoder_channels(shared_ptr<data::decode::Decoder> &dec
 			(data::SignalBase*)s.combo_->itemData(
 				s.combo_->currentIndex()).value<void*>();
 
-		for (shared_ptr<Signal> sig : sigs)
-			if (sig->base().get() == selection) {
-				channel_map[s.pdch_] = sig->base();
+		for (shared_ptr<data::SignalBase> sig : sigs)
+			if (sig.get() == selection) {
+				channel_map[s.pdch_] = sig;
 				break;
 			}
 	}
diff --git a/pv/view/view.cpp b/pv/view/view.cpp
index 570e7b76..7e47140c 100644
--- a/pv/view/view.cpp
+++ b/pv/view/view.cpp
@@ -186,6 +186,21 @@ const Session& View::session() const
 	return session_;
 }
 
+std::unordered_set< std::shared_ptr<view::Signal> > View::signals() const
+{
+	return signals_;
+}
+
+void View::clear_signals()
+{
+	signals_.clear();
+}
+
+void View::add_signal(const shared_ptr<view::Signal> signal)
+{
+	signals_.insert(signal);
+}
+
 View* View::view()
 {
 	return this;
@@ -397,11 +412,9 @@ void View::set_scale_offset(double scale, const Timestamp& offset)
 
 set< shared_ptr<SignalData> > View::get_visible_data() const
 {
-	const unordered_set< shared_ptr<Signal> > sigs(session().signals());
-
 	// Make a set of all the visible data objects
 	set< shared_ptr<SignalData> > visible_data;
-	for (const shared_ptr<Signal> sig : sigs)
+	for (const shared_ptr<Signal> sig : signals_)
 		if (sig->enabled())
 			visible_data.insert(sig->data());
 
@@ -803,10 +816,8 @@ void View::determine_time_unit()
 {
 	// Check whether we know the sample rate and hence can use time as the unit
 	if (time_unit_ == util::TimeUnit::Samples) {
-		const unordered_set< shared_ptr<Signal> > sigs(session().signals());
-
 		// Check all signals but...
-		for (const shared_ptr<Signal> signal : sigs) {
+		for (const shared_ptr<Signal> signal : signals_) {
 			const shared_ptr<SignalData> data = signal->data();
 
 			// ...only check first segment of each
@@ -938,6 +949,7 @@ void View::signals_changed()
 
 	if (!session_.device()) {
 		reset_scroll();
+		signals_.clear();
 	} else {
 		sr_dev = session_.device()->device();
 		assert(sr_dev);
@@ -952,9 +964,7 @@ void View::signals_changed()
 	const set<shared_ptr<Trace>> prev_traces(
 		prev_trace_list.begin(), prev_trace_list.end());
 
-	const unordered_set< shared_ptr<Signal> > sigs(session_.signals());
-
-	set< shared_ptr<Trace> > traces(sigs.begin(), sigs.end());
+	set< shared_ptr<Trace> > traces(signals_.begin(), signals_.end());
 
 #ifdef ENABLE_DECODE
 	const vector< shared_ptr<DecodeTrace> > decode_traces(
@@ -975,7 +985,7 @@ void View::signals_changed()
 	// Make a look-up table of sigrok Channels to pulseview Signals
 	unordered_map<shared_ptr<data::SignalBase>, shared_ptr<Signal> >
 		signal_map;
-	for (const shared_ptr<Signal> &sig : sigs)
+	for (const shared_ptr<Signal> &sig : signals_)
 		signal_map[sig->base()] = sig;
 
 	// Populate channel groups
diff --git a/pv/view/view.hpp b/pv/view/view.hpp
index 47f1b8dc..d699be4f 100644
--- a/pv/view/view.hpp
+++ b/pv/view/view.hpp
@@ -53,6 +53,7 @@ namespace view {
 class CursorHeader;
 class Header;
 class Ruler;
+class Signal;
 class Trace;
 class Viewport;
 class TriggerMarker;
@@ -81,6 +82,15 @@ public:
 	Session& session();
 	const Session& session() const;
 
+	/**
+	 * Returns the signals contained in this view.
+	 */
+	std::unordered_set< std::shared_ptr<view::Signal> > signals() const;
+
+	void clear_signals();
+
+	void add_signal(const std::shared_ptr<view::Signal> signal);
+
 	/**
 	 * Returns the view of the owner.
 	 */
@@ -355,6 +365,8 @@ private:
 	Ruler *ruler_;
 	Header *header_;
 
+	std::unordered_set< std::shared_ptr<view::Signal> > signals_;
+
 	/// The view time scale in seconds per pixel.
 	double scale_;
 
-- 
2.30.2