Modify

Opened 3 months ago

Closed 2 months ago

Last modified 2 months 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 3 months ago.
refactor_geoimagelayer.2.patch (55.9 KB) - added by francois2 3 months ago.
New patch
refactor_geoimagelayer.3.patch (61.8 KB) - added by francois2 3 months ago.
Patch v3
refactor_geoimagelayer.4.patch (61.9 KB) - added by francois2 3 months ago.
Patch v3.1
refactor_geoimagelayer_plugin.patch (21.0 KB) - added by francois2 3 months ago.
The patch on the photo adjust plugin

Download all attachments as: .zip

Change History (16)

Changed 3 months ago by francois2

comment:1 Changed 3 months ago by francois2

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 Changed 3 months ago by Don-vip

Component: CoreCore image mapping

comment:3 Changed 3 months ago by Don-vip

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 Changed 3 months ago by Don-vip

Milestone: 18.12

comment:5 in reply to:  3 Changed 3 months ago by francois2

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.

Changed 3 months ago by francois2

New patch

comment:6 Changed 3 months ago by francois2

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 Changed 3 months ago by Don-vip

2 is ok :)

Changed 3 months ago by francois2

Patch v3

Changed 3 months ago by francois2

Patch v3.1

Changed 3 months ago by francois2

The patch on the photo adjust plugin

comment:8 Changed 3 months ago by francois2

I updated the patch to add new methods to update the image and a new callback listener 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 for osd update and doing nothing most of the time.

Version 0, edited 3 months ago by francois2 (next)

comment:9 Changed 2 months ago by francois2

Description: modified (diff)

comment:10 Changed 2 months ago by Don-vip

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 Changed 2 months ago by Don-vip

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.

Add Comment


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

 
Note: See TracTickets for help on using tickets.