sis-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From desruisse...@apache.org
Subject [sis] branch geoapi-4.0 updated: Fix a problem at initialization of CRS choices (was initialized to the wrong CRS).
Date Fri, 24 Apr 2020 17:38:07 GMT
This is an automated email from the ASF dual-hosted git repository.

desruisseaux pushed a commit to branch geoapi-4.0
in repository https://gitbox.apache.org/repos/asf/sis.git


The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
     new b729668  Fix a problem at initialization of CRS choices (was initialized to the wrong
CRS).
b729668 is described below

commit b72966864487dcacaf1471b1324c639037966114
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Fri Apr 24 19:36:52 2020 +0200

    Fix a problem at initialization of CRS choices (was initialized to the wrong CRS).
---
 .../java/org/apache/sis/gui/map/StatusBar.java     | 42 ++++++++++---------
 .../org/apache/sis/gui/referencing/MenuSync.java   | 48 +++++++++++++++++-----
 .../gui/referencing/RecentReferenceSystems.java    | 38 +++++++++--------
 3 files changed, 82 insertions(+), 46 deletions(-)

diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/StatusBar.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/StatusBar.java
index 8e053cb..3cf8192 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/StatusBar.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/StatusBar.java
@@ -29,6 +29,7 @@ import javafx.scene.layout.Priority;
 import javafx.scene.control.Label;
 import javafx.scene.control.Button;
 import javafx.scene.control.Tooltip;
+import javafx.scene.control.Menu;
 import javafx.scene.control.ContextMenu;
 import javafx.scene.input.MouseEvent;
 import javafx.scene.text.TextAlignment;
@@ -275,9 +276,9 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
      * If the {@code choices} argument is non-null, user will be able to select different
CRS
      * using the contextual menu on the status bar.
      *
-     * @param  choices  the manager of reference systems chosen by user, or {@code null}
if none.
+     * @param  systemChooser  the manager of reference systems chosen by user, or {@code
null} if none.
      */
-    public StatusBar(final RecentReferenceSystems choices) {
+    public StatusBar(final RecentReferenceSystems systemChooser) {
         localToObjectiveCRS = MathTransforms.identity(BIDIMENSIONAL);
         localToFormatCRS    = localToObjectiveCRS;
         targetCoordinates   = new GeneralDirectPosition(BIDIMENSIONAL);
@@ -297,12 +298,15 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
         position.setTextAlignment(TextAlignment.RIGHT);
         canvasProperty = new SimpleObjectProperty<>(this, "canvas");
         canvasProperty.addListener(this::onCanvasSpecified);
-        if (choices == null) {
+        if (systemChooser == null) {
             areaOfInterest = null;
         } else {
-            areaOfInterest = choices.areaOfInterest;
-            final ContextMenu menu = new ContextMenu(choices.createMenuItems(this::onSelectCRS));
-            view.setOnMousePressed((MouseEvent event) -> {
+            areaOfInterest = systemChooser.areaOfInterest;
+            final Menu choices = systemChooser.createMenuItems((property, oldValue, newValue)
-> {
+                findFormatOperation(newValue instanceof CoordinateReferenceSystem ? (CoordinateReferenceSystem)
newValue : null);
+            });
+            final ContextMenu menu = new ContextMenu(choices);
+            view.setOnMousePressed((event) -> {
                 if (event.isSecondaryButtonDown()) {
                     menu.show((HBox) event.getSource(), event.getScreenX(), event.getScreenY());
                 } else {
@@ -607,6 +611,19 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
                 }
             });
         } else {
+            /*
+             * If the requested CRS is the objective CRS, avoid above costly operation.
+             * The work that we need to do is to cancel the effect of `localToFormatCRS`.
+             * As a special case if `objectiveCRS` was unknown before this method call,
+             * set it to the given value. This is needed for initializing the format CRS
+             * to the first reference system listed in `RecentReferenceSystems` choices.
+             * We could not do this work at construction time because the CRS choices may
+             * be computed in a background thread, in which case it became known only a
+             * little bit later and given to `StatusBar` through listeners.
+             */
+            if (objectiveCRS == null) {
+                objectiveCRS = crs;
+            }
             position.setMinWidth(0);
             resetFormatCRS(Styles.NORMAL_TEXT);
         }
@@ -920,19 +937,6 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
     }
 
     /**
-     * Invoked when the user selects a new reference system for the coordinates to show in
status bar.
-     *
-     * @param  property  the {@link org.apache.sis.gui.referencing.MenuSync} property.
-     * @param  oldValue  the old reference system, or {@code null} if none.
-     * @param  newValue  the CRS to use for formatting coordinates in this status bar.
-     */
-    private void onSelectCRS(ObservableValue<? extends ReferenceSystem> property,
-                             ReferenceSystem oldValue, ReferenceSystem newValue)
-    {
-        findFormatOperation(newValue instanceof CoordinateReferenceSystem ? (CoordinateReferenceSystem)
newValue : null);
-    }
-
-    /**
      * Returns the error message currently shown.
      *
      * @return the current error message, or an empty value if none.
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/MenuSync.java
b/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/MenuSync.java
index a51c018..f25f675 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/MenuSync.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/MenuSync.java
@@ -19,7 +19,6 @@ package org.apache.sis.gui.referencing;
 import java.util.Arrays;
 import java.util.IdentityHashMap;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ChangeListener;
@@ -86,8 +85,7 @@ final class MenuSync extends SimpleObjectProperty<ReferenceSystem>
implements Ev
      * @param  bean     the menu to keep synchronized with the list of reference systems.
      * @param  action   the user-specified action to execute when a reference system is selected.
      */
-    @SuppressWarnings("ThisEscapedInObjectConstruction")      // `this` is last and this
class is final.
-    MenuSync(final RecentReferenceSystems owner, final List<ReferenceSystem> systems,
+    MenuSync(final RecentReferenceSystems owner, final ObservableList<ReferenceSystem>
systems,
              final Menu bean, final ChangeListener<ReferenceSystem> action)
     {
         super(bean, "value");
@@ -95,11 +93,31 @@ final class MenuSync extends SimpleObjectProperty<ReferenceSystem>
implements Ev
         this.menus  = bean.getItems();
         this.group  = new ToggleGroup();
         this.action = action;
+        /*
+         * We do not register listener for `systems` list.
+         * Instead `notifyChanges(…)` will be invoked directly by RecentReferenceSystems.
+         */
         final MenuItem[] items = new MenuItem[systems.size()];
         for (int i=0; i<items.length; i++) {
             items[i] = createItem(systems.get(i));
         }
         menus.setAll(items);
+        initialize(systems);
+    }
+
+    /**
+     * Sets the initial value to the first item in the {@code systems} list, if any.
+     * This method is invoked in JavaFX thread at construction time or, if it didn't
+     * work at some later time when the systems list may contain an element.
+     * This method should not be invoked anymore after initialization succeeded.
+     */
+    private void initialize(final ObservableList<? extends ReferenceSystem> systems)
{
+        for (final ReferenceSystem system : systems) {
+            if (system != RecentReferenceSystems.OTHER) {
+                set(system);
+                break;
+            }
+        }
     }
 
     /**
@@ -176,6 +194,12 @@ final class MenuSync extends SimpleObjectProperty<ReferenceSystem>
implements Ev
             dispose(recycle.next());
         }
         GUIUtilities.copyAsDiff(Arrays.asList(items), menus);
+        /*
+         * If we had no previously selected item, selects it now.
+         */
+        if (get() == null) {
+            initialize(systems);
+        }
     }
 
     /**
@@ -191,7 +215,6 @@ final class MenuSync extends SimpleObjectProperty<ReferenceSystem>
implements Ev
         // ClassCastException should not happen because this listener is registered only
on MenuItem.
         final Object value = ((MenuItem) event.getSource()).getProperties().get(REFERENCE_SYSTEM_KEY);
         ReferenceSystem crs = (value == CHOOSER) ? RecentReferenceSystems.OTHER : (ReferenceSystem)
value;
-        action.changed(this, get(), crs);
         if (crs != RecentReferenceSystems.OTHER) {
             set(crs);
         }
@@ -203,13 +226,18 @@ final class MenuSync extends SimpleObjectProperty<ReferenceSystem>
implements Ev
      */
     @Override
     public void set(final ReferenceSystem system) {
-        super.set(system);
-        for (final MenuItem item : menus) {
-            if (item instanceof RadioMenuItem && item.getProperties().get(REFERENCE_SYSTEM_KEY)
== system) {
-                ((RadioMenuItem) item).setSelected(true);
-                return;
+        final ReferenceSystem old = get();
+        if (old != system) {
+            super.set(system);
+            for (final MenuItem item : menus) {
+                if (item instanceof RadioMenuItem && item.getProperties().get(REFERENCE_SYSTEM_KEY)
== system) {
+                    ((RadioMenuItem) item).setSelected(true);
+                    action.changed(this, old, system);
+                    return;
+                }
             }
+            group.selectToggle(null);
+            action.changed(this, old, null);
         }
-        group.selectToggle(null);
     }
 }
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/RecentReferenceSystems.java
b/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/RecentReferenceSystems.java
index 6355f7a..f6ae41d 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/RecentReferenceSystems.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/referencing/RecentReferenceSystems.java
@@ -524,20 +524,19 @@ public class RecentReferenceSystems {
      *                  {@code systems} list has been computed.
      */
     private void setReferenceSystems(final List<ReferenceSystem> systems, final ComparisonMode
mode) {
-        if (systems != null) {
+        if (systems != null) try {
+            isAdjusting = true;
             /*
-             * The call to `copyAsDiff(…)` may cause `ChoiceBox` values to be lost if the
corresponding item
-             * in the `referenceSystems` list is temporarily removed (before to be inserted
elsewhere).
-             * Save the values before to modify the list.
+             * The call to `copyAsDiff(…)` may cause some ChoiceBox values to be lost if
the corresponding
+             * item in the `referenceSystems` list is temporarily removed before to be inserted
elsewhere.
+             * Save the values before to modify the list. Note that if `referenceSystems`
was empty before
+             * the copy, `controlValues` should be null before the copy but may become non-null
after the
+             * copy because listeners will have initialized them to the first `ReferenceSystem`
available.
+             * Those non-null values will not be reflected in the `values` array, so we should
ignore them.
              */
             final ReferenceSystem[] values = controlValues.stream().map(WritableValue::getValue).toArray(ReferenceSystem[]::new);
-            try {
-                isAdjusting = true;
-                if (GUIUtilities.copyAsDiff(systems, referenceSystems)) {
-                    notifyChanges();
-                }
-            } finally {
-                isAdjusting = false;
+            if (GUIUtilities.copyAsDiff(systems, referenceSystems)) {
+                notifyChanges();
             }
             /*
              * Restore the previous selections. This code also serves another purpose: the
previous selection
@@ -549,15 +548,19 @@ public class RecentReferenceSystems {
             final int n = referenceSystems.size();
             for (int j=0; j<values.length; j++) {
                 ReferenceSystem system = values[j];
-                for (int i=0; i<n; i++) {
-                    final ReferenceSystem candidate = referenceSystems.get(i);
-                    if (Utilities.deepEquals(candidate, system, mode)) {
-                        system = candidate;
-                        break;
+                if (system != null) {                   // See comment about empty `referenceSystems`
list.
+                    for (int i=0; i<n; i++) {
+                        final ReferenceSystem candidate = referenceSystems.get(i);
+                        if (Utilities.deepEquals(candidate, system, mode)) {
+                            system = candidate;
+                            break;
+                        }
                     }
+                    controlValues.get(j).setValue(system);
                 }
-                controlValues.get(j).setValue(system);
             }
+        } finally {
+            isAdjusting = false;
         }
     }
 
@@ -581,6 +584,7 @@ public class RecentReferenceSystems {
                                       final ReferenceSystem oldValue, ReferenceSystem newValue)
         {
             if (isAdjusting) {
+                action.changed(property, oldValue, newValue);
                 return;
             }
             if (newValue == OTHER) {


Mime
View raw message