Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#17050 closed enhancement (fixed)

[Patch] Refactor the GeoImageLayer and related to use a Data class with a selection listener

Reported by: francois2 Owned by: team
Priority: normal Milestone: 18.12
Component: Core image mapping Version:
Keywords: Cc:

Description (last modified by francois2)

My goal is to add the ability to select multiple images in the geoimagelayer to be able to move them with the photoadjust plugin.

I started the work by moving the handle of the data to a new class ImageData with a selection listener. Most of the methods of the GeoImageLayer have been moved. The ImageViewerDialog now listen for the selection change. This remove the coupling with the GeoImageLayer.

I also fixed a few bugs with buttons on the ImageViewDialog

Attachments (5)

refactor_geoimagelayer.patch (55.4 KB ) - added by francois2 5 years ago.
refactor_geoimagelayer.2.patch (55.9 KB ) - added by francois2 5 years ago.
New patch
refactor_geoimagelayer.3.patch (61.8 KB ) - added by francois2 5 years ago.
Patch v3
refactor_geoimagelayer.4.patch (61.9 KB ) - added by francois2 5 years ago.
Patch v3.1
refactor_geoimagelayer_plugin.patch (21.0 KB ) - added by francois2 5 years ago.
The patch on the photo adjust plugin

Download all attachments as: .zip

Change History (16)

by francois2, 5 years ago

comment:1 by francois2, 5 years ago

Summary: Refactor the GeoImageLayer and related to use a Data class with a selection listener[Patch] Refactor the GeoImageLayer and related to use a Data class with a selection listener

comment:2 by Don-vip, 5 years ago

Component: CoreCore image mapping

comment:3 by Don-vip, 5 years ago

Wow impressive, thanks! Some feedback below:

  1. Can you please explain this change?
    @@ -493,7 +544,7 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange
          * @since 6392
          */
         public static GeoImageLayer getCurrentLayer() {
    -        return getInstance().currentLayer;
    +        return null;
         }
     
         /**
    
  1. Did you run ant checkstyle pmd to check the code style too?

comment:4 by Don-vip, 5 years ago

Milestone: 18.12

in reply to:  3 comment:5 by francois2, 5 years ago

Replying to Don-vip:

Wow impressive, thanks! Some feedback below:

Thanks!

  1. Can you please explain this change?
    @@ -493,7 +544,7 @@ public final class ImageViewerDialog extends ToggleDialog implements LayerChange
          * @since 6392
          */
         public static GeoImageLayer getCurrentLayer() {
    -        return getInstance().currentLayer;
    +        return null;
         }
     
         /**
    

I forgot to remove it. I fixed that as well as removing some unused methods.

  1. Did you run ant checkstyle pmd to check the code style too?

Nope. I fixed a few warnings.

by francois2, 5 years ago

New patch

comment:6 by francois2, 5 years ago

I'm working on a new patch to allow multiple image selection. In the meantime, I saw that the current patch broke the photoadjust plugin.

There is multiple options:

  1. wait until all patch are ready
  2. wait until I provide the patch to fix the photoadjust plugin

I prefer the 2. What do you think ?

comment:7 by Don-vip, 5 years ago

2 is ok :)

by francois2, 5 years ago

Patch v3

by francois2, 5 years ago

Patch v3.1

by francois2, 5 years ago

The patch on the photo adjust plugin

comment:8 by francois2, 5 years ago

I updated the patch to add new methods to update the image and a new callback when the image(s)change.

It introduced a performance problem in the photoadjust plugin, as on each move the image dialog was refreshed. I fixed it by checking the osd update and doing nothing most of the time.

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

comment:9 by francois2, 5 years ago

Description: modified (diff)

comment:10 by Don-vip, 5 years ago

Resolution: fixed
Status: newclosed

In 14590/josm:

fix #17050 - Refactor the GeoImageLayer and related to use a Data class with a selection listener (patch by francois2, modified)

comment:11 by Don-vip, 5 years ago

Thanks for the patches! :)
Plugin updated in [o34786:34787].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain team.
as The resolution will be set.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.