Undo: turn dive- and trip-fields into flags

The divesEdited signal sends the changed field as a parameter.
Since some undo-commands change multiple fields, this led to
numerous signals for a single command. This in turn would lead
to multiple profile-reloads and statistic recalculations.

Therefore, turn the enum into a bitfield. For simplicity,
provide a constructor that takes classical flags and turns
them into the bitfield. This is necessary because C-style
named initialization is only supported on C++20 onward!

Is this somewhat overengineered? Yes, maybe.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>

core/subsurface-qt/DiveListNotifier.h

@@ -9,30 +9,59 @@
 
 #include <QObject>
 
-// Dive and trip fields that can be edited.
+// Dive and trip fields that can be edited. Use bit fields so that we can pass multiple fields at once.
-// Use "enum class" to not polute the global name space.
+// Provides an inlined flag-based constructur because sadly C-style designated initializers are only supported since C++20.
-enum class DiveField {
+struct DiveField {
-	NR,
+	// Note: using int instead of the more natural bool, because gcc produces significantly worse code with
-	DATETIME,
+	// bool. clang, on the other hand, does fine.
-	DEPTH,
+	unsigned int nr : 1;
-	DURATION,
+	unsigned int datetime : 1;
-	AIR_TEMP,
+	unsigned int depth : 1;
-	WATER_TEMP,
+	unsigned int duration : 1;
-	ATM_PRESS,
+	unsigned int air_temp : 1;
-	DIVESITE,
+	unsigned int water_temp : 1;
-	DIVEMASTER,
+	unsigned int atm_press : 1;
-	BUDDY,
+	unsigned int divesite : 1;
-	RATING,
+	unsigned int divemaster : 1;
-	VISIBILITY,
+	unsigned int buddy : 1;
-	SUIT,
+	unsigned int rating : 1;
-	TAGS,
+	unsigned int visibility : 1;
-	MODE,
+	unsigned int suit : 1;
-	NOTES,
+	unsigned int tags : 1;
-	SALINITY
+	unsigned int mode : 1;
+	unsigned int notes : 1;
+	unsigned int salinity : 1;
+	enum Flags {
+		NONE = 0,
+		NR = 1 << 0,
+		DATETIME = 1 << 1,
+		DEPTH = 1 << 2,
+		DURATION = 1 << 3,
+		AIR_TEMP = 1 << 4,
+		WATER_TEMP = 1 << 5,
+		ATM_PRESS = 1 << 6,
+		DIVESITE = 1 << 7,
+		DIVEMASTER = 1 << 8,
+		BUDDY = 1 << 9,
+		RATING = 1 << 10,
+		VISIBILITY = 1 << 11,
+		SUIT = 1 << 12,
+		TAGS = 1 << 13,
+		MODE = 1 << 14,
+		NOTES = 1 << 15,
+		SALINITY = 1 << 16
+	};
+	DiveField(int flags);
 };
-enum class TripField {
+struct TripField {
-	LOCATION,
+	unsigned int location : 1;
-	NOTES
+	unsigned int notes : 1;
+	enum Flags {
+		NONE = 0,
+		LOCATION = 1 << 0,
+		NOTES = 1 << 1
+	};
+	TripField(int flags);
 };
 
 class DiveListNotifier : public QObject {
@@ -130,4 +159,30 @@ inline DiveListNotifier::InCommandMarker DiveListNotifier::enterCommand()
 	return InCommandMarker(*this);
 }
 
+inline DiveField::DiveField(int flags) :
+	nr((flags & NR) != 0),
+	datetime((flags & DATETIME) != 0),
+	depth((flags & DEPTH) != 0),
+	duration((flags & DURATION) != 0),
+	air_temp((flags & AIR_TEMP) != 0),
+	water_temp((flags & WATER_TEMP) != 0),
+	atm_press((flags & ATM_PRESS) != 0),
+	divesite((flags & DIVESITE) != 0),
+	divemaster((flags & DIVEMASTER) != 0),
+	buddy((flags & BUDDY) != 0),
+	rating((flags & RATING) != 0),
+	visibility((flags & VISIBILITY) != 0),
+	suit((flags & SUIT) != 0),
+	tags((flags & TAGS) != 0),
+	mode((flags & MODE) != 0),
+	notes((flags & NOTES) != 0),
+	salinity((flags & SALINITY) != 0)
+{
+}
+
+inline TripField::TripField(int flags) :
+	location((flags & LOCATION) != 0),
+	notes((flags & NOTES) != 0)
+{
+}
 #endif

desktop-widgets/command_edit.cpp

@@ -803,25 +803,16 @@ void PasteDives::undo()
 	}
 
 	// Send signals.
-	// TODO: We send one signal per changed field. This means that the dive list may
+	DiveField fields(DiveField::NONE);
-	// update the entry numerous times. Perhaps change the field-id into flags?
+	fields.notes = what.notes;
-	// There seems to be a number of enums / flags describing dive fields. Perhaps unify them all?
+	fields.divemaster = what.divemaster;
-	if (what.notes)
+	fields.buddy = what.buddy;
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::NOTES);
+	fields.suit = what.suit;
-	if (what.divemaster)
+	fields.rating = what.rating;
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::DIVEMASTER);
+	fields.visibility = what.visibility;
-	if (what.buddy)
+	fields.divesite = what.divesite;
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::BUDDY);
+	fields.tags = what.tags;
-	if (what.suit)
+	emit diveListNotifier.divesChanged(divesToNotify, fields);
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::SUIT);
-	if (what.rating)
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::RATING);
-	if (what.visibility)
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::VISIBILITY);
-	if (what.divesite)
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::DIVESITE);
-	if (what.tags)
-		emit diveListNotifier.divesChanged(divesToNotify, DiveField::TAGS);
 	if (what.cylinders)
 		emit diveListNotifier.cylindersReset(divesToNotify);
 	if (what.weights)
@@ -888,14 +879,8 @@ void ReplanDive::undo()
 	fixup_dive(d);
 
 	QVector<dive *> divesToNotify = { d };
-	// TODO: Turn field into flags to avoid multiple signals
+	emit diveListNotifier.divesChanged(divesToNotify, DiveField::DATETIME | DiveField::DURATION | DiveField::DEPTH | DiveField::MODE |
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::DATETIME);
+							  DiveField::NOTES | DiveField::SALINITY | DiveField::ATM_PRESS);
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::DURATION);
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::DEPTH);
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::MODE);
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::NOTES);
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::SALINITY);
-	emit diveListNotifier.divesChanged(divesToNotify, DiveField::ATM_PRESS);
 	emit diveListNotifier.cylindersReset(divesToNotify);
 }
 

desktop-widgets/mapwidget.cpp

@@ -104,7 +104,7 @@ void MapWidget::coordinatesChanged(struct dive_site *ds, const location_t &locat
 
 void MapWidget::divesChanged(const QVector<dive *> &, DiveField field)
 {
-	if (field == DiveField::DIVESITE)
+	if (field.divesite)
 		reload();
 }
 

desktop-widgets/tab-widgets/TabDiveEquipment.cpp

@@ -108,13 +108,8 @@ void TabDiveEquipment::divesChanged(const QVector<dive *> &dives, DiveField fiel
 	if (!current_dive || !dives.contains(current_dive))
 		return;
 
-	switch(field) {
+	if (field.suit)
-	case DiveField::SUIT:
 		ui.suit->setText(QString(current_dive->suit));
-		break;
-	default:
-		break;
-	}
 }
 
 void TabDiveEquipment::toggleTriggeredColumn()

desktop-widgets/tab-widgets/TabDiveInformation.cpp

@@ -141,30 +141,18 @@ void TabDiveInformation::divesChanged(const QVector<dive *> &dives, DiveField fi
 	if (!current_dive || !dives.contains(current_dive))
 		return;
 
-	switch(field) {
+	if (field.duration || field.depth || field.mode)
-	case DiveField::DURATION:
-	case DiveField::DEPTH:
-	case DiveField::MODE:
 		updateProfile();
-		break;
+	if (field.air_temp)
-	case DiveField::AIR_TEMP:
 		ui->airTemperatureText->setText(get_temperature_string(current_dive->airtemp, true));
-		break;
+	if (field.water_temp)
-	case DiveField::WATER_TEMP:
 		ui->waterTemperatureText->setText(get_temperature_string(current_dive->watertemp, true));
-		break;
+	if (field.atm_press)
-	case DiveField::ATM_PRESS:
 		ui->atmPressVal->setText(ui->atmPressVal->text().sprintf("%d",current_dive->surface_pressure.mbar));
-		break;
+	if (field.datetime)
-	case DiveField::DATETIME:
 		updateWhen();
-		break;
+	if (field.salinity)
-	case DiveField::SALINITY:
 		updateSalinity();
-		break;
-	default:
-		break;
-	}
 }
 
 void TabDiveInformation::on_atmPressType_currentIndexChanged(int index) { updateTextBox(COMBO_CHANGED); }

desktop-widgets/tab-widgets/TabDiveStatistics.cpp

@@ -53,17 +53,8 @@ void TabDiveStatistics::divesChanged(const QVector<dive *> &dives, DiveField fie
 		return;
 
 	// TODO: make this more fine grained. Currently, the core can only calculate *all* statistics.
-	switch(field) {
+	if (field.duration || field.depth || field.mode || field.air_temp || field.water_temp)
-	case DiveField::DURATION:
-	case DiveField::DEPTH:
-	case DiveField::MODE:
-	case DiveField::AIR_TEMP:
-	case DiveField::WATER_TEMP:
 		updateData();
-		break;
-	default:
-		break;
-	}
 }
 
 void TabDiveStatistics::updateData()

desktop-widgets/tab-widgets/maintab.cpp

@@ -284,53 +284,39 @@ void MainTab::divesChanged(const QVector<dive *> &dives, DiveField field)
 	if (!current_dive || !dives.contains(current_dive))
 		return;
 
-	switch(field) {
+	if (field.duration)
-	case DiveField::DURATION:
 		ui.duration->setText(render_seconds_to_string(current_dive->duration.seconds));
-		profileFromDive(current_dive);
+	if (field.depth)
-		break;
-	case DiveField::DEPTH:
 		ui.depth->setText(get_depth_string(current_dive->maxdepth, true));
-		profileFromDive(current_dive);
+	if (field.air_temp)
-		break;
-	case DiveField::AIR_TEMP:
 		ui.airtemp->setText(get_temperature_string(current_dive->airtemp, true));
-		break;
+	if (field.water_temp)
-	case DiveField::WATER_TEMP:
 		ui.watertemp->setText(get_temperature_string(current_dive->watertemp, true));
-		break;
+	if (field.rating)
-	case DiveField::RATING:
 		ui.rating->setCurrentStars(current_dive->rating);
-		break;
+	if (field.visibility)
-	case DiveField::VISIBILITY:
 		ui.visibility->setCurrentStars(current_dive->visibility);
-		break;
+	if (field.notes)
-	case DiveField::NOTES:
 		updateNotes(current_dive);
-		break;
+	if (field.mode)
-	case DiveField::MODE:
 		updateMode(current_dive);
-		break;
+	if (field.datetime) {
-	case DiveField::DATETIME:
 		updateDateTime(current_dive);
 		MainWindow::instance()->graphics->dateTimeChanged();
 		DivePlannerPointsModel::instance()->getDiveplan().when = current_dive->when;
-		break;
-	case DiveField::DIVESITE:
-		updateDiveSite(current_dive);
-		break;
-	case DiveField::TAGS:
-		ui.tagWidget->setText(get_taglist_string(current_dive->tag_list));
-		break;
-	case DiveField::BUDDY:
-		ui.buddy->setText(current_dive->buddy);
-		break;
-	case DiveField::DIVEMASTER:
-		ui.divemaster->setText(current_dive->divemaster);
-		break;
-	default:
-		break;
 	}
+	if (field.divesite)
+		updateDiveSite(current_dive);
+	if (field.tags)
+		ui.tagWidget->setText(get_taglist_string(current_dive->tag_list));
+	if (field.buddy)
+		ui.buddy->setText(current_dive->buddy);
+	if (field.divemaster)
+		ui.divemaster->setText(current_dive->divemaster);
+
+	// If duration or depth changed, the profile needs to be replotted
+	if (field.duration || field.depth)
+		profileFromDive(current_dive);
 }
 
 void MainTab::diveSiteEdited(dive_site *ds, int)
@@ -347,16 +333,10 @@ void MainTab::tripChanged(dive_trip *trip, TripField field)
 	if (currentTrip != trip)
 		return;
 
-	switch(field) {
+	if (field.notes)
-	case TripField::NOTES:
 		ui.notes->setText(currentTrip->notes);
-		break;
+	if (field.location)
-	case TripField::LOCATION:
 		ui.diveTripLocation->setText(currentTrip->location);
-		break;
-	default:
-		break;
-	}
 }
 
 void MainTab::nextInputField(QKeyEvent *event)