[Patch] Align images in the photoadjust plugin

[francois2]

This feature allow to select 2 images, and interpolate images position between the 2 selected images.
The new menu entry is enabled when 2 images are selected.

Demo:

[holgermappt]

It is nice that you can position a lot of photos that don't have an initial position, i.e. no GPS data, by positioning the first and the last and then everything else between the two with the new function. But for photos that are on the map already it is not that easy to see what is between the two selected, i.e. what will be updated. Would it make sense to draw something like a line from photo to photo in the order they are sorted (just an idea)?

The interpolate() function is a little bit confusing.

• If the first photo is firstPhoto, then the name of the variable with the position oft that photo should relate to the photo variable, e.g. firstPos instead of pos1.
• If a variable is named nbPhotos I would assume that it reflects the total number of photos, either including the first and last that are not modified or just the photos that are updated. But it is neither of the two. It is used as number of segments between first and last, so a better name is nbSegments.
• I would pre-calculate (pos2.getX() - pos1.getX()) / nbPhotos (same for Y) as that is needed for all updated photos. Is there an offset object that can be added to pos1? Then it could be written as newPos = pos1 + i * offset.

My internal rule is to have at least three characters for variables, mainly because I use a text editor and no Java IDE. Search and replace is a lot easier that way. It would be nice if you could use idx instead of i and evt instead of e (try to search for e, it will match almost every line).

I need to read a little more about @Mocked and Expectations to understand what happens in the test.

[francois2]

Hi, thanks for the review. I'll improve the code based on your comments. I'm quite busy but I'll try to do it by the end of the week.

[francois2]

Align images in the photoadjust plugin

[francois2]

It is nice that you can position a lot of photos that don't have an initial position, i.e. no GPS data, by positioning the first and the last and then everything else between the two with the new function. But for photos that are on the map already it is not that easy to see what is between the two selected, i.e. what will be updated. Would it make sense to draw something like a line from photo to photo in the order they are sorted (just an idea)?

That would be great! We should add a preview dialog before doing the interpolation and a button to confirm it. Or are you thinking of another way to do it?

Anyway, I updated the patch with your comments.

[francois2]

Hi there,

Any update on this?

[holgermappt]

Replying to francois2:

Any update on this?

I didn't have time to look into this lately, but I plan to do so in a week or so.

[holgermappt]

Replying to francois2:

But for photos that are on the map already it is not that easy to see what is between the two selected, i.e. what will be updated. Would it make sense to draw something like a line from photo to photo in the order they are sorted (just an idea)?

That would be great! We should add a preview dialog before doing the interpolation and a button to confirm it. Or are you thinking of another way to do it?

I was thinking of a line that is displayed if two photos are selected. The line would connect the first (selected) photo with the second (not selected), third, ... (all not selected), to the last (selected). Or the line is always displayed, like a GPX track with the photos as waypoints.

The question is how generic this should be and what other operations are possible. If different operations are possible based on the selected photos then it would be nice if there is a hint what is possible and how the result would look like. E.g. a panel with buttons for the different operations with the buttons for possible operations active and the others grayed out. If the mouse is over a button a live preview is shown on the map.

Some sort of preview makes sense anyway as there is no easy way to undo the photo operations. Or we add a generic undo first (here is a related post: ​https://lists.openstreetmap.org/pipermail/josm-dev/2016-January/007605.html)

That are all ideas from the usability point of view. So far I didn't think about how it can be implemented in Java.

[francois2]

That are all ideas from the usability point of view. So far I didn't think about how it can be implemented in Java.

These improvements would be great. I need to think of how to do that from an UX point of view. In the meantime, is this patch ready to be merged?

[Tyndare]

I love this feature, the patch works well !
Actually I would also need interpolation of photo direction (my camera doesn't have a compass), if I have time I'll try to work on it (as a second menu entry).
I think that the generic "undo" would be more useful than a preview/confirm dialog here.
Thanks.

[francois2]

Replying to Tyndare:

I love this feature, the patch works well !

Thanks !

Actually I would also need interpolation of photo direction (my camera doesn't have a compass), if I have time I'll try to work on it (as a second menu entry).

You may be interested in this patch (​https://github.com/francois2metz/josm-plugins/commit/352b58ea47d3777fc978706650e23e49ecc5bded) that add a menu entry on the geoimagelayer to set every image to look at the next one in the sequence (except the last one, this must be fixed). Tell me if it fit your needs.

[Don-vip]

@holgermappt what's the status of this patch?