Modify

Opened 4 years ago

Closed 8 days ago

Last modified 8 days ago

#11905 closed enhancement (fixed)

[Patch] Option to move multiple images at once

Reported by: skorbut Owned by: Don-vip
Priority: normal Milestone: 19.09
Component: Core image mapping Version:
Keywords: sotmfr2019 Cc:

Description

I'd like to have an option to move multiple images at once. This would be useful if one tries to adjust images whose gpx track has a roughly constant offset to the real position (e.g. a road).

The images could be selectable using the rectangular and lasso selection tool. Alternatively/additionally multiple images may be selected using a suitable modifier key. The Ctrl key would come to mind but that is already used to modify the viewing direction of the image. Maybe the Alt key? Furthermore the combination of Alt+Shift+Click could be used to select the range of the currently selected image up to the clicked image?

Attachments (3)

patch_multiple_image_core.patch (37.8 KB) - added by francois2 5 months ago.
Patch select multiple image in the core v2.1
patch_multiple_image_core_reviewed.patch (29.9 KB) - added by Don-vip 2 months ago.
core patch, updated and reviewed
patch_multiple_image_plugin.patch (12.5 KB) - added by francois2 2 weeks ago.
Patch move multiple image in the photoadjust plugin v3.1

Download all attachments as: .zip

Change History (50)

comment:1 Changed 4 years ago by holgermappt

Component: Plugin photoadjustCore image mapping
Owner: changed from holgermappt to team

The selection of images is something that is done by the JOSM core. Right now you can select only one image because the image viewer can display only one image and the GeoImageLayer has only one variable to store the index of the selected image. I'm not sure how this can be changed in a meaningful way.

comment:2 Changed 14 months ago by JLZimmermann

This is a really good idea. If we could have an option display or not picture and by this way move pictures like if these were nodes it would be really helpfull.

comment:3 Changed 9 months ago by francois2

Owner: changed from team to francois2

comment:4 Changed 9 months ago by francois2

Status: newassigned

comment:5 Changed 9 months ago by francois2

I attached 2 patchs to implement the multiple selection of images in the core and multiple image editing in the photoadjust plugin.

In the core, the multiple image selection is disabled by default. It’s enabled on the plugin side. The rationale is that the image selection provides nothing without the plugin.

To select another photo use shift+click. Then can you move the selection by clicking on an image and dragging it around. You can select multiple photos by selecting a photo and then pressing shift+ctrl+click on another one. The photos between the 2 will be selected.

The shortcut to move one image to a specific point has been updated from shift to alt (to not conflict with the selection). I can move it back to shift if needed, but I wanted to make it consistent with the data selection. This new feature is a good opportunity to change it.

For now, moving multiple images with alt move all of them at the same point. I believe this should be changed to only translate the center of the selection to the new point. Tell me what you think. I can patch it later.

Also, updating the direction of multiple images is relative to the mouse point for each images.

See a demo here https://i.imgur.com/J1sx2pi.gif

Last edited 5 months ago by francois2 (previous) (diff)

comment:6 Changed 9 months ago by francois2

Summary: Option to move multiple images at once[Patch] Option to move multiple images at once

comment:7 Changed 8 months ago by francois2

After thinking a bit about it, I'm not sure about having a special path to enable/disable multiple image selection. Removing it is pretty easy, so tell me.

comment:8 Changed 8 months ago by holgermappt

The "Geotagged Images" panel could still be used to delete multiple selected images. I.e. if multiple images are selected to delete them, then it makes sense to have the complete select functionality in the core and have it enabled by default.

The content of the Geotagged Images panel could be updated later to show a preview of all selected images, a file list, or a combination of both. The first/last image buttons would still work if multiple images are selected, so I would not disable them.

The shortcuts for move (shift) can be changed if needed. It is not needed if the Photoadjust map mode (icon above the bus on the left side) is selected, so it is not that important. Alt+mouse is caught by my window manager and will move the complete JOSM window.

Have you checked the behavior with multiple image layers? That was tricky in the past.

The Photoadjust plugin has a menu entry to edit the properties of the selected photo (the patch disables it if multiple images are selected). I'm not sure if it would make sense to enable edits of the properties that are the same for all selected photos, e.g. to set the speed for all. Maybe not. Anyway, that can be changed later if needed.

I didn't review the patches or tried anything yet, but the changes look good so far. Okay, the spelling error fix in SelectAction.java should go into a separate ticket.

comment:9 Changed 8 months ago by francois2

Thanks for the review!

I'll remove the enable/disable multiple image selection and enable the first/last buttons in this patch.

Have you checked the behavior with multiple image layers? That was tricky in the past.

I did a little bit. The selection is still cleared on other layers. I'll check again.

I take others point as future features. Most of them are already on my todo list!

comment:10 Changed 8 months ago by francois2

I updated the patch and the demo: https://i.imgur.com/jKZjkGk.gif

comment:11 Changed 8 months ago by francois2

FIY I have multiple patchs ready after this one:

  • removing/deleting multiple images at once
  • moving multiple images at once with alt translate all selected photos instead of moving them a the mouse position
  • interpolate photo position between 2 selected photos (to align photos on a street for instance)

I can update this patch with these features if needed.

comment:12 Changed 8 months ago by holgermappt

I finally managed to compile the patches. Here are some observations:

  • I cannot place a photo on the map, alt+click selects the complete window (I guess this is a KDE default). I don't know how common such a setup is, but this needs to be configurable to map it to a different key. It can be any key, e.g. p+click (p like "position" or "place"). But that will work in the photoadjust map mode only as p is "split line" in a data layer.
  • Sometimes the Geotagged Images panel shows a photo if two are selected. But I'm not able to reproduce it.
  • The first/previous/next/last buttons work unreliably. Sometimes they are grayed out but become enabled if the mouse is moved over them. Sometimes the "previous" is grayed out but the "first" is not. Sometimes the button is drawn in the clicked state after the mouse was released.
  • Ctrl+click works as expected if multiple photos are selected. That can be used if multiple photos show the same object.
  • Only the photos within one geo image layer can be selected, this is as expected.
  • Photos that are not in the active geo image layer can be selected, but not moved, even if the map mode is active. My expectation as user would be to be able to move all photos that I'm able to select.
  • I'm not sure what should happen if multiple photos are selected and alt+click is done. What do you mean with "translate all selected photos"?
  • Idea: Show a line between the photos to indicate the order if shift+ctrl are hold down. Somehow mark the first photo. That makes it clear what photos will get selected.

It helps me to think of use cases, and this is what I can think of, not all relate to this ticket:

  • Sort and/or select photos based on different attributes. Next/previous based on that criteria, i.e. not by file name as it is now. I think of a list/table view with sortable columns. The list would reflect the select state.
  • Select one or multiple photos to delete them (e.g. they are not needed for mapping).
  • Select one photo on the map to move it.
  • Select multiple photos on the map to move them, e.g. to adjust an offset.
  • Set direction for one or multiple photos. E.g. all photos that show the same object.
  • Select one or multiple photos to create a new geo image layer with them. E.g. to create different sets of photos. I'm not sure if that is a real use case.
  • Place (untagged) photos one by one by stepping through the image panel (next, place, next, ...).
  • Change a property like speed or altitude for one or multiple photos (in the property dialog). Either set all to same value or add an offset.
  • Geometrical operations. There are some operations I can think of:
    • Distribute between first and last with equal distance between photos.
    • Distribute between first and last with distance proportional to photo capture time difference (i.e. assuming constant speed while taking photos).
    • Move to selected line (perpendicular), e.g. to move to a street.

The question is what functionality goes into core and what into the plugin. If we keep the current setup then the select+delete part goes into core and the rest into the plugin. But that is an arbitrary separation. We might want to create a new ticket for the core part if the functionality can be separated.

comment:13 in reply to:  12 ; Changed 8 months ago by francois2

Hi, thanks a lot for your review

Replying to holgermappt:

  • I cannot place a photo on the map, alt+click selects the complete window (I guess this is a KDE default). I don't know how common such a setup is, but this needs to be configurable to map it to a different key. It can be any key, e.g. p+click (p like "position" or "place"). But that will work in the photoadjust map mode only as p is "split line" in a data layer.

ok. Let's change this shortcut. I'll find something.

  • Sometimes the Geotagged Images panel shows a photo if two are selected. But I'm not able to reproduce it.

I didn't manage to reproduce it. If you have any ideas of what you did, tell me!

  • The first/previous/next/last buttons work unreliably. Sometimes they are grayed out but become enabled if the mouse is moved over them. Sometimes the "previous" is grayed out but the "first" is not. Sometimes the button is drawn in the clicked state after the mouse was released.

When multiple images are selected, the previous/next buttons can be disabled, but first/last buttons are not. For the other things, I didn't manage to reproduce them.

  • Ctrl+click works as expected if multiple photos are selected. That can be used if multiple photos show the same object.
  • Only the photos within one geo image layer can be selected, this is as expected.
  • Photos that are not in the active geo image layer can be selected, but not moved, even if the map mode is active. My expectation as user would be to be able to move all photos that I'm able to select.

Good catch. I'll fix it

  • I'm not sure what should happen if multiple photos are selected and alt+click is done. What do you mean with "translate all selected photos"?

In this patch, this move all photos to the mouse position. In a future patch it will translate them: Get the center of selected photos, then translate them with the new center at the mouse position. See the attached gif:
https://i.imgur.com/WrD0AWF.gif

  • Idea: Show a line between the photos to indicate the order if shift+ctrl are hold down. Somehow mark the first photo. That makes it clear what photos will get selected.

Good idea. I'll take a look. Maybe in a later patch.

It helps me to think of use cases, and this is what I can think of, not all relate to this ticket:

  • Sort and/or select photos based on different attributes. Next/previous based on that criteria, i.e. not by file name as it is now. I think of a list/table view with sortable columns. The list would reflect the select state.

good idea

  • Select one or multiple photos to delete them (e.g. they are not needed for mapping).

implemented in a future patch

  • Select one photo on the map to move it.

What kind of interactions do you want? I's already possible.

  • Select multiple photos on the map to move them, e.g. to adjust an offset.
  • Set direction for one or multiple photos. E.g. all photos that show the same object.
  • Select one or multiple photos to create a new geo image layer with them. E.g. to create different sets of photos. I'm not sure if that is a real use case.
  • Place (untagged) photos one by one by stepping through the image panel (next, place, next, ...).
  • Change a property like speed or altitude for one or multiple photos (in the property dialog). Either set all to same value or add an offset.

all above features are good ideas

  • Geometrical operations. There are some operations I can think of:
    • Distribute between first and last with equal distance between photos.

implemented in a future patch

  • Distribute between first and last with distance proportional to photo capture time difference (i.e. assuming constant speed while taking photos).

Good idea

  • Move to selected line (perpendicular), e.g. to move to a street.

The question is what functionality goes into core and what into the plugin. If we keep the current setup then the select+delete part goes into core and the rest into the plugin. But that is an arbitrary separation. We might want to create a new ticket for the core part if the functionality can be separated.

Agreed.

comment:14 in reply to:  13 Changed 7 months ago by francois2

I updated the patch to update the shortcut used to place a photo on the map. It's now ctrl + alt + click.

  • Ctrl+click works as expected if multiple photos are selected. That can be used if multiple photos show the same object.
  • Only the photos within one geo image layer can be selected, this is as expected.
  • Photos that are not in the active geo image layer can be selected, but not moved, even if the map mode is active. My expectation as user would be to be able to move all photos that I'm able to select.

Good catch. I'll fix it

I looked at the code, and it's seems to be volontary. See PhotoAdjustMapMode#enterMode. It could be handled by a future patch.

comment:15 Changed 6 months ago by francois2

Hi,

Any news on these patchs? I can rebase them if needed.

comment:16 in reply to:  15 ; Changed 6 months ago by holgermappt

Replying to francois2:

Any news on these patchs? I can rebase them if needed.

The only news that I can provide is that I had to revert the patch from January because it constantly crashed JOSM. Unfortunately I don't have time to look at this at the moment, sorry.

comment:17 in reply to:  16 Changed 6 months ago by francois2

Replying to holgermappt:

Replying to francois2:

Any news on these patchs? I can rebase them if needed.

The only news that I can provide is that I had to revert the patch from January because it constantly crashed JOSM. Unfortunately I don't have time to look at this at the moment, sorry.

oh :( Do you have any stacktrace? Meanwhile I'll rebase my patch and see if I can reproduce them.

Changed 5 months ago by francois2

Patch select multiple image in the core v2.1

comment:18 Changed 5 months ago by francois2

I rebased and updated the core patch as there was a conflict. I didn't manage to reproduce any crashes. Any guidance would be greatly appreciated!

comment:19 Changed 3 months ago by holgermappt

I created a wiki page with the items that should be considered: PhotoSelectionAndAdjustments. To be consistent with the standard meaning of shift+click and control+click (select multiple and add/remove single) I assigned that keys to the core select functionality. For the plugin the alt modifier could be added. There are some options (1/2, a/b, A/B). The next step would be to decide what to implement, not everything might make sense.

The patches work for me in the current version, just the alt+click does not work, but alt+control+click does the same job.

comment:20 in reply to:  19 ; Changed 3 months ago by francois2

Replying to holgermappt:

I created a wiki page with the items that should be considered: PhotoSelectionAndAdjustments. To be consistent with the standard meaning of shift+click and control+click (select multiple and add/remove single) I assigned that keys to the core select functionality. For the plugin the alt modifier could be added. There are some options (1/2, a/b, A/B). The next step would be to decide what to implement, not everything might make sense.

The patches work for me in the current version, just the alt+click does not work, but alt+control+click does the same job.

What did you expect with alt+click ?

comment:21 in reply to:  20 ; Changed 3 months ago by holgermappt

Replying to francois2:

What did you expect with alt+click ?

Alt+click is the key/mouse sequence to position a photo, if the patches are applied. At least that is what is displayed in the status line. I expected to be able to place a photo on the map.

Last edited 3 months ago by holgermappt (previous) (diff)

comment:22 in reply to:  21 Changed 3 months ago by francois2

Replying to holgermappt:

Replying to francois2:

What did you expect with alt+click ?

Alt+click is the key/mouse sequence to position a photo, if the patches are applied. At least that is what is displayed in the status line. I expected to be able to place a photo on the map.

I updated it to ctrl + alt + click comment #14, since it was not working for you. I can revert it.

comment:23 Changed 3 months ago by francois2

At least that is what is displayed in the status line.

I looked at the code, and there is nothing on the plugin that update the status line. It seems that this the default one from JOSM.
But it could be a great idea to update it. I will see if it's possible, if yes I'll do it in a later patch.

comment:24 Changed 2 months ago by Don-vip

Keywords: sotmfr2019 added

comment:25 Changed 2 months ago by Don-vip

Milestone: 19.07

comment:26 Changed 2 months ago by Don-vip

Milestone: 19.0719.08

Milestone renamed

Changed 2 months ago by Don-vip

core patch, updated and reviewed

comment:27 Changed 2 months ago by Don-vip

@francois2 I have reviewed the core patch and made some modifications in new attachment:

  • reverted the deletion of public API to avoid breaking plugins
  • reverted some minor renames of methods. Even if it would look nice, it would break plugins too
  • modified some code

I didn't test anything. Can you please check this patch works with an updated plugin?

comment:28 Changed 6 weeks ago by Don-vip

Owner: changed from francois2 to skorbut
Status: assignedneedinfo

comment:29 Changed 6 weeks ago by Don-vip

Owner: changed from skorbut to francois2

comment:30 Changed 4 weeks ago by Don-vip

@francois2: ping :)

comment:31 Changed 3 weeks ago by Don-vip

Milestone: 19.0819.09

Ticket retargeted after milestone closed

Changed 2 weeks ago by francois2

Patch move multiple image in the photoadjust plugin v3.1

comment:32 Changed 2 weeks ago by francois2

I updated the patch to adapt to the changes of the core. Basically I renamed selectedImagesChanged back to selectedImageChanged. I also fixed a few wordings in the comments about the new shortcuts to move photos with control+alt+click.

comment:33 Changed 2 weeks ago by Don-vip

Great! So my core patch works fine, and I can apply it? :)

comment:34 Changed 2 weeks ago by francois2

I was a bit in the dark with the renaming. If all tests pass that should be fine ;)

comment:35 Changed 2 weeks ago by Don-vip

Owner: changed from francois2 to Don-vip
Status: needinfonew

comment:36 Changed 2 weeks ago by Don-vip

In 15333/josm:

see #11905 - Option to move multiple images at once (core part, patch by francois2, modified)

comment:37 Changed 2 weeks ago by Don-vip

In 15334/josm:

see #11905 - fix java warnings

comment:38 Changed 2 weeks ago by Don-vip

@holgermappt core patch is merged, can you please take a look at the plugin patch and merge it if you agree with the changes?

comment:39 in reply to:  38 ; Changed 12 days ago by holgermappt

Replying to Don-vip:

@holgermappt core patch is merged, can you please take a look at the plugin patch and merge it if you agree with the changes?

Can you please describe what the expected behavior is, i.e. what can I do with or without the plugin, what key/click sequences are available? On PhotoSelectionAndAdjustments I tried to describe what could be implemented. What was actually implemented?

Without the photoadjust plugin I can select additional images with shift+click. That moves the selected image if the plugin is active. How can I select multiple images with the plugin active?

Shift+click de-selects the image if the plugin is active. Was that the original behavior? My expectation is that the image stays selected.

comment:40 in reply to:  39 Changed 12 days ago by holgermappt

Replying to holgermappt:

Without the photoadjust plugin I can select additional images with shift+click. That moves the selected image if the plugin is active. How can I select multiple images with the plugin active?

Shift+click de-selects the image if the plugin is active. Was that the original behavior? My expectation is that the image stays selected.

The comments above are for the unpatched version, sorry. With the patch I get this functionality:

  • click on a photo: select it, deselect all previously selected photos
  • shift+click: add/remove photo from selection, remove all photos of other layers from selection
  • click+drag: move selected photos
  • control+alt+click: position all selected photos at the pointer position; first I thought this does not make sense if multiple photos are selected, but after a closer look it might be useful for a first "sort" step; if clicked on a photo the clicked photo is selected after the previously selected photos are positioned at the pointer
  • control+click: set direction for all selected photos; if clicked on a photo the clicked photo is selected after the direction was set for the previously selected photos
  • the photo property editor is disabled if multiple photos are selected
  • the image viewer is disabled if multiple photos are selected
  • photos of an image layer that is not active can be selected, but not positioned or modified otherwise

Is that the complete functionality? Are there plans to implement more functionality?

comment:41 Changed 12 days ago by francois2

I do plan to add more features.

I had a few patchs ready that need a small rework since the core patch was modified:

  • Removing/deleting multiple images (core patch)
  • Align a sequence of images between the 2 selected images (plugin patch)
  • When using control+alt+click, translate them instead of moving them at the same location. See the gif of the comment:13. (plugin patch)

I do plan to work on the plugin if have more needs/feedbacks. I have your list of features and some others requests from seasoned mapillary contributors ;). This should keep me busy for a while.

comment:42 Changed 8 days ago by holgermappt

I reviewed the plugin patch and committed it with some minor modifications in comments and formatting. I removed the data argument from PhotoAdjustWorker.translatePhoto() as it is not used.

I assume we leave this ticket active to track the other improvements francois2 is working on, or should that go into separate tickets?

comment:43 Changed 8 days ago by Klumbumbus

comment:44 in reply to:  42 Changed 8 days ago by francois2

Replying to holgermappt:

I assume we leave this ticket active to track the other improvements francois2 is working on, or should that go into separate tickets?

My guess if you keep it open is that it'll be a never ending ticket. I don't know your convention, so feel free to do what you want.

comment:45 Changed 8 days ago by holgermappt

Resolution: fixed
Status: newclosed

I'm closing this ticket as the initial request was implemented. Please open new tickets for further improvements. Thanks for the patches!

comment:46 in reply to:  45 Changed 8 days ago by francois2

Replying to holgermappt:

Please open new tickets for further improvements.

I will.

Replying to holgermappt:

Thanks for the patches!

Thanks for merging them !

comment:47 Changed 8 days ago by Don-vip

Thanks guys!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Don-vip.
as The resolution will be set.
The resolution will be deleted.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.