Picture handling: cleaning up the mess

We had pointers to data structures on the stack which we frequently
reallocated. These data structure contain basically a filename and an
offset. We then create a hash of the pointers to those datastructures with
the filename being the key. And then we passed those pointers around
through a Qt model(!!!) only in order to then later look up by filename
what the offset might be.

I am at a loss for words for the lunacy behind this design.

How about we just remember the offsets and pass the integers around?

Signed-off-by: Dirk Hohndel <dirk@hohndel.org>

dive.c

@@ -2495,10 +2495,10 @@ static void picture_free( struct picture *p){
 	free( p->filename );
 	free( p );
 }
-void dive_remove_picture(struct picture *p)
+void dive_remove_picture(char *filename)
 {
 	struct picture **ep = &current_dive->picture_list;
-	while (ep && !same_string((*ep)->filename, p->filename))
+	while (ep && !same_string((*ep)->filename, filename))
 		ep = &(*ep)->next;
 	if (ep) {
 		struct picture *temp = (*ep)->next;

dive.h

@@ -311,7 +311,7 @@ struct picture {
 extern struct picture *alloc_picture();
 extern void dive_create_picture(struct dive *d, char *filename, int shift_time);
 extern void dive_add_picture(struct dive *d, struct picture *newpic);
-extern void dive_remove_picture(struct picture *pic);
+extern void dive_remove_picture(char *filename);
 extern unsigned int dive_get_picture_count(struct dive *d);
 extern void picture_load_exif_data(struct picture *p, timestamp_t *timestamp);
 extern void dive_set_geodata_from_picture(struct dive *d, struct picture *pic);

qt-ui/divepicturewidget.cpp

@@ -49,7 +49,7 @@ void DivePictureModel::updateDivePictures()
 	stringPixmapCache.clear();
 	QStringList pictures;
 	FOR_EACH_PICTURE (&displayed_dive) {
-		stringPixmapCache[QString(picture->filename)].picture = picture;
+		stringPixmapCache[QString(picture->filename)].offsetSeconds = picture->offset.seconds;
 		pictures.push_back(QString(picture->filename));
 	}
 
@@ -89,7 +89,7 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const
 	} else if (index.column() == 1) {
 		switch (role) {
 		case Qt::UserRole:
-			ret = QVariant::fromValue((void *)stringPixmapCache[key].picture);
+			ret = QVariant::fromValue((int)stringPixmapCache[key].offsetSeconds);
 		break;
 		case Qt::DisplayRole:
 			ret = key;
@@ -100,7 +100,7 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const
 
 void DivePictureModel::removePicture(const QString &fileUrl)
 {
-	dive_remove_picture(stringPixmapCache[fileUrl].picture);
+	dive_remove_picture(fileUrl.toUtf8().data());
 	copy_dive(current_dive, &displayed_dive);
 	updateDivePictures();
 	mark_divelist_changed(true);

qt-ui/divepicturewidget.h

@@ -5,10 +5,9 @@
 #include <QListView>
 #include <QThread>
 
-struct picture;
 struct PhotoHelper {
 	QImage image;
-	struct picture *picture;
+	int offsetSeconds;
 };
 
 class DivePictureModel : public QAbstractTableModel {

qt-ui/profile/profilewidget2.cpp

@@ -1360,17 +1360,17 @@ void ProfileWidget2::plotPictures()
 	double x, y, lastX = -1.0, lastY = -1.0;
 	DivePictureModel *m = DivePictureModel::instance();
 	for (int i = 0; i < m->rowCount(); i++) {
-		struct picture *pic  = (struct picture*) m->index(i,1).data(Qt::UserRole).value<void*>();
+		int offsetSeconds = m->index(i,1).data(Qt::UserRole).value<int>();
 		// it's a correct picture, but doesn't have a timestamp: only show on the widget near the
 		// information area.
-		if (!pic->offset.seconds)
+		if (!offsetSeconds)
 			continue;
 		DivePictureItem *item = new DivePictureItem();
 		item->setPixmap(m->index(i,0).data(Qt::DecorationRole).value<QPixmap>());
 		item->setFileUrl(m->index(i,1).data().toString());
 		// let's put the picture at the correct time, but at a fixed "depth" on the profile
 		// not sure this is ideal, but it seems to look right.
-		x = timeAxis->posAtValue(pic->offset.seconds);
+		x = timeAxis->posAtValue(offsetSeconds);
 		if (i == 0)
 			y = 10;
 		else if (fabs(x - lastX) < 4)