[GEOT-7109] Swt Module review and update #3921

moovida · 2022-06-08T10:34:14Z

This PR upgrades the SWT module as per discussions had in the mailinglist.

This basically upgrades swt libraries, updates to latest gt api and fixes the bugs reported in JIRA.

After some discussion, it has been decided to remove the RCP related part, since it is too complex to maintain (it would need an existing RCP project in a different repository). Also in the docs these parts were removed.

Checklist

I have read the contribution guidelines.
I have sent a Contribution Licence Agreement (not required for small changes, e.g., fixing typos in documentation).
First PR targets the main branch (backports managed later; ignore for branch specific issues).
Avoid Java 9+ split packages.
All the build checks are green (see automated QA checks).

…iles due to wrong formatting

moovida · 2022-06-08T10:36:20Z

@IanMayo , for your info. This is the PR. I am sorry it took that long, but after the fixes I never got to do the docs until today. Which resulted in merge conflicts. Should be all green now.

IanMayo · 2022-06-08T14:12:09Z

Thanks @moovida , you've done a good job. I'm surprised at the volume of SWT changes - but I guess you've done some general tidying/refactoring too, right?

Is it an option to distribute the sample shapefile with the tutorial? Loading/displaying the coastlines does give an "acknowledgement" that data has been loaded/rendered, IMHO.

moovida · 2022-06-08T14:21:25Z

Hi @IanMayo , yes there have been a couple of quick refactorings to cleanup and give the possibility to handle actions.

I am not sure how you mean to distribute a shp sample? You would like to be able to run the example and have a shp directly loaded? Or a sample to load with the "open shapefile" option?

IanMayo · 2022-06-08T14:57:49Z

Hello @moovida , for the tutorial element of the PR, it would seem useful to finish with some loaded data - since this demonstrates:

data on the plot
items in the layers panel
a working EPSG

I see there is already a shapefile distributed in docs\src\main\resources\org\geotools\grid. Is it an option to include the coastlines shapefile that you originally used in the tutorial? (I recognise the original tutorial was a demonstration of you loading a shapefile from your local PC - but this would be more of a series of steps for a potential user to load/view the data).

moovida · 2022-06-08T15:14:31Z

Hi @IanMayo , I like the idea. We could also show how a "load default shp" action is added:

/**
 * Action to open a shapefile from a geotools source code location.
 */
public class LoadDefaultShpAction extends MapAction implements ISelectionChangedListener {

    public LoadDefaultShpAction() {
        super("Load default SHP", "Load the default geotools installation shapefile.", null);
    }

    public void run() {
        try {
            MapContent mapContent = mapPane.getMapContent();
            
            File shapeFile = new File("........geotools/docs/src/main/resources/org/geotools/grid/oz.shp");
            ShapefileDataStore store = new ShapefileDataStore(shapeFile.toURI().toURL());
            SimpleFeatureSource featureSource = store.getFeatureSource();
            SimpleFeatureCollection shapefile = featureSource.getFeatures();
            Style style = Utils.createStyle(shapeFile, featureSource);
            mapContent.addLayer(new FeatureLayer(shapefile, style));

            mapPane.redraw();
        } catch (Exception e) {
            e.printStackTrace();
        }

    }

    public void selectionChanged( SelectionChangedEvent arg0 ) {
    }
}

Which would be nice for the user to load something right away.

Or, we could just show how to load a shp at startup.

My main issue is that a user that is trying this is not forced to be doing this from the geotools source. In fact I imagine most people doing this from maven geotools. Therefore a shp from the repo might not be an option.

Should we just tell in the tutorial to place the path to an existing shapefile on the user's disk?

IanMayo · 2022-06-08T15:24:48Z

Therefore a shp from the repo might not be an option.

oops, of course. Just in case the "student" doesn't work with shapefiles (and may not have one handy), but still wants to complete the tutorial. I guess we could provide a URL for a shapefile that we can expect to be persistent - such as: https://www.naturalearthdata.com/downloads/110m-physical-vectors/

mprins

please run the formatting tools on the code and the pom file; specifically the pom file was formatted according to the project rules, but that is destroyed with this PR.

You can use mvn clean install -T1.1C -f modules/unsupported/swt/pom.xml

modules/unsupported/swt/pom.xml

moovida · 2022-06-12T11:25:17Z

please run the formatting tools on the code and the pom file; specifically the pom file was formatted according to the project rules, but that is destroyed with this PR.

You can use mvn clean install -T1.1C -f modules/unsupported/swt/pom.xml

Thanks a lot for this.

moovida · 2022-06-12T12:13:19Z

oops, of course. Just in case the "student" doesn't work with shapefiles (and may not have one handy), but still wants to complete the tutorial. I guess we could provide a URL for a shapefile that we can expect to be persistent - such as: https://www.naturalearthdata.com/downloads/110m-physical-vectors/

@IanMayo, let me know what you think. I can make it even more tutorial-verbose, but I think this might be out of scope.

modules/unsupported/swt/pom.xml

IanMayo · 2022-06-13T08:38:35Z

@IanMayo, let me know what you think. I can make it even more tutorial-verbose, but I think this might be out of scope.

The new version, with OSM & Shapefiles is great - thanks.

IanMayo

Appreciate both the fixes/updates and the more comprehensive tutorial.

aaime · 2022-06-13T09:01:32Z

Added labels for automatic backport (I think you might want to backport this right?).
Backport bot should be ready shortly: #3923

aaime · 2022-06-13T09:43:56Z

Bot ready, squash merging and checking if any backport PR is automatically generated.

geoserver-bot · 2022-06-13T09:44:45Z

The backport to 27.x failed:

The process '/usr/bin/git' failed with exit code 1

stderr

error: could not apply 4ce2d1a2f7... apply proper formatting rules
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

stdout

Auto-merging modules/unsupported/swt/pom.xml
[backport-3921-to-27.x efa2c9f1a5] fix dependencies and some minor improvements
 Author: Andrea Antonello <andrea.antonello@gmail.com>
 Date: Thu Mar 31 16:25:38 2022 +0200
 9 files changed, 65 insertions(+), 9 deletions(-)
 create mode 100644 modules/unsupported/swt/src/main/java/org/geotools/swt/action/ExitAction.java
Auto-merging modules/unsupported/swt/pom.xml
[backport-3921-to-27.x 373d48ab5f] upgrade to uptodate swt libraries + fixes in tools
 Author: Andrea Antonello <andrea.antonello@gmail.com>
 Date: Fri Apr 1 08:54:29 2022 +0200
 5 files changed, 229 insertions(+), 168 deletions(-)
 rewrite modules/unsupported/swt/pom.xml (89%)
[backport-3921-to-27.x 7829e3e1a3] fix for crs madness on loading
 Author: Andrea Antonello <andrea.antonello@gmail.com>
 Date: Fri Apr 1 10:24:22 2022 +0200
 4 files changed, 205 insertions(+), 154 deletions(-)
[backport-3921-to-27.x a8e06b4656] re-fix necessary swt actions class to add to the menus + resetting Utiles due to wrong formatting
 Author: Andrea Antonello <andrea.antonello@gmail.com>
 Date: Wed Jun 8 12:24:31 2022 +0200
 4 files changed, 174 insertions(+), 85 deletions(-)
 create mode 100644 modules/unsupported/swt/src/main/java/org/geotools/swt/utils/SwtActionsHandler.java
[backport-3921-to-27.x 4cf06dea2e] update swt modules documentation
 Author: Andrea Antonello <andrea.antonello@gmail.com>
 Date: Wed Jun 8 12:25:32 2022 +0200
 7 files changed, 63 insertions(+), 271 deletions(-)
 delete mode 100644 docs/user/images/gtswt_rcp_01.png
 delete mode 100644 docs/user/images/gtswt_rcp_02.png
 rewrite docs/user/images/gtswt_standalone_01.png (99%)
 create mode 100644 docs/user/images/gtswt_standalone_02.png
 delete mode 100644 docs/user/unsupported/swt/rcp.rst
 rewrite docs/user/unsupported/swt/swtmapframe.rst (70%)
Auto-merging modules/unsupported/swt/pom.xml
CONFLICT (content): Merge conflict in modules/unsupported/swt/pom.xml

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-27.x 27.x
# Navigate to the new working tree
cd .worktrees/backport-27.x
# Create a new branch
git switch --create backport-3921-to-27.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 79d16038bee6129ff05c2241e0c48b7fb81d0957,c2a5c2e8d344cc06c221d16494738c8fc8e9aed9,30e6a273475cbe42cc943c8d48a48f9703455dcf,784b32c08a5a3b02b6c42a3fa03dccbf31c4fd77,90ca7bf9646f5cdc9896895eb92ff6bc46b28517,4ce2d1a2f773411096134ff62ea5e4b145b1694d,1f0307e4e34c5441fcc24e7e63376e15b7c218fe,04cb51b513f862dbf95c02fd369017a6d5008757
# Push it to GitHub
git push --set-upstream origin backport-3921-to-27.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-27.x

Then, create a pull request where the base branch is 27.x and the compare/head branch is backport-3921-to-27.x.

geoserver-bot · 2022-06-13T09:44:47Z

The backport to 26.x failed:

The process '/usr/bin/git' failed with exit code 1

stderr

error: could not apply 79d16038be... fix dependencies and some minor improvements
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

stdout

Auto-merging modules/unsupported/swt/pom.xml
Auto-merging modules/unsupported/swt/src/main/java/org/geotools/swt/control/ExceptionMonitor.java
CONFLICT (content): Merge conflict in modules/unsupported/swt/src/main/java/org/geotools/swt/control/ExceptionMonitor.java

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-26.x 26.x
# Navigate to the new working tree
cd .worktrees/backport-26.x
# Create a new branch
git switch --create backport-3921-to-26.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick 79d16038bee6129ff05c2241e0c48b7fb81d0957,c2a5c2e8d344cc06c221d16494738c8fc8e9aed9,30e6a273475cbe42cc943c8d48a48f9703455dcf,784b32c08a5a3b02b6c42a3fa03dccbf31c4fd77,90ca7bf9646f5cdc9896895eb92ff6bc46b28517,4ce2d1a2f773411096134ff62ea5e4b145b1694d,1f0307e4e34c5441fcc24e7e63376e15b7c218fe,04cb51b513f862dbf95c02fd369017a6d5008757
# Push it to GitHub
git push --set-upstream origin backport-3921-to-26.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-26.x

Then, create a pull request where the base branch is 26.x and the compare/head branch is backport-3921-to-26.x.

aaime · 2022-06-13T09:46:08Z

Well it run, but found conflicts... looks like a backport will have to be done manually, if you plan to do one.

moovida · 2022-06-13T12:18:09Z

Hi @aaime , thank you.
It is strange, because I read about formatting issues. Could that have something to do with the fact that I initially scrambled the formatting and fixed it on the last commits? It should not, since it is squashed, right?

aaime · 2022-06-13T12:19:11Z

Correct, it should not.... try to do a cherry-pick and find out what is the source of the conflict :-D

moovida · 2022-06-13T12:40:30Z

Well, I tried the manual backport and fixed the small issues it had for 26.x

Am I supposed to do this?

git push --set-upstream origin backport-3921-to-26.x

even if along the way it took the original geotools repo as upstream? It is the first time I try to do a backport.

aaime · 2022-06-13T12:41:59Z

Just a push on your fork, no need to set the upstream there, you can set it while opening the PR.
Remember there is 27.x too, main is now 28-SNAPSHOT.

moovida · 2022-06-13T12:44:33Z

In my case origin is my repo, so the push should be there right (the set-upstream should take care of it in the push?)?
Sorry, I just do not want to break stuff :-)

Yes, 27 is next, if I am able to close 26.

moovida · 2022-06-13T12:51:48Z

Ok, the backport PR: #3926
shows me zillions of conflicting files. Not even sure how that can happen, but if this need much more care, then I will have to lay it down for now.
Not even sure @IanMayo is interested in a backport, even if it would sure be handy in general.

IanMayo · 2022-06-13T12:54:55Z

Hello @moovida

No, the backport won't bring value to me. If it just worked, then that would be fine. If it's troublesome then my needs are met with the 28.x version.

moovida · 2022-06-13T13:09:34Z

Ok, I was just trying to go against the wrong branch. In fact it should work now. Were really small fixes. The 2 PR are now open for 26 and 27.

moovida added 5 commits June 8, 2022 11:28

fix dependencies and some minor improvements

79d1603

upgrade to uptodate swt libraries + fixes in tools

c2a5c2e

fix for crs madness on loading

30e6a27

re-fix necessary swt actions class to add to the menus + resetting Ut…

784b32c

…iles due to wrong formatting

update swt modules documentation

90ca7bf

mprins requested changes Jun 10, 2022

View reviewed changes

modules/unsupported/swt/pom.xml Outdated Show resolved Hide resolved

moovida added 3 commits June 12, 2022 13:25

apply proper formatting rules

4ce2d1a

add data loading example and mapaction example to docs

1f0307e

cleanup commments in pom

04cb51b

mprins approved these changes Jun 13, 2022

View reviewed changes

modules/unsupported/swt/pom.xml Outdated Show resolved Hide resolved

IanMayo approved these changes Jun 13, 2022

View reviewed changes

aaime added backport 27.x Backport to 27.x backport 26.x labels Jun 13, 2022

aaime merged commit aaa25ef into geotools:main Jun 13, 2022

geoserver-bot added the failed backport label Jun 13, 2022

moovida mentioned this pull request Jun 13, 2022

Backport 3921 to 26.x #3926

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GEOT-7109] Swt Module review and update #3921

[GEOT-7109] Swt Module review and update #3921

moovida commented Jun 8, 2022 •

edited by mprins

moovida commented Jun 8, 2022

IanMayo commented Jun 8, 2022

moovida commented Jun 8, 2022

IanMayo commented Jun 8, 2022

moovida commented Jun 8, 2022

IanMayo commented Jun 8, 2022

mprins left a comment

moovida commented Jun 12, 2022

moovida commented Jun 12, 2022

IanMayo commented Jun 13, 2022

IanMayo left a comment

aaime commented Jun 13, 2022 •

edited

aaime commented Jun 13, 2022

geoserver-bot commented Jun 13, 2022

geoserver-bot commented Jun 13, 2022

aaime commented Jun 13, 2022

moovida commented Jun 13, 2022

aaime commented Jun 13, 2022

moovida commented Jun 13, 2022

aaime commented Jun 13, 2022

moovida commented Jun 13, 2022

moovida commented Jun 13, 2022

IanMayo commented Jun 13, 2022

moovida commented Jun 13, 2022

[GEOT-7109] Swt Module review and update #3921

[GEOT-7109] Swt Module review and update #3921

Conversation

moovida commented Jun 8, 2022 • edited by mprins

Checklist

moovida commented Jun 8, 2022

IanMayo commented Jun 8, 2022

moovida commented Jun 8, 2022

IanMayo commented Jun 8, 2022

moovida commented Jun 8, 2022

IanMayo commented Jun 8, 2022

mprins left a comment

Choose a reason for hiding this comment

moovida commented Jun 12, 2022

moovida commented Jun 12, 2022

IanMayo commented Jun 13, 2022

IanMayo left a comment

Choose a reason for hiding this comment

aaime commented Jun 13, 2022 • edited

aaime commented Jun 13, 2022

geoserver-bot commented Jun 13, 2022

geoserver-bot commented Jun 13, 2022

aaime commented Jun 13, 2022

moovida commented Jun 13, 2022

aaime commented Jun 13, 2022

moovida commented Jun 13, 2022

aaime commented Jun 13, 2022

moovida commented Jun 13, 2022

moovida commented Jun 13, 2022

IanMayo commented Jun 13, 2022

moovida commented Jun 13, 2022

moovida commented Jun 8, 2022 •

edited by mprins

aaime commented Jun 13, 2022 •

edited