Dive media: transport dive-id in drag'n'drop events

9efb56e2d4 introduced rather complex
logic for picture drag'n'drop events onto the profile. Among other
things, the code had to check whether the picture actually belongs
to the displayed dive.

This can be simplified by transporting the dive-id in the drag'n'drop
event structure. The flow goes like this:
DivePictureModel--(1)-->DivePictureWidget--(2)-->ProfileWidget

For (1), we can use the Qt::UserRole role. This was used to transport
the picture-offset, but this is not needed anymore since ProfileWidget
was decoupled from DivePictureModel.

For (2), we simply replace the "position" value, which was never used.
Why would the receiver care which pixel was pressed in the media-tab?

This commit also contains a minor cleanup in DivePictureWidget:
QListView::mousePressEvent(event) was called in both branches of an
if and can therefore be removed from the if. This is so trivial,
that it doesn't warrant its own commit.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2018-07-14 17:40:24 +02:00 committed by Dirk Hohndel
parent 8ff7314b21
commit c5f66c5538
3 changed files with 29 additions and 34 deletions

View file

@ -32,7 +32,9 @@ void DivePictureWidget::mouseDoubleClickEvent(QMouseEvent *event)
void DivePictureWidget::mousePressEvent(QMouseEvent *event) void DivePictureWidget::mousePressEvent(QMouseEvent *event)
{ {
if (event->button() == Qt::LeftButton && event->modifiers() == Qt::NoModifier) { if (event->button() == Qt::LeftButton && event->modifiers() == Qt::NoModifier) {
QString filename = model()->data(indexAt(event->pos()), Qt::DisplayPropertyRole).toString(); QModelIndex index = indexAt(event->pos());
QString filename = model()->data(index, Qt::DisplayPropertyRole).toString();
int diveId = model()->data(index, Qt::UserRole).toInt();
if (!filename.isEmpty()) { if (!filename.isEmpty()) {
int dim = lrint(defaultIconMetrics().sz_pic * 0.2); int dim = lrint(defaultIconMetrics().sz_pic * 0.2);
@ -42,7 +44,7 @@ void DivePictureWidget::mousePressEvent(QMouseEvent *event)
QByteArray itemData; QByteArray itemData;
QDataStream dataStream(&itemData, QIODevice::WriteOnly); QDataStream dataStream(&itemData, QIODevice::WriteOnly);
dataStream << filename << event->pos(); dataStream << filename << diveId;
QMimeData *mimeData = new QMimeData; QMimeData *mimeData = new QMimeData;
mimeData->setData("application/x-subsurfaceimagedrop", itemData); mimeData->setData("application/x-subsurfaceimagedrop", itemData);
@ -53,11 +55,8 @@ void DivePictureWidget::mousePressEvent(QMouseEvent *event)
drag->exec(Qt::CopyAction | Qt::MoveAction, Qt::CopyAction); drag->exec(Qt::CopyAction | Qt::MoveAction, Qt::CopyAction);
} }
QListView::mousePressEvent(event);
} else {
QListView::mousePressEvent(event);
} }
QListView::mousePressEvent(event);
} }
void DivePictureWidget::wheelEvent(QWheelEvent *event) void DivePictureWidget::wheelEvent(QWheelEvent *event)

View file

@ -2202,8 +2202,17 @@ void ProfileWidget2::dropEvent(QDropEvent *event)
QDataStream dataStream(&itemData, QIODevice::ReadOnly); QDataStream dataStream(&itemData, QIODevice::ReadOnly);
QString filename; QString filename;
QPoint pos; int diveId;
dataStream >> filename >> pos; dataStream >> filename >> diveId;
// If the id of the drag & dropped picture belongs to a different dive, then
// the offset we determine makes no sense what so ever. Simply ignore such an event.
// In the future, we might think about duplicating the picture or moving the picture
// from one dive to the other.
if (!current_dive || displayed_dive.id != diveId) {
event->ignore();
return;
}
#ifndef SUBSURFACE_MOBILE #ifndef SUBSURFACE_MOBILE
// Calculate time in dive where picture was dropped and whether the new position is during the dive. // Calculate time in dive where picture was dropped and whether the new position is during the dive.
@ -2211,19 +2220,15 @@ void ProfileWidget2::dropEvent(QDropEvent *event)
offset_t offset { (int32_t)lrint(timeAxis->valueAt(mappedPos)) }; offset_t offset { (int32_t)lrint(timeAxis->valueAt(mappedPos)) };
bool duringDive = current_dive && offset.seconds > 0 && offset.seconds < current_dive->duration.seconds; bool duringDive = current_dive && offset.seconds > 0 && offset.seconds < current_dive->duration.seconds;
// Flag which states whether the drag&dropped picture actually belongs to this dive.
// If this is not the case, the calculated offset makes no sense whatsoever and we must ignore the event.
bool belongsToDive = true;
// A picture was drag&dropped onto the profile: We have four cases to consider: // A picture was drag&dropped onto the profile: We have four cases to consider:
// 1a) The image was already shown on the profile and is moved to a different position on the profile. // 1a) The image was already shown on the profile and is moved to a different position on the profile.
// Calculate the new position and move the picture. // Calculate the new position and move the picture.
// 1b) The image was on the profile and is moved outside of the dive time. // 1b) The image was on the profile and is moved outside of the dive time.
// Remove the picture. // Remove the picture.
// 2a) The image was not on the profile, but belongs to the current dive. // 2a) The image was not on the profile and is moved into the dive time.
// Add the picture to the profile if it is during the dive. // Add the picture to the profile.
// 2b) The picture does not belong to the current dive. // 2b) The image was not on the profile and is moved outside of the dive time.
// For now, do nothing. We may think about adding the picture to the dive. // Do nothing.
auto oldPos = std::find_if(pictures.begin(), pictures.end(), [filename](const PictureEntry &e) auto oldPos = std::find_if(pictures.begin(), pictures.end(), [filename](const PictureEntry &e)
{ return e.filename == filename; }); { return e.filename == filename; });
if (oldPos != pictures.end()) { if (oldPos != pictures.end()) {
@ -2250,16 +2255,9 @@ void ProfileWidget2::dropEvent(QDropEvent *event)
// In both cases the picture list changed, therefore we must recalculate the y-coordinatesA. // In both cases the picture list changed, therefore we must recalculate the y-coordinatesA.
calculatePictureYPositions(); calculatePictureYPositions();
} else { } else {
// Cases 2a) and 2b): picture not on profile. Check if it belongs to current dive. // Cases 2a) and 2b): picture not on profile. We only have to take action for
// Note that FOR_EACH_PICTURE handles current_dive being null gracefully. // the first case: picture is moved into dive-time.
bool found = false; if (duringDive) {
FOR_EACH_PICTURE(current_dive) {
if (picture->filename == filename) {
found = true;
break;
}
}
if (found && duringDive) {
// Case 2a): add the picture at the appropriate position. // Case 2a): add the picture at the appropriate position.
// The case move from outside-to-outside of the profile plot was handled by // The case move from outside-to-outside of the profile plot was handled by
// the "&& duringDive" condition in the if above. // the "&& duringDive" condition in the if above.
@ -2273,14 +2271,10 @@ void ProfileWidget2::dropEvent(QDropEvent *event)
newPos = pictures.emplace(newPos, offset, filename, scene()); newPos = pictures.emplace(newPos, offset, filename, scene());
updateThumbnailXPos(*newPos); updateThumbnailXPos(*newPos);
calculatePictureYPositions(); calculatePictureYPositions();
} else if (!found) {
// Case 2b): Unknown picture. Ignore.
belongsToDive = false;
} }
} }
// Only signal the drag&drop action if the picture actually belongs to the dive. // Only signal the drag&drop action if the picture actually belongs to the dive.
if (belongsToDive)
DivePictureModel::instance()->updateDivePictureOffset(displayed_dive.id, filename, offset.seconds); DivePictureModel::instance()->updateDivePictureOffset(displayed_dive.id, filename, offset.seconds);
#endif #endif

View file

@ -96,14 +96,16 @@ QVariant DivePictureModel::data(const QModelIndex &index, int role) const
break; break;
case Qt::DisplayPropertyRole: case Qt::DisplayPropertyRole:
ret = QFileInfo(entry.filename).filePath(); ret = QFileInfo(entry.filename).filePath();
break;
case Qt::UserRole:
ret = entry.diveId;
break;
} }
} else if (index.column() == 1) { } else if (index.column() == 1) {
switch (role) { switch (role) {
case Qt::UserRole:
ret = QVariant::fromValue(entry.offsetSeconds);
break;
case Qt::DisplayRole: case Qt::DisplayRole:
ret = entry.filename; ret = entry.filename;
break;
} }
} }
return ret; return ret;