From 9e3e0a5a05504ba67c571b1fd438a666ae0e3f30 Mon Sep 17 00:00:00 2001 From: Berthold Stoeger Date: Thu, 30 May 2024 15:00:28 +0200 Subject: [PATCH] core: turn picture-table into std::vector<> Signed-off-by: Berthold Stoeger --- Subsurface-mobile.pro | 2 - backend-shared/exportfuncs.cpp | 6 +- commands/command.h | 4 +- commands/command_pictures.cpp | 32 ++++---- core/CMakeLists.txt | 2 - core/dive.cpp | 33 ++------ core/dive.h | 2 +- core/load-git.cpp | 6 +- core/parse-xml.cpp | 2 +- core/parse.cpp | 6 +- core/parse.h | 2 +- core/picture.cpp | 97 +++++++---------------- core/picture.h | 42 ++++------ core/pictureobj.cpp | 27 ------- core/pictureobj.h | 25 ------ core/qthelper.cpp | 20 +---- core/qthelper.h | 5 +- core/save-git.cpp | 13 ++- core/save-html.cpp | 16 +++- core/save-xml.cpp | 12 +-- core/subsurface-qt/divelistnotifier.h | 3 +- desktop-widgets/divelistview.cpp | 11 +-- desktop-widgets/findmovedimagesdialog.cpp | 4 +- profile-widget/profilewidget2.cpp | 32 +++++--- profile-widget/profilewidget2.h | 7 +- qt-models/divepicturemodel.cpp | 15 +--- qt-models/divepicturemodel.h | 6 +- qt-models/divetripmodel.cpp | 6 +- tests/testpicture.cpp | 48 ++++++----- 29 files changed, 170 insertions(+), 316 deletions(-) delete mode 100644 core/pictureobj.cpp delete mode 100644 core/pictureobj.h diff --git a/Subsurface-mobile.pro b/Subsurface-mobile.pro index 3d2288476..273d466f8 100644 --- a/Subsurface-mobile.pro +++ b/Subsurface-mobile.pro @@ -66,7 +66,6 @@ SOURCES += subsurface-mobile-main.cpp \ core/parse-xml.cpp \ core/parse.cpp \ core/picture.cpp \ - core/pictureobj.cpp \ core/sample.cpp \ core/import-suunto.cpp \ core/import-shearwater.cpp \ @@ -217,7 +216,6 @@ HEADERS += \ core/units.h \ core/version.h \ core/picture.h \ - core/pictureobj.h \ core/planner.h \ core/divesite.h \ core/checkcloudconnection.h \ diff --git a/backend-shared/exportfuncs.cpp b/backend-shared/exportfuncs.cpp index f5b36e613..0383a56cc 100644 --- a/backend-shared/exportfuncs.cpp +++ b/backend-shared/exportfuncs.cpp @@ -280,14 +280,14 @@ void export_depths(const char *filename, bool selected_only) if (selected_only && !dive->selected) continue; - FOR_EACH_PICTURE (dive) { + for (auto &picture: dive->pictures) { depth_t depth; for (auto &s: dive->dcs[0].samples) { - if ((int32_t)s.time.seconds > picture->offset.seconds) + if ((int32_t)s.time.seconds > picture.offset.seconds) break; depth = s.depth; } - put_format(&buf, "%s\t%.1f", picture->filename, get_depth_units(depth.mm, NULL, &unit)); + put_format(&buf, "%s\t%.1f", picture.filename.c_str(), get_depth_units(depth.mm, NULL, &unit)); put_format(&buf, "%s\n", unit); } } diff --git a/commands/command.h b/commands/command.h index ec41dc652..4eeb7c553 100644 --- a/commands/command.h +++ b/commands/command.h @@ -4,7 +4,7 @@ #include "core/divelog.h" #include "core/equipment.h" -#include "core/pictureobj.h" +#include "core/picture.h" #include "core/taxonomy.h" #include #include @@ -144,7 +144,7 @@ struct PictureListForDeletion { }; struct PictureListForAddition { dive *d; - std::vector pics; + std::vector pics; }; void setPictureOffset(dive *d, const QString &filename, offset_t offset); void removePictures(const std::vector &pictures); diff --git a/commands/command_pictures.cpp b/commands/command_pictures.cpp index 0a0e927bc..78067e6b7 100644 --- a/commands/command_pictures.cpp +++ b/commands/command_pictures.cpp @@ -2,15 +2,16 @@ #include "command_pictures.h" #include "core/errorhelper.h" +#include "core/range.h" #include "core/subsurface-qt/divelistnotifier.h" #include "qt-models/divelocationmodel.h" namespace Command { -static picture *dive_get_picture(const dive *d, const QString &fn) +static picture *dive_get_picture(dive *d, const QString &fn) { - int idx = get_picture_idx(&d->pictures, qPrintable(fn)); - return idx < 0 ? nullptr : &d->pictures.pictures[idx]; + int idx = get_picture_idx(d->pictures, fn.toStdString()); + return idx < 0 ? nullptr : &d->pictures[idx]; } SetPictureOffset::SetPictureOffset(dive *dIn, const QString &filenameIn, offset_t offsetIn) : @@ -33,7 +34,7 @@ void SetPictureOffset::redo() // Instead of trying to be smart, let's simply resort the picture table. // If someone complains about speed, do our usual "smart" thing. - sort_picture_table(&d->pictures); + std::sort(d->pictures.begin(), d->pictures.end()); emit diveListNotifier.pictureOffsetChanged(d, filename, newOffset); invalidate_dive_cache(d); } @@ -55,10 +56,9 @@ static PictureListForDeletion filterPictureListForDeletion(const PictureListForD PictureListForDeletion res; res.d = p.d; res.filenames.reserve(p.filenames.size()); - for (int i = 0; i < p.d->pictures.nr; ++i) { - std::string fn = p.d->pictures.pictures[i].filename; - if (std::find(p.filenames.begin(), p.filenames.end(), fn) != p.filenames.end()) - res.filenames.push_back(fn); + for (auto &pic: p.d->pictures) { + if (range_contains(p.filenames, pic.filename)) + res.filenames.push_back(pic.filename); } return res; } @@ -72,14 +72,14 @@ static std::vector removePictures(std::vectorpictures, fn.c_str()); + int idx = get_picture_idx(list.d->pictures, fn); if (idx < 0) { report_info("removePictures(): picture disappeared!"); continue; // Huh? We made sure that this can't happen by filtering out non-existent pictures. } filenames.push_back(QString::fromStdString(fn)); - toAdd.pics.emplace_back(list.d->pictures.pictures[idx]); - remove_from_picture_table(&list.d->pictures, idx); + toAdd.pics.emplace_back(list.d->pictures[idx]); + list.d->pictures.erase(list.d->pictures.begin() + idx); } if (!toAdd.pics.empty()) res.push_back(toAdd); @@ -98,17 +98,17 @@ static std::vector addPictures(std::vector res; for (const PictureListForAddition &list: picturesToAdd) { - QVector picsForSignal; + QVector picsForSignal; PictureListForDeletion toRemove; toRemove.d = list.d; - for (const PictureObj &pic: list.pics) { - int idx = get_picture_idx(&list.d->pictures, pic.filename.c_str()); // This should *not* already exist! + for (const picture &pic: list.pics) { + int idx = get_picture_idx(list.d->pictures, pic.filename); // This should *not* already exist! if (idx >= 0) { report_info("addPictures(): picture disappeared!"); continue; // Huh? We made sure that this can't happen by filtering out existing pictures. } picsForSignal.push_back(pic); - add_picture(&list.d->pictures, pic.toCore()); + add_picture(list.d->pictures, pic); toRemove.filenames.push_back(pic.filename); } if (!toRemove.filenames.empty()) @@ -164,7 +164,7 @@ AddPictures::AddPictures(const std::vector &pictures) : std::sort(p.pics.begin(), p.pics.end()); // Find a picture with a location - auto it = std::find_if(p.pics.begin(), p.pics.end(), [](const PictureObj &p) { return has_location(&p.location); }); + auto it = std::find_if(p.pics.begin(), p.pics.end(), [](const picture &p) { return has_location(&p.location); }); if (it != p.pics.end()) { // There is a dive with a location, we might want to modify the dive accordingly. struct dive_site *ds = p.d->dive_site; diff --git a/core/CMakeLists.txt b/core/CMakeLists.txt index b6459ef3e..9e2025118 100644 --- a/core/CMakeLists.txt +++ b/core/CMakeLists.txt @@ -138,8 +138,6 @@ set(SUBSURFACE_CORE_LIB_SRCS parse.h picture.cpp picture.h - pictureobj.cpp - pictureobj.h planner.cpp planner.h plannernotes.cpp diff --git a/core/dive.cpp b/core/dive.cpp index 5174a1f43..95a978247 100644 --- a/core/dive.cpp +++ b/core/dive.cpp @@ -49,10 +49,9 @@ dive::dive() : dcs(1) id = dive_getUniqID(); } -static void free_dive_structures(struct dive *d); dive::~dive() { - free_dive_structures(this); + fulltext_unregister(this); // TODO: this is a layering violation. Remove. } dive::dive(dive &&) = default; @@ -163,24 +162,6 @@ static void copy_dc_renumber(struct dive *d, const struct dive *s, const int cyl } } -static void free_dive_structures(struct dive *d) -{ - if (!d) - return; - fulltext_unregister(d); - /* free the strings */ - d->buddy.clear(); - d->diveguide.clear(); - d->notes.clear(); - d->suit.clear(); - /* free tags, additional dive computers, and pictures */ - d->tags.clear(); - d->cylinders.clear(); - d->weightsystems.clear(); - clear_picture_table(&d->pictures); - free(d->pictures.pictures); -} - /* copy_dive makes duplicates of many components of a dive; * in order not to leak memory, we need to free those. * copy_dive doesn't play with the divetrip and forward/backward pointers @@ -189,7 +170,7 @@ void clear_dive(struct dive *d) { if (!d) return; - free_dive_structures(d); + fulltext_unregister(d); *d = dive(); } @@ -198,15 +179,11 @@ void clear_dive(struct dive *d) * any impact on the source */ void copy_dive(const struct dive *s, struct dive *d) { - clear_dive(d); - /* simply copy things over, but then make actual copies of the - * relevant components that are referenced through pointers, - * so all the strings and the structured lists */ + /* simply copy things over, but then clear fulltext cache and dive cache. */ + fulltext_unregister(d); *d = *s; - memset(&d->pictures, 0, sizeof(d->pictures)); d->full_text = NULL; invalidate_dive_cache(d); - copy_pictures(&s->pictures, &d->pictures); } static void copy_dive_onedc(const struct dive *s, const struct divecomputer &sdc, struct dive *d) @@ -2336,7 +2313,7 @@ struct dive *merge_dives(const struct dive *a, const struct dive *b, int offset, MERGE_NONZERO(res, a, b, current); MERGE_NONZERO(res, a, b, surge); MERGE_NONZERO(res, a, b, chill); - copy_pictures(a->pictures.nr ? &a->pictures : &b->pictures, &res->pictures); + res->pictures = !a->pictures.empty() ? a->pictures : b->pictures; res->tags = taglist_merge(a->tags, b->tags); /* if we get dives without any gas / cylinder information in an import, make sure * that there is at leatst one entry in the cylinder map for that dive */ diff --git a/core/dive.h b/core/dive.h index e85bbeedd..d268271ef 100644 --- a/core/dive.h +++ b/core/dive.h @@ -50,7 +50,7 @@ struct dive { tag_list tags; std::vector dcs; // Attn: pointers to divecomputers are not stable! int id = 0; // unique ID for this dive - struct picture_table pictures = { }; + picture_table pictures; unsigned char git_id[20] = {}; bool notrip = false; /* Don't autogroup this dive to a trip */ bool selected = false; diff --git a/core/load-git.cpp b/core/load-git.cpp index e4cfcbd46..79c9bbca5 100644 --- a/core/load-git.cpp +++ b/core/load-git.cpp @@ -46,7 +46,7 @@ struct git_parser_state { std::string filter_constraint_range_mode; bool filter_constraint_negate = false; std::string filter_constraint_data; - struct picture active_pic = { 0 }; + struct picture active_pic; struct dive_site *active_site = nullptr; std::unique_ptr active_filter; struct divelog *log = nullptr; @@ -1048,7 +1048,7 @@ static void parse_settings_fingerprint(char *line, struct git_parser_state *stat static void parse_picture_filename(char *, struct git_parser_state *state) { - state->active_pic.filename = get_first_converted_string_c(state); + state->active_pic.filename = get_first_converted_string(state); } static void parse_picture_gps(char *line, struct git_parser_state *state) @@ -1756,7 +1756,7 @@ static int parse_picture_entry(struct git_parser_state *state, const git_tree_en state->active_pic.offset.seconds = offset; for_each_line(blob, picture_parser, state); - add_picture(&state->active_dive->pictures, state->active_pic); + add_picture(state->active_dive->pictures, std::move(state->active_pic)); git_blob_free(blob); /* add_picture took ownership of the data - diff --git a/core/parse-xml.cpp b/core/parse-xml.cpp index 691715d5a..78ae13f2a 100644 --- a/core/parse-xml.cpp +++ b/core/parse-xml.cpp @@ -1271,7 +1271,7 @@ static void try_to_fill_dive(struct dive *dive, const char *name, char *buf, str if (match_dc_data_fields(&dive->dcs[0], name, buf, state)) return; - if (MATCH("filename.picture", utf8_string, &state->cur_picture.filename)) + if (MATCH("filename.picture", utf8_string_std, &state->cur_picture.filename)) return; if (MATCH("offset.picture", offsettime, &state->cur_picture.offset)) return; diff --git a/core/parse.cpp b/core/parse.cpp index 6c9002cec..ba3c44784 100644 --- a/core/parse.cpp +++ b/core/parse.cpp @@ -70,10 +70,10 @@ void event_end(struct parser_state *state) struct divecomputer *dc = get_dc(state); if (state->cur_event.type == 123) { struct picture pic; - pic.filename = strdup(state->cur_event.name.c_str()); + pic.filename = state->cur_event.name; /* theoretically this could fail - but we didn't support multi year offsets */ pic.offset.seconds = state->cur_event.time.seconds; - add_picture(&state->cur_dive->pictures, pic); /* Takes ownership. */ + add_picture(state->cur_dive->pictures, std::move(pic)); } else { /* At some point gas change events did not have any type. Thus we need to add * one on import, if we encounter the type one missing. @@ -307,7 +307,7 @@ void picture_start(struct parser_state *state) void picture_end(struct parser_state *state) { - add_picture(&state->cur_dive->pictures, state->cur_picture); + add_picture(state->cur_dive->pictures, std::move(state->cur_picture)); /* dive_add_picture took ownership, we can just clear out copy of the data */ state->cur_picture = picture(); } diff --git a/core/parse.h b/core/parse.h index a2d5a1622..5a69b9b91 100644 --- a/core/parse.h +++ b/core/parse.h @@ -59,7 +59,7 @@ struct parser_state { location_t cur_location; struct dive_trip *cur_trip = nullptr; /* owning */ struct sample *cur_sample = nullptr; /* non-owning */ - struct picture cur_picture { 0 }; /* owning */ + struct picture cur_picture; /* owning */ std::unique_ptr cur_filter; /* owning */ std::string fulltext; /* owning */ std::string fulltext_string_mode; /* owning */ diff --git a/core/picture.cpp b/core/picture.cpp index 3de5f1b35..527dcf52d 100644 --- a/core/picture.cpp +++ b/core/picture.cpp @@ -4,71 +4,28 @@ #if !defined(SUBSURFACE_MOBILE) #include "metadata.h" #endif +#include "range.h" #include "subsurface-string.h" -#include "table.h" #include #include -static void free_picture(struct picture picture) +bool picture::operator<(const struct picture &p) const { - free(picture.filename); - picture.filename = NULL; + return std::tie(offset.seconds, filename) < + std::tie(p.offset.seconds, p.filename); } -static int comp_pictures(struct picture a, struct picture b) +void add_picture(picture_table &t, struct picture newpic) { - if (a.offset.seconds < b.offset.seconds) - return -1; - if (a.offset.seconds > b.offset.seconds) - return 1; - return strcmp(a.filename ?: "", b.filename ?: ""); + auto it = std::lower_bound(t.begin(), t.end(), newpic); + t.insert(it, std::move(newpic)); } -static bool picture_less_than(struct picture a, struct picture b) +int get_picture_idx(const picture_table &t, const std::string &filename) { - return comp_pictures(a, b) < 0; -} -/* picture table functions */ -//static MAKE_GET_IDX(picture_table, struct picture, pictures) -static MAKE_GROW_TABLE(picture_table, struct picture, pictures) -static MAKE_GET_INSERTION_INDEX(picture_table, struct picture, pictures, picture_less_than) -MAKE_ADD_TO(picture_table, struct picture, pictures) -MAKE_REMOVE_FROM(picture_table, pictures) -MAKE_SORT(picture_table, struct picture, pictures, comp_pictures) -//MAKE_REMOVE(picture_table, struct picture, picture) -MAKE_CLEAR_TABLE(picture_table, pictures, picture) - -/* Add a clone of a picture to the end of a picture table. - * Cloned means that the filename-string is copied. */ -static void add_cloned_picture(struct picture_table *t, struct picture pic) -{ - pic.filename = copy_string(pic.filename); - int idx = picture_table_get_insertion_index(t, pic); - add_to_picture_table(t, idx, pic); -} - -void copy_pictures(const struct picture_table *s, struct picture_table *d) -{ - int i; - clear_picture_table(d); - for (i = 0; i < s->nr; i++) - add_cloned_picture(d, s->pictures[i]); -} - -void add_picture(struct picture_table *t, struct picture newpic) -{ - int idx = picture_table_get_insertion_index(t, newpic); - add_to_picture_table(t, idx, newpic); -} - -int get_picture_idx(const struct picture_table *t, const char *filename) -{ - for (int i = 0; i < t->nr; ++i) { - if (same_string(t->pictures[i].filename, filename)) - return i; - } - return -1; + return index_of_if(t, [&filename] (const picture &p) + { return p.filename == filename; }); } #if !defined(SUBSURFACE_MOBILE) @@ -113,37 +70,37 @@ static struct dive *nearest_selected_dive(timestamp_t timestamp) // only add pictures that have timestamps between 30 minutes before the dive and // 30 minutes after the dive ends -#define D30MIN (30 * 60) +static constexpr timestamp_t d30min = 30 * 60; static bool dive_check_picture_time(const struct dive *d, timestamp_t timestamp) { - return time_from_dive(d, timestamp) < D30MIN; + return time_from_dive(d, timestamp) < d30min; } /* Creates a picture and indicates the dive to which this picture should be added. * The caller is responsible for actually adding the picture to the dive. - * If no appropriate dive was found, no picture is created and NULL is returned. + * If no appropriate dive was found, no picture is created and null is returned. */ -struct picture *create_picture(const char *filename, timestamp_t shift_time, bool match_all, struct dive **dive) +std::pair, dive *> create_picture(const std::string &filename, timestamp_t shift_time, bool match_all) { struct metadata metadata; timestamp_t timestamp; - get_metadata(filename, &metadata); + get_metadata(filename.c_str(), &metadata); timestamp = metadata.timestamp + shift_time; - *dive = nearest_selected_dive(timestamp); + struct dive *dive = nearest_selected_dive(timestamp); - if (!*dive) - return NULL; - if (get_picture_idx(&(*dive)->pictures, filename) >= 0) - return NULL; - if (!match_all && !dive_check_picture_time(*dive, timestamp)) - return NULL; + if (!dive) + return { {}, nullptr }; + if (get_picture_idx(dive->pictures, filename) >= 0) + return { {}, nullptr }; + if (!match_all && !dive_check_picture_time(dive, timestamp)) + return { {}, nullptr }; - struct picture *picture = (struct picture *)malloc(sizeof(struct picture)); - picture->filename = strdup(filename); - picture->offset.seconds = metadata.timestamp - (*dive)->when + shift_time; - picture->location = metadata.location; - return picture; + struct picture picture; + picture.filename = filename; + picture.offset.seconds = metadata.timestamp - dive->when + shift_time; + picture.location = metadata.location; + return { picture, dive }; } bool picture_check_valid_time(timestamp_t timestamp, timestamp_t shift_time) diff --git a/core/picture.h b/core/picture.h index 58248c6c4..de584af7f 100644 --- a/core/picture.h +++ b/core/picture.h @@ -1,47 +1,35 @@ // SPDX-License-Identifier: GPL-2.0 +// picture (more precisely media) related strutures and functions #ifndef PICTURE_H #define PICTURE_H -// picture (more precisely media) related strutures and functions #include "units.h" -#include // For NULL +#include +#include +#include struct dive; struct picture { - char *filename = nullptr; + std::string filename; offset_t offset; location_t location; + bool operator<(const picture &) const; }; -/* loop through all pictures of a dive */ -#define FOR_EACH_PICTURE(_dive) \ - if ((_dive) && (_dive)->pictures.nr) \ - for (struct picture *picture = (_dive)->pictures.pictures; \ - picture < (_dive)->pictures.pictures + (_dive)->pictures.nr; \ - picture++) - /* Table of pictures. Attention: this stores pictures, - * *not* pointers to pictures. This has two crucial consequences: - * 1) Pointers to pictures are not stable. They may be - * invalidated if the table is reallocated. - * 2) add_to_picture_table(), etc. take ownership of the - * picture. Notably of the filename. */ -struct picture_table { - int nr, allocated; - struct picture *pictures; -}; + * *not* pointers to pictures. This means that + * pointers to pictures are not stable. They are + * invalidated if the table is reallocated. + */ +using picture_table = std::vector; /* picture table functions */ -extern void clear_picture_table(struct picture_table *); -extern void add_to_picture_table(struct picture_table *, int idx, struct picture pic); -extern void copy_pictures(const struct picture_table *s, struct picture_table *d); -extern void add_picture(struct picture_table *, struct picture newpic); -extern void remove_from_picture_table(struct picture_table *, int idx); -extern int get_picture_idx(const struct picture_table *, const char *filename); /* Return -1 if not found */ -extern void sort_picture_table(struct picture_table *); +extern void add_to_picture_table(picture_table &, int idx, struct picture pic); +extern void add_picture(picture_table &, struct picture newpic); +extern int get_picture_idx(const picture_table &, const std::string &filename); /* Return -1 if not found */ -extern struct picture *create_picture(const char *filename, timestamp_t shift_time, bool match_all, struct dive **dive); +extern std::pair, dive *> create_picture(const std::string &filename, timestamp_t shift_time, bool match_all); extern bool picture_check_valid_time(timestamp_t timestamp, timestamp_t shift_time); #endif // PICTURE_H diff --git a/core/pictureobj.cpp b/core/pictureobj.cpp deleted file mode 100644 index cb6b32675..000000000 --- a/core/pictureobj.cpp +++ /dev/null @@ -1,27 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#include "pictureobj.h" -#include "qthelper.h" - -PictureObj::PictureObj() : offset({ 0 }), location({ 0 }) -{ -} - -PictureObj::PictureObj(const picture &pic) : filename(pic.filename), offset(pic.offset), location(pic.location) -{ -} - -picture PictureObj::toCore() const -{ - return picture { - strdup(filename.c_str()), - offset, - location - }; -} - -bool PictureObj::operator<(const PictureObj &p2) const -{ - if (offset.seconds != p2.offset.seconds) - return offset.seconds < p2.offset.seconds; - return strcmp(filename.c_str(), p2.filename.c_str()) < 0; -} diff --git a/core/pictureobj.h b/core/pictureobj.h deleted file mode 100644 index 6ea20259b..000000000 --- a/core/pictureobj.h +++ /dev/null @@ -1,25 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 - -#ifndef PICTUREOBJ_H -#define PICTUREOBJ_H - -// A tiny helper class that represents a struct picture of the core -// It does, however, keep the filename as a std::string so that C++ code -// doesn't have do its own memory-management. - -#include "core/units.h" -#include "core/picture.h" -#include - -struct PictureObj { - std::string filename; - offset_t offset; - location_t location; - - PictureObj(); // Initialize to empty picture. - PictureObj(const picture &pic); // Create from core struct picture. - picture toCore() const; // Turn into core structure. Caller responsible for freeing. - bool operator<(const PictureObj &p2) const; -}; - -#endif // PICTUREOBJ_H diff --git a/core/qthelper.cpp b/core/qthelper.cpp index f0582d01a..d89c40d48 100644 --- a/core/qthelper.cpp +++ b/core/qthelper.cpp @@ -333,24 +333,12 @@ std::string move_away(const std::string &old_path) return newPath; } -std::string get_file_name(const char *fileName) +std::string get_file_name(const std::string &fileName) { - QFileInfo fileInfo(fileName); + QFileInfo fileInfo(fileName.c_str()); return fileInfo.fileName().toStdString(); } -void copy_image_and_overwrite(const char *cfileName, const char *path, const char *cnewName) -{ - QString fileName(cfileName); - QString newName(path); - newName += cnewName; - QFile file(newName); - if (file.exists()) - file.remove(); - if (!QFile::copy(fileName, newName)) - report_info("copy of %s to %s failed", cfileName, qPrintable(newName)); -} - static bool lessThan(const QPair &a, const QPair &b) { return a.second < b.second; @@ -1174,9 +1162,9 @@ QStringList videoExtensionFilters() return filters; } -const char *local_file_path(struct picture *picture) +std::string local_file_path(const struct picture &picture) { - return copy_qstring(localFilePath(picture->filename)); + return localFilePath(QString::fromStdString(picture.filename)).toStdString(); } QString get_gas_string(struct gasmix gas) diff --git a/core/qthelper.h b/core/qthelper.h index 93e37d32c..f6d6b5a65 100644 --- a/core/qthelper.h +++ b/core/qthelper.h @@ -98,7 +98,7 @@ extern QString (*changesCallback)(); void uiNotification(const QString &msg); std::string get_changes_made(); std::string subsurface_user_agent(); -std::string get_file_name(const char *fileName); +std::string get_file_name(const std::string &fileName); std::string move_away(const std::string &path); #if defined __APPLE__ @@ -110,8 +110,7 @@ std::string move_away(const std::string &path); bool canReachCloudServer(struct git_info *); void updateWindowTitle(); void subsurface_mkdir(const char *dir); -void copy_image_and_overwrite(const char *cfileName, const char *path, const char *cnewName); -const char *local_file_path(struct picture *picture); +std::string local_file_path(const struct picture &picture); char *hashfile_name_string(); enum deco_mode decoMode(bool in_planner); void parse_seabear_header(const char *filename, struct xml_params *params); diff --git a/core/save-git.cpp b/core/save-git.cpp index 402cee1f2..49c312651 100644 --- a/core/save-git.cpp +++ b/core/save-git.cpp @@ -612,15 +612,15 @@ static int save_one_divecomputer(git_repository *repo, struct dir *tree, struct return ret; } -static int save_one_picture(git_repository *repo, struct dir *dir, struct picture *pic) +static int save_one_picture(git_repository *repo, struct dir *dir, const struct picture &pic) { - int offset = pic->offset.seconds; + int offset = pic.offset.seconds; membuffer buf; char sign = '+'; unsigned h; - show_utf8(&buf, "filename ", pic->filename, "\n"); - put_location(&buf, &pic->location, "gps ", "\n"); + show_utf8(&buf, "filename ", pic.filename.c_str(), "\n"); + put_location(&buf, &pic.location, "gps ", "\n"); /* Picture loading will load even negative offsets.. */ if (offset < 0) { @@ -637,11 +637,10 @@ static int save_one_picture(git_repository *repo, struct dir *dir, struct pictur static int save_pictures(git_repository *repo, struct dir *dir, struct dive *dive) { - if (dive->pictures.nr > 0) { + if (!dive->pictures.empty()) { dir = mktree(repo, dir, "Pictures"); - FOR_EACH_PICTURE(dive) { + for (auto &picture: dive->pictures) save_one_picture(repo, dir, picture); - } } return 0; } diff --git a/core/save-html.cpp b/core/save-html.cpp index d7f7212a9..873c61055 100644 --- a/core/save-html.cpp +++ b/core/save-html.cpp @@ -21,6 +21,7 @@ #include #include +#include static void write_attribute(struct membuffer *b, const char *att_name, const char *value, const char *separator) { @@ -31,13 +32,24 @@ static void write_attribute(struct membuffer *b, const char *att_name, const cha put_format(b, "\"%s", separator); } +static void copy_image_and_overwrite(const std::string &cfileName, const std::string &path, const std::string &cnewName) +{ + QString fileName = QString::fromStdString(cfileName); + std::string newName = path + cnewName; + QFile file(QString::fromStdString(newName)); + if (file.exists()) + file.remove(); + if (!QFile::copy(fileName, QString::fromStdString(newName))) + report_info("copy of %s to %s failed", cfileName.c_str(), newName.c_str()); +} + static void save_photos(struct membuffer *b, const char *photos_dir, const struct dive *dive) { - if (dive->pictures.nr <= 0) + if (dive->pictures.empty()) return; const char *separator = "\"photos\":["; - FOR_EACH_PICTURE(dive) { + for (auto &picture: dive->pictures) { put_string(b, separator); separator = ", "; std::string fname = get_file_name(local_file_path(picture)); diff --git a/core/save-xml.cpp b/core/save-xml.cpp index b72238946..31ef59e27 100644 --- a/core/save-xml.cpp +++ b/core/save-xml.cpp @@ -456,13 +456,13 @@ static void save_dc(struct membuffer *b, struct dive *dive, struct divecomputer put_format(b, " \n"); } -static void save_picture(struct membuffer *b, struct picture *pic) +static void save_picture(struct membuffer *b, const struct picture &pic) { put_string(b, " offset.seconds) { - int offset = pic->offset.seconds; + if (pic.offset.seconds) { + int offset = pic.offset.seconds; char sign = '+'; if (offset < 0) { sign = '-'; @@ -470,7 +470,7 @@ static void save_picture(struct membuffer *b, struct picture *pic) } put_format(b, " offset='%c%u:%02u min'", sign, FRACTION_TUPLE(offset, 60)); } - put_location(b, &pic->location, " gps='","'"); + put_location(b, &pic.location, " gps='","'"); put_string(b, "/>\n"); } @@ -528,7 +528,7 @@ void save_one_dive_to_mb(struct membuffer *b, struct dive *dive, bool anonymize) /* Save the dive computer data */ for (auto &dc: dive->dcs) save_dc(b, dive, &dc); - FOR_EACH_PICTURE(dive) + for (auto &picture: dive->pictures) save_picture(b, picture); put_format(b, "\n"); } diff --git a/core/subsurface-qt/divelistnotifier.h b/core/subsurface-qt/divelistnotifier.h index 98339e339..53ea260e5 100644 --- a/core/subsurface-qt/divelistnotifier.h +++ b/core/subsurface-qt/divelistnotifier.h @@ -6,7 +6,6 @@ #define DIVELISTNOTIFIER_H #include "core/dive.h" -#include "core/pictureobj.h" #include @@ -133,7 +132,7 @@ signals: // Picture (media) related signals void pictureOffsetChanged(dive *d, QString filename, offset_t offset); void picturesRemoved(dive *d, QVector filenames); - void picturesAdded(dive *d, QVector pics); + void picturesAdded(dive *d, QVector pics); // Devices related signals void deviceEdited(); diff --git a/desktop-widgets/divelistview.cpp b/desktop-widgets/divelistview.cpp index 978c86e95..376177830 100644 --- a/desktop-widgets/divelistview.cpp +++ b/desktop-widgets/divelistview.cpp @@ -835,18 +835,15 @@ void DiveListView::matchImagesToDives(const QStringList &fileNames) // Create the data structure of pictures to be added: a list of pictures per dive. std::vector pics; for (const QString &fileName: fileNames) { - struct dive *d; - picture *pic = create_picture(qPrintable(fileName), shiftDialog.amount(), shiftDialog.matchAll(), &d); + auto [pic, d] = create_picture(fileName.toStdString(), shiftDialog.amount(), shiftDialog.matchAll()); if (!pic) continue; - PictureObj pObj(*pic); - free(pic); - auto it = std::find_if(pics.begin(), pics.end(), [d](const Command::PictureListForAddition &l) { return l.d == d; }); + auto it = std::find_if(pics.begin(), pics.end(), [dive=d](const Command::PictureListForAddition &l) { return l.d == dive; }); if (it == pics.end()) - pics.push_back(Command::PictureListForAddition { d, { std::move(pObj) } }); + pics.push_back(Command::PictureListForAddition { d, { std::move(*pic) } }); else - it->pics.push_back(std::move(pObj)); + it->pics.push_back(std::move(*pic)); } if (pics.empty()) diff --git a/desktop-widgets/findmovedimagesdialog.cpp b/desktop-widgets/findmovedimagesdialog.cpp index a68291b14..0d4b75997 100644 --- a/desktop-widgets/findmovedimagesdialog.cpp +++ b/desktop-widgets/findmovedimagesdialog.cpp @@ -188,8 +188,8 @@ void FindMovedImagesDialog::on_scanButton_clicked() struct dive *dive; for_each_dive (i, dive) if (!onlySelected || dive->selected) - FOR_EACH_PICTURE(dive) - imagePaths.append(QString(picture->filename)); + for (auto &picture: dive->pictures) + imagePaths.append(QString::fromStdString(picture.filename)); stopScanning = 0; QFuture> future = QtConcurrent::run( // Note that we capture everything but "this" by copy to avoid dangling references. diff --git a/profile-widget/profilewidget2.cpp b/profile-widget/profilewidget2.cpp index ce235a27b..f8a9159dd 100644 --- a/profile-widget/profilewidget2.cpp +++ b/profile-widget/profilewidget2.cpp @@ -1077,8 +1077,10 @@ void ProfileWidget2::updateDurationLine(PictureEntry &e) // This function is called asynchronously by the thumbnailer if a thumbnail // was fetched from disk or freshly calculated. -void ProfileWidget2::updateThumbnail(QString filename, QImage thumbnail, duration_t duration) +void ProfileWidget2::updateThumbnail(QString filenameIn, QImage thumbnail, duration_t duration) { + std::string filename = filenameIn.toStdString(); + // Find the picture with the given filename auto it = std::find_if(pictures.begin(), pictures.end(), [&filename](const PictureEntry &e) { return e.filename == filename; }); @@ -1101,7 +1103,7 @@ void ProfileWidget2::updateThumbnail(QString filename, QImage thumbnail, duratio } // Create a PictureEntry object and add its thumbnail to the scene if profile pictures are shown. -ProfileWidget2::PictureEntry::PictureEntry(offset_t offsetIn, const QString &filenameIn, ProfileWidget2 *profile, bool synchronous) : offset(offsetIn), +ProfileWidget2::PictureEntry::PictureEntry(offset_t offsetIn, const std::string &filenameIn, ProfileWidget2 *profile, bool synchronous) : offset(offsetIn), duration(duration_t {0}), filename(filenameIn), thumbnail(new DivePictureItem) @@ -1110,9 +1112,9 @@ ProfileWidget2::PictureEntry::PictureEntry(offset_t offsetIn, const QString &fil int size = Thumbnailer::defaultThumbnailSize(); scene->addItem(thumbnail.get()); thumbnail->setVisible(prefs.show_pictures_in_profile); - QImage img = Thumbnailer::instance()->fetchThumbnail(filename, synchronous).scaled(size, size, Qt::KeepAspectRatio); + QImage img = Thumbnailer::instance()->fetchThumbnail(QString::fromStdString(filename), synchronous).scaled(size, size, Qt::KeepAspectRatio); thumbnail->setPixmap(QPixmap::fromImage(img)); - thumbnail->setFileUrl(filename); + thumbnail->setFileUrl(QString::fromStdString(filename)); connect(thumbnail.get(), &DivePictureItem::removePicture, profile, &ProfileWidget2::removePicture); } @@ -1205,13 +1207,15 @@ void ProfileWidget2::plotPicturesInternal(const struct dive *d, bool synchronous if (currentState == EDIT || currentState == PLAN) return; + if (!d) + return; + // Fetch all pictures of the dive, but consider only those that are within the dive time. // For each picture, create a PictureEntry object in the pictures-vector. // emplace_back() constructs an object at the end of the vector. The parameters are passed directly to the constructor. - // Note that FOR_EACH_PICTURE handles d being null gracefully. - FOR_EACH_PICTURE(d) { - if (picture->offset.seconds > 0 && picture->offset.seconds <= d->duration.seconds) - pictures.emplace_back(picture->offset, QString(picture->filename), this, synchronous); + for (auto &picture: d->pictures) { + if (picture.offset.seconds > 0 && picture.offset.seconds <= d->duration.seconds) + pictures.emplace_back(picture.offset, picture.filename, this, synchronous); } if (pictures.empty()) return; @@ -1240,16 +1244,16 @@ void ProfileWidget2::picturesRemoved(dive *d, QVector fileUrls) // (c.f. erase-remove idiom: https://en.wikipedia.org/wiki/Erase%E2%80%93remove_idiom) auto it = std::remove_if(pictures.begin(), pictures.end(), [&fileUrls](const PictureEntry &e) // Check whether filename of entry is in list of provided filenames - { return std::find(fileUrls.begin(), fileUrls.end(), e.filename) != fileUrls.end(); }); + { return std::find(fileUrls.begin(), fileUrls.end(), QString::fromStdString(e.filename)) != fileUrls.end(); }); pictures.erase(it, pictures.end()); calculatePictureYPositions(); } -void ProfileWidget2::picturesAdded(dive *d, QVector pics) +void ProfileWidget2::picturesAdded(dive *d, QVector pics) { - for (const PictureObj &pic: pics) { + for (const picture &pic: pics) { if (pic.offset.seconds > 0 && pic.offset.seconds <= d->duration.seconds) { - pictures.emplace_back(pic.offset, QString::fromStdString(pic.filename), this, false); + pictures.emplace_back(pic.offset, pic.filename, this, false); updateThumbnailXPos(pictures.back()); } } @@ -1302,11 +1306,13 @@ void ProfileWidget2::dropEvent(QDropEvent *event) } #ifndef SUBSURFACE_MOBILE -void ProfileWidget2::pictureOffsetChanged(dive *dIn, QString filename, offset_t offset) +void ProfileWidget2::pictureOffsetChanged(dive *dIn, QString filenameIn, offset_t offset) { if (dIn != d) return; // Picture of a different dive than the one shown changed. + std::string filename = filenameIn.toStdString(); // TODO: can we move std::string through Qt's signal/slot system? + // Calculate time in dive where picture was dropped and whether the new position is during the dive. bool duringDive = d && offset.seconds > 0 && offset.seconds < d->duration.seconds; diff --git a/profile-widget/profilewidget2.h b/profile-widget/profilewidget2.h index 7b727b040..b8587d009 100644 --- a/profile-widget/profilewidget2.h +++ b/profile-widget/profilewidget2.h @@ -17,7 +17,6 @@ // * It needs to be dynamic, things should *flow* on it, not just appear / disappear. // */ #include "profile-widget/divelineitem.h" -#include "core/pictureobj.h" #include "core/units.h" #include "core/subsurface-qt/divelistnotifier.h" @@ -78,7 +77,7 @@ slots: // Necessary to call from QAction's signals. #ifndef SUBSURFACE_MOBILE void plotPictures(); void picturesRemoved(dive *d, QVector filenames); - void picturesAdded(dive *d, QVector pics); + void picturesAdded(dive *d, QVector pics); void pointsReset(); void pointInserted(const QModelIndex &parent, int start, int end); void pointsRemoved(const QModelIndex &, int start, int end); @@ -163,11 +162,11 @@ private: struct PictureEntry { offset_t offset; duration_t duration; - QString filename; + std::string filename; std::unique_ptr thumbnail; // For videos with known duration, we represent the duration of the video by a line std::unique_ptr durationLine; - PictureEntry (offset_t offsetIn, const QString &filenameIn, ProfileWidget2 *profile, bool synchronous); + PictureEntry (offset_t offsetIn, const std::string &filenameIn, ProfileWidget2 *profile, bool synchronous); bool operator< (const PictureEntry &e) const; }; void updateThumbnailXPos(PictureEntry &e); diff --git a/qt-models/divepicturemodel.cpp b/qt-models/divepicturemodel.cpp index 1d6c0f314..d49ef414c 100644 --- a/qt-models/divepicturemodel.cpp +++ b/qt-models/divepicturemodel.cpp @@ -13,13 +13,6 @@ #include #include -PictureEntry::PictureEntry(dive *dIn, const PictureObj &p) : d(dIn), - filename(p.filename), - offsetSeconds(p.offset.seconds), - length({ 0 }) -{ -} - PictureEntry::PictureEntry(dive *dIn, const picture &p) : d(dIn), filename(p.filename), offsetSeconds(p.offset.seconds), @@ -91,8 +84,8 @@ void DivePictureModel::updateDivePictures() for (struct dive *dive: getDiveSelection()) { size_t first = pictures.size(); - FOR_EACH_PICTURE(dive) - pictures.push_back(PictureEntry(dive, *picture)); + for (auto &picture: dive->pictures) + pictures.push_back(PictureEntry(dive, picture)); // Sort pictures of this dive by offset. // Thus, the list will be sorted by (dive, offset). @@ -197,7 +190,7 @@ void DivePictureModel::picturesRemoved(dive *d, QVector filenamesIn) } // Assumes that pics is sorted! -void DivePictureModel::picturesAdded(dive *d, QVector picsIn) +void DivePictureModel::picturesAdded(dive *d, QVector picsIn) { // We only display pictures of selected dives if (!d->selected || picsIn.empty()) @@ -206,7 +199,7 @@ void DivePictureModel::picturesAdded(dive *d, QVector picsIn) // Convert the picture-data into our own format std::vector pics; pics.reserve(picsIn.size()); - for (const PictureObj &pic: picsIn) + for (const picture &pic: picsIn) pics.push_back(PictureEntry(d, pic)); // Insert batch-wise to avoid too many reloads diff --git a/qt-models/divepicturemodel.h b/qt-models/divepicturemodel.h index d7b043d44..fa4ccaef8 100644 --- a/qt-models/divepicturemodel.h +++ b/qt-models/divepicturemodel.h @@ -2,8 +2,7 @@ #ifndef DIVEPICTUREMODEL_H #define DIVEPICTUREMODEL_H -#include "core/units.h" -#include "core/pictureobj.h" +#include "core/picture.h" #include #include @@ -17,7 +16,6 @@ struct PictureEntry { QImage image; int offsetSeconds; duration_t length; - PictureEntry(dive *, const PictureObj &); PictureEntry(dive *, const picture &); bool operator<(const PictureEntry &) const; }; @@ -36,7 +34,7 @@ public slots: void updateThumbnail(QString filename, QImage thumbnail, duration_t duration); void pictureOffsetChanged(dive *d, const QString filename, offset_t offset); void picturesRemoved(dive *d, QVector filenames); - void picturesAdded(dive *d, QVector pics); + void picturesAdded(dive *d, QVector pics); private: DivePictureModel(); std::vector pictures; diff --git a/qt-models/divetripmodel.cpp b/qt-models/divetripmodel.cpp index ce61af8ce..c94058c25 100644 --- a/qt-models/divetripmodel.cpp +++ b/qt-models/divetripmodel.cpp @@ -147,8 +147,8 @@ static int countPhotos(const struct dive *d) const int bufperiod = 120; // A 2-min buffer period. Photos within 2 min of dive are assumed as int diveTotaltime = dive_endtime(d) - d->when; // taken during the dive, not before/after. int pic_offset, icon_index = 0; - FOR_EACH_PICTURE (d) { // Step through each of the pictures for this dive: - pic_offset = picture->offset.seconds; + for (auto &picture: d->pictures) { // Step through each of the pictures for this dive: + pic_offset = picture.offset.seconds; if ((pic_offset < -bufperiod) | (pic_offset > diveTotaltime+bufperiod)) { icon_index |= 0x02; // If picture is before/after the dive // then set the appropriate bit ... @@ -378,7 +378,7 @@ QVariant DiveTripModelBase::diveData(const struct dive *d, int column, int role) case PHOTOS: // If there are photos, show one of the three photo icons: fish= photos during dive; // sun=photos before/after dive; sun+fish=photos during dive as well as before/after - if (d->pictures.nr > 0) + if (!d->pictures.empty()) return getPhotoIcon(countPhotos(d)); break; } diff --git a/tests/testpicture.cpp b/tests/testpicture.cpp index 3c92ce4c0..14e54977b 100644 --- a/tests/testpicture.cpp +++ b/tests/testpicture.cpp @@ -24,45 +24,43 @@ void TestPicture::initTestCase() void TestPicture::addPicture() { - struct dive *dive, *dive1, *dive2; - struct picture *pic1, *pic2; verbose = 1; QCOMPARE(parse_file(SUBSURFACE_TEST_DATA "/dives/test44.xml", &divelog), 0); - dive = get_dive(0); + struct dive *dive = get_dive(0); // Pictures will be added to selected dives dive->selected = true; QVERIFY(dive != NULL); // So far no picture in dive - QVERIFY(dive->pictures.nr == 0); + QVERIFY(dive->pictures.size() == 0); - pic1 = create_picture(SUBSURFACE_TEST_DATA "/dives/images/wreck.jpg", 0, false, &dive1); - pic2 = create_picture(SUBSURFACE_TEST_DATA "/dives/images/data_after_EOI.jpg", 0, false, &dive2); - QVERIFY(pic1 != NULL); - QVERIFY(pic2 != NULL); - QVERIFY(dive1 == dive); - QVERIFY(dive2 == dive); + { + auto [pic1, dive1] = create_picture(SUBSURFACE_TEST_DATA "/dives/images/wreck.jpg", 0, false); + auto [pic2, dive2] = create_picture(SUBSURFACE_TEST_DATA "/dives/images/data_after_EOI.jpg", 0, false); + QVERIFY(pic1); + QVERIFY(pic2); + QVERIFY(dive1 == dive); + QVERIFY(dive2 == dive); - add_picture(&dive->pictures, *pic1); - add_picture(&dive->pictures, *pic2); - free(pic1); - free(pic2); + add_picture(dive->pictures, std::move(*pic1)); + add_picture(dive->pictures, std::move(*pic2)); + } // Now there are two pictures - QVERIFY(dive->pictures.nr == 2); - pic1 = &dive->pictures.pictures[0]; - pic2 = &dive->pictures.pictures[1]; + QVERIFY(dive->pictures.size() == 2); + const picture &pic1 = dive->pictures[0]; + const picture &pic2 = dive->pictures[1]; // 1st appearing at time 21:01 // 2nd appearing at time 22:01 - QVERIFY(pic1->offset.seconds == 1261); - QVERIFY(pic1->location.lat.udeg == 47934500); - QVERIFY(pic1->location.lon.udeg == 11334500); - QVERIFY(pic2->offset.seconds == 1321); + QVERIFY(pic1.offset.seconds == 1261); + QVERIFY(pic1.location.lat.udeg == 47934500); + QVERIFY(pic1.location.lon.udeg == 11334500); + QVERIFY(pic2.offset.seconds == 1321); - learnPictureFilename(pic1->filename, PIC1_NAME); - learnPictureFilename(pic2->filename, PIC2_NAME); - QCOMPARE(localFilePath(pic1->filename), QString(PIC1_NAME)); - QCOMPARE(localFilePath(pic2->filename), QString(PIC2_NAME)); + learnPictureFilename(QString::fromStdString(pic1.filename), PIC1_NAME); + learnPictureFilename(QString::fromStdString(pic2.filename), PIC2_NAME); + QCOMPARE(localFilePath(QString::fromStdString(pic1.filename)), QString(PIC1_NAME)); + QCOMPARE(localFilePath(QString::fromStdString(pic2.filename)), QString(PIC2_NAME)); } QTEST_GUILESS_MAIN(TestPicture)