Dive pictures: remove hashes

In the last commits, the canonical-to-local filename map was made
independent from the image hashes and the location of moved images
was based on filename not hashes. The hashes are now in principle
unused (except for conversion of old-style local filename lookups).

Therefore, remove the hashes in this commit. This makes addition
of images distinctly faster.

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

core/imagedownloader.cpp

@@ -61,7 +61,6 @@ void ImageDownloader::saveImage(QNetworkReply *reply)
 				imageFile.waitForBytesWritten(-1);
 				imageFile.close();
 				learnPictureFilename(filename, imageFile.fileName());
-				hashPicture(filename);	// hashPicture transforms canonical into local filename
 			}
 			emit loaded(filename);
 		}
@@ -83,10 +82,6 @@ static std::pair<QImage, bool> fetchImage(const QString &filename, const QString
 	QUrl url = QUrl::fromUserInput(filename);
 	if (url.isLocalFile()) {
 		thumb.load(url.toLocalFile());
-		// If we loaded successfully, make sure the hash is up to date.
-		// Note that hashPicture() takes the *original* filename.
-		if (!thumb.isNull())
-			hashPicture(originalFilename);
 	} else if (tryDownload) {
 		// This has to be done in UI main thread, because QNetworkManager refuses
 		// to treat requests from other threads. invokeMethod() is Qt's way of calling a
@@ -247,8 +242,7 @@ void Thumbnailer::processItem(QString filename, bool tryDownload)
 
 void Thumbnailer::imageDownloaded(QString filename)
 {
-	// Image was downloaded and the filename connected with a hash.
-	// Try thumbnailing again.
+	// Image was downloaded -> try thumbnailing again.
 	QMutexLocker l(&lock);
 	workingOn[filename] = QtConcurrent::run(&pool, [this, filename]() { processItem(filename, false); });
 }

core/load-git.c

@@ -953,15 +953,6 @@ static void parse_picture_gps(char *line, struct membuffer *str, void *_pic)
 	pic->longitude = parse_degrees(line, &line);
 }
 
-static void parse_picture_hash(char *line, struct membuffer *str, void *_pic)
-{
-	UNUSED(line);
-	struct picture *pic = _pic;
-	char *hash = get_utf8(str);
-	register_hash(pic->filename, get_utf8(str));
-	free(hash);
-}
-
 /* These need to be sorted! */
 struct keyword_action dc_action[] = {
 #undef D
@@ -1035,7 +1026,7 @@ static void settings_parser(char *line, struct membuffer *str, void *_unused)
 static struct keyword_action picture_action[] = {
 #undef D
 #define D(x) { #x, parse_picture_ ## x }
-	D(filename), D(gps), D(hash)
+	D(filename), D(gps)
 };
 
 static void picture_parser(char *line, struct membuffer *str, void *_pic)

core/parse-xml.c

@@ -1221,7 +1221,7 @@ static void try_to_fill_dive(struct dive *dive, const char *name, char *buf)
 	if (MATCH("gps.picture", gps_picture_location, cur_picture))
 		return;
 	if (MATCH("hash.picture", utf8_string, &hash)) {
-		register_hash(cur_picture->filename, hash);
+		/* Legacy -> ignore. */
 		free(hash);
 		return;
 	}

core/qthelper.cpp

@@ -1042,27 +1042,10 @@ extern "C" void reverseGeoLookup(degrees_t latitude, degrees_t longitude, uint32
 	}
 }
 
-QHash<QString, QByteArray> hashOf;
-QMutex hashOfMutex;
-QHash<QString, QString> localFilenameOf;
+static QMutex hashOfMutex;
+static QHash<QString, QString> localFilenameOf;
 
-static QByteArray getHash(const QString &filename)
-{
-	QMutexLocker locker(&hashOfMutex);
-	return hashOf[filename];
-}
-
-QString hashString(const char *filename)
-{
-	return getHash(QString(filename)).toHex();
-}
-
-extern "C" char * hashstring(const char *filename)
-{
-	return strdup(qPrintable(hashString(filename)));
-}
-
-const QString hashfile_name()
+static const QString hashfile_name()
 {
 	return QString(system_default_directory()).append("/hashes");
 }
@@ -1137,7 +1120,7 @@ struct HashToFile {
 
 // During a transition period, convert the hash->localFilename into a canonicalFilename->localFilename.
 // TODO: remove this code in due course
-static void convertLocalFilename(const QHash<QByteArray, QString> &hashToLocal)
+static void convertLocalFilename(const QHash<QString, QByteArray> &hashOf, const QHash<QByteArray, QString> &hashToLocal)
 {
 	// Bail out early if there is nothing to do
 	if (hashToLocal.isEmpty())
@@ -1172,25 +1155,20 @@ void read_hashes()
 	if (hashfile.open(QIODevice::ReadOnly)) {
 		QDataStream stream(&hashfile);
 		QHash<QByteArray, QString> localFilenameByHash;
+		QHash<QString, QByteArray> hashOf;
 		stream >> localFilenameByHash;		// For backwards compatibility
-		QMutexLocker locker(&hashOfMutex);
-		stream >> hashOf;
-		locker.unlock();
+		stream >> hashOf;			// For backwards compatibility
 		QHash <QString, QImage> thumbnailCache;
 		stream >> thumbnailCache;		// For backwards compatibility
+		QMutexLocker locker(&hashOfMutex);
 		stream >> localFilenameOf;
+		locker.unlock();
 		hashfile.close();
 		convertThumbnails(thumbnailCache);
-		convertLocalFilename(localFilenameByHash);
+		convertLocalFilename(hashOf, localFilenameByHash);
 	}
 	QMutexLocker locker(&hashOfMutex);
 	localFilenameOf.remove("");
-	QMutableHashIterator<QString, QByteArray> iter(hashOf);
-	while (iter.hasNext()) {
-		iter.next();
-		if (iter.value().isEmpty())
-			iter.remove();
-	}
 
 	// Make sure that the thumbnail directory exists
 	QDir().mkpath(thumbnailDir());
@@ -1204,7 +1182,7 @@ void write_hashes()
 	if (hashfile.open(QIODevice::WriteOnly)) {
 		QDataStream stream(&hashfile);
 		stream << QHash<QByteArray, QString>();	// Empty hash to filename - for backwards compatibility
-		stream << hashOf;
+		stream << QHash<QString, QByteArray>(); // Empty hashes - for backwards compatibility
 		stream << QHash<QString,QImage>();	// Empty thumbnailCache - for backwards compatibility
 		stream << localFilenameOf;
 		hashfile.commit();
@@ -1213,32 +1191,6 @@ void write_hashes()
 	}
 }
 
-// Add hash if not already known
-extern "C" void register_hash(const char *filename, const char *hash)
-{
-	if (empty_string(filename) || empty_string(hash))
-		return;
-	QString filenameString(filename);
-
-	QMutexLocker locker(&hashOfMutex);
-	if (!hashOf.contains(filenameString)) {
-		QByteArray hashBuf = QByteArray::fromHex(hash);
-		hashOf[filename] =  hashBuf;
-	}
-}
-
-QByteArray hashFile(const QString &filename)
-{
-	QCryptographicHash hash(QCryptographicHash::Sha1);
-	QFile imagefile(filename);
-	if (imagefile.exists() && imagefile.open(QIODevice::ReadOnly)) {
-		hash.addData(&imagefile);
-		return hash.result();
-	} else {
-		return QByteArray();
-	}
-}
-
 void learnPictureFilename(const QString &originalName, const QString &localName)
 {
 	if (originalName.isEmpty() || localName.isEmpty())
@@ -1257,18 +1209,6 @@ QString localFilePath(const QString &originalFilename)
 	return localFilenameOf.value(originalFilename, originalFilename);
 }
 
-// This works on a copy of the string, because it runs in asynchronous context
-void hashPicture(QString filename)
-{
-	QByteArray oldHash = getHash(filename);
-	QByteArray hash = hashFile(localFilePath(filename));
-	if (!hash.isEmpty() && hash != oldHash) {
-		QMutexLocker locker(&hashOfMutex);
-		hashOf[filename] =  hash;
-		mark_divelist_changed(true);
-	}
-}
-
 QStringList imageExtensionFilters() {
 	QStringList filters;
 	foreach (QString format, QImageReader::supportedImageFormats()) {
@@ -1312,19 +1252,19 @@ struct ImageMatch {
 	int score;
 };
 
-static void learnImage(const QString &filename, QMap<QString, ImageMatch> &matches)
+static void learnImage(const QString &filename, QMap<QString, ImageMatch> &matches, const QVector<QString> &imageFilenames)
 {
 	// Find the original filenames with the highest match-score
 	QStringList newMatches;
-	QByteArray hash = hashFile(filename);
 	int bestScore = 1;
-	for (auto it = hashOf.cbegin(); it != hashOf.cend(); ++it) {
-		int score = matchFilename(filename, it.key());
+
+	for (const QString &originalFilename: imageFilenames) {
+		int score = matchFilename(filename, originalFilename);
 		if (score < bestScore)
 			continue;
 		if (score > bestScore)
 			newMatches.clear();
-		newMatches.append(it.key());
+		newMatches.append(originalFilename);
 		bestScore = score;
 	}
 
@@ -1338,7 +1278,7 @@ static void learnImage(const QString &filename, QMap<QString, ImageMatch> &match
 	}
 }
 
-void learnImages(const QStringList &dirNames, int max_recursions)
+void learnImages(const QStringList &dirNames, int max_recursions, const QVector<QString> &imageFilenames)
 {
 	QStringList filters = imageExtensionFilters();
 	QMap<QString, ImageMatch> matches;
@@ -1354,7 +1294,7 @@ void learnImages(const QStringList &dirNames, int max_recursions)
 		QDir dir(stack.last().takeLast());
 
 		for (const QString &file: dir.entryList(filters, QDir::Files))
-			learnImage(dir.absoluteFilePath(file), matches);
+			learnImage(dir.absoluteFilePath(file), matches, imageFilenames);
 		if (stack.size() <= max_recursions) {
 			stack.append(QStringList());
 			for (const QString &dirname: dir.entryList(QStringList(), QDir::NoDotAndDotDot | QDir::Dirs))

core/qthelper.h

@@ -27,16 +27,10 @@ QString get_divepoint_gas_string(struct dive *d, const divedatapoint& dp);
 QString get_taglist_string(struct tag_entry *tag_list);
 void read_hashes();
 void write_hashes();
-void updateHash(struct picture *picture);
-QByteArray hashFile(const QString &filename);
-QString hashString(const char *filename);
 QString thumbnailFileName(const QString &filename);
-void learnImages(const QStringList &dirNames, int max_recursions);
+void learnImages(const QStringList &dirNames, int max_recursions, const QVector<QString> &imageFilenames);
 void learnPictureFilename(const QString &originalName, const QString &localName);
-void hashPicture(QString filename);
-extern "C" char *hashstring(const char *filename);
 QString localFilePath(const QString &originalFilename);
-void learnHash(const QString &originalName, const QString &localName, const QByteArray &hash);
 weight_t string_to_weight(const char *str);
 depth_t string_to_depth(const char *str);
 pressure_t string_to_pressure(const char *str);
@@ -109,8 +103,6 @@ void updateWindowTitle();
 void subsurface_mkdir(const char *dir);
 char *get_file_name(const char *fileName);
 void copy_image_and_overwrite(const char *cfileName, const char *path, const char *cnewName);
-char *hashstring(const char *filename);
-void register_hash(const char *filename, const char *hash);
 char *move_away(const char *path);
 const char *local_file_path(struct picture *picture);
 char *cloud_url();

core/save-git.c

@@ -610,14 +610,10 @@ static int save_one_picture(git_repository *repo, struct dir *dir, struct pictur
 	int offset = pic->offset.seconds;
 	struct membuffer buf = { 0 };
 	char sign = '+';
-	char *hash;
 	unsigned h;
 
 	show_utf8(&buf, "filename ", pic->filename, "\n");
 	show_gps(&buf, pic->latitude, pic->longitude);
-	hash = hashstring(pic->filename);
-	show_utf8(&buf, "hash ", hash, "\n");
-	free(hash);
 
 	/* Picture loading will load even negative offsets.. */
 	if (offset < 0) {

core/save-xml.c

@@ -439,10 +439,6 @@ static void save_picture(struct membuffer *b, struct picture *pic)
 		put_degrees(b, pic->latitude, " gps='", " ");
 		put_degrees(b, pic->longitude, "", "'");
 	}
-	char *hash = hashstring(pic->filename);
-	if (!empty_string(hash))
-		put_format(b, " hash='%s'", hash);
-	free(hash);
 
 	put_string(b, "/>\n");
 }

core/subsurfacestartup.c

@@ -180,7 +180,7 @@ void print_files()
 	printf("Cloud URL: %s\n", filename);
 	free((void *)filename);
 	char *tmp = hashfile_name_string();
-	printf("Image hashes: %s\n", tmp);
+	printf("Image filename table: %s\n", tmp);
 	free(tmp);
 	tmp = picturedir_string();
 	printf("Local picture directory: %s\n\n", tmp);

desktop-widgets/divelistview.cpp

@@ -986,9 +986,8 @@ void DiveListView::loadImageFromURL(QUrl url)
 			return;
 		}
 
-		// Since we already downloaded the image we can cache it as well.
 		QCryptographicHash hash(QCryptographicHash::Sha1);
-		hash.addData(imageData);
+		hash.addData(url.toString().toUtf8());
 		QString path = QStandardPaths::standardLocations(QStandardPaths::CacheLocation).first();
 		QDir dir(path);
 		if (!dir.exists())

desktop-widgets/mainwindow.cpp

@@ -701,9 +701,9 @@ void MainWindow::on_actionCloudOnline_triggered()
 	updateCloudOnlineStatus();
 }
 
-static void learnImageDirs(QStringList dirnames)
+static void learnImageDirs(QStringList dirnames, QVector<QString> imageFilenames)
 {
-	learnImages(dirnames, 10);
+	learnImages(dirnames, 10, imageFilenames);
 	DivePictureModel::instance()->updateDivePictures();
 }
 
@@ -720,7 +720,13 @@ void MainWindow::on_actionHash_images_triggered()
 		dirnames = dialog.selectedFiles();
 	if (dirnames.isEmpty())
 		return;
-	future = QtConcurrent::run(learnImageDirs,dirnames);
+	QVector<QString> imageFilenames;
+	int i;
+	struct dive *dive;
+	for_each_dive (i, dive)
+		FOR_EACH_PICTURE(dive)
+			imageFilenames.append(QString(picture->filename));
+	future = QtConcurrent::run(learnImageDirs, dirnames, imageFilenames);
 	MainWindow::instance()->getNotificationWidget()->showNotification(tr("Scanning images...(this can take a while)"), KMessageWidget::Information);
 	MainWindow::instance()->getNotificationWidget()->setFuture(future);
 

tests/testpicture.cpp

@@ -44,12 +44,8 @@ void TestPicture::addPicture()
 	QVERIFY(pic1->longitude.udeg == 11334500);
 	QVERIFY(pic2->offset.seconds == 1321);
 
-	hashPicture(pic1->filename);
-	hashPicture(pic2->filename);
 	learnPictureFilename(pic1->filename, PIC1_NAME);
 	learnPictureFilename(pic2->filename, PIC2_NAME);
-	QCOMPARE(hashstring(pic1->filename), PIC1_HASH);
-	QCOMPARE(hashstring(pic2->filename), PIC2_HASH);
 	QCOMPARE(localFilePath(pic1->filename), QString(PIC1_NAME));
 	QCOMPARE(localFilePath(pic2->filename), QString(PIC2_NAME));
 }