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 other CRS selection problems (wrong CRS checked in menu items, "Other…" not working). This commit fixes the last problems we have seen so far.
Date Sat, 25 Apr 2020 10:21:19 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 6404696  Fix other CRS selection problems (wrong CRS checked in menu items, "Other…"
not working). This commit fixes the last problems we have seen so far.
6404696 is described below

commit 64046968baa6d21b4d5f4b229795b1f75537a556
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Sat Apr 25 12:19:13 2020 +0200

    Fix other CRS selection problems (wrong CRS checked in menu items, "Other…" not working).
    This commit fixes the last problems we have seen so far.
---
 .../java/org/apache/sis/gui/map/StatusBar.java     | 147 +++++++++++++++------
 .../org/apache/sis/gui/referencing/MenuSync.java   |  14 +-
 .../gui/referencing/RecentReferenceSystems.java    |  56 +++++++-
 3 files changed, 170 insertions(+), 47 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 3cf8192..c418b5a 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
@@ -39,6 +39,7 @@ import javafx.beans.value.ObservableValue;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ChangeListener;
+import javafx.collections.ListChangeListener;
 import javafx.concurrent.Task;
 import org.opengis.geometry.Envelope;
 import org.opengis.geometry.MismatchedDimensionException;
@@ -66,11 +67,11 @@ import org.apache.sis.util.Classes;
 import org.apache.sis.util.Utilities;
 import org.apache.sis.util.Exceptions;
 import org.apache.sis.util.ArgumentChecks;
+import org.apache.sis.util.ComparisonMode;
 import org.apache.sis.util.logging.Logging;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.gui.Widget;
 import org.apache.sis.gui.referencing.RecentReferenceSystems;
-import org.apache.sis.internal.referencing.ReferencingUtilities;
 import org.apache.sis.internal.gui.BackgroundThreads;
 import org.apache.sis.internal.gui.ExceptionReporter;
 import org.apache.sis.internal.gui.Resources;
@@ -137,11 +138,16 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
     private double lastX, lastY;
 
     /**
-     * The area of interest, or {@code null} if none. Used for computing {@link #objectiveToFormatCRS}.
-     * This field is a reference to the {@link RecentReferenceSystems#areaOfInterest} property.
-     * We do not make this property public because it does not belong to this object.
+     * The manager of reference systems chosen by user, or {@code null} if none.
+     * The {@link RecentReferenceSystems#areaOfInterest} property is used for
+     * computing {@link #objectiveToFormatCRS}.
      */
-    private final ObjectProperty<Envelope> areaOfInterest;
+    private final RecentReferenceSystems systemChooser;
+
+    /**
+     * The selected reference system, or {@code null} if there is no such property.
+     */
+    private final ObjectProperty<ReferenceSystem> selectedSystem;
 
     /**
      * The reference system used for rendering the data for which this status bar is providing
cursor coordinates.
@@ -206,6 +212,7 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
      *
      * @see #sourceCoordinates
      * @see #position
+     * @see #setTargetCRS(CoordinateReferenceSystem)
      */
     private GeneralDirectPosition targetCoordinates;
 
@@ -298,13 +305,37 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
         position.setTextAlignment(TextAlignment.RIGHT);
         canvasProperty = new SimpleObjectProperty<>(this, "canvas");
         canvasProperty.addListener(this::onCanvasSpecified);
+        this.systemChooser = systemChooser;
         if (systemChooser == null) {
-            areaOfInterest = null;
+            selectedSystem = null;
         } else {
-            areaOfInterest = systemChooser.areaOfInterest;
+            /*
+             * Ensure (as much as possible) that the CRS of coordinates formatted in this
status bar is one
+             * of the CRSs in the list of choices offered to the user.  It happens often
that the CRS given
+             * to `applyCanvasGeometry(GridGeometry)` has (λ,φ) axis order but the CRS
offered to user have
+             * (φ,λ) axis order (because we try to comply with definitions following geographers
practice).
+             * In such case we will replace (λ,φ) by (φ,λ). Since we use the list of
choices as the source
+             * of desired CRS, we have to listen to new elements added to that list. This
is necessary since
+             * the list of often empty at construction time and filled later after a background
thread task.
+             */
+            systemChooser.getItems().addListener((ListChangeListener.Change<? extends
ReferenceSystem> change) -> {
+                while (change.next()) {
+                    if (change.wasAdded() || change.wasReplaced()) {
+                        setReplaceableTargetCRS(format.getDefaultCRS());
+                        break;
+                    }
+                }
+            });
+            /*
+             * Create a contextual menu offering to user a choice of CRS in which to display
the coordinates.
+             * The CRS choices are controlled by `RecentReferenceSystems`. Selection of a
new CRS causes the
+             * `setTargetCRS(…)` method to be invoked. Contextual menu can be invoked anywhere
on the HBox;
+             * we do not register this menu to `position` only because it is a relatively
small area.
+             */
             final Menu choices = systemChooser.createMenuItems((property, oldValue, newValue)
-> {
-                findFormatOperation(newValue instanceof CoordinateReferenceSystem ? (CoordinateReferenceSystem)
newValue : null);
+                setTargetCRS(newValue instanceof CoordinateReferenceSystem ? (CoordinateReferenceSystem)
newValue : null);
             });
+            selectedSystem = RecentReferenceSystems.getSelectedProperty(choices);
             final ContextMenu menu = new ContextMenu(choices);
             view.setOnMousePressed((event) -> {
                 if (event.isSecondaryButtonDown()) {
@@ -411,7 +442,10 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
         {
             if (!value) try {
                 apply(getCanvas().getGridGeometry());
-                reformat();
+                /*
+                 * Do not hide `position` since we assume that "real world" coordinates are
still valid.
+                 * Do not try to rewrite position neither since `lastX` and `lastY` are not
valid anymore.
+                 */
             } catch (RenderException e) {
                 setErrorMessage(null, e);
             }
@@ -526,21 +560,15 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
         inflatePrecisions   = inflate;
         precisions          = null;
         lastX = lastY       = Double.NaN;                           // Not valid anymove
— see above block comment.
-        CoordinateReferenceSystem restore = null;
         if (sameCRS) {
-            if (objectiveToFormatCRS != null) {
-                localToFormatCRS = MathTransforms.concatenate(localToCRS, objectiveToFormatCRS.getMathTransform());
-            }
+            updateLocalToFormatCRS();
             // Keep the format CRS unchanged since we made `localToFormatCRS` consistent
with its value.
         } else {
             objectiveToFormatCRS = null;
-            restore = format.getDefaultCRS();           // CRS to restore in a background
thread.
-            setFormatCRS(crs);                          // Should be invoked before to set
precision.
+            setFormatCRS(crs);                                      // Should be invoked
before to set precision.
+            crs = setReplaceableTargetCRS(crs);                     // May invoke later setFormatCRS(…)
again.
         }
         format.setGroundPrecision(Quantities.create(resolution, unit));
-        if (ReferencingUtilities.getDimension(restore) == localToFormatCRS.getTargetDimensions())
{
-            findFormatOperation(restore);               // Not executed if `restore` is null.
-        }
         /*
          * If this is the first time that this method is invoked after `setCanvas(MapCanvas)`,
          * the listeners are not yet registered and should be added now. Listeners registration
@@ -550,20 +578,67 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
         if (geometry != null && !isMouseListenerRegistered) {
             registerMouseListeners(canvasProperty.getValue());
         }
+        /*
+         * If the CRS changed, we may need to update the selected menu item. It happens when
this method
+         * is invoked because new data have been loaded, as opposed to this method being
invoked after a
+         * gesture event (zoom, pan, rotation).
+         */
+        if (!sameCRS && selectedSystem != null) {
+            selectedSystem.set(crs);
+        }
+    }
+
+    /**
+     * Computes {@link #localToFormatCRS} after a change of {@link #localToObjectiveCRS}.
+     * Other properties, in particular {@link #objectiveToFormatCRS}, must be valid.
+     */
+    private void updateLocalToFormatCRS() {
+        if (objectiveToFormatCRS != null) {
+            localToFormatCRS = MathTransforms.concatenate(localToObjectiveCRS, objectiveToFormatCRS.getMathTransform());
+        }
+    }
+
+    /**
+     * Sets the CRS of the position shown in this status bar after replacement by one of
the available CRS
+     * if a match is found. This method compares the given CRS with the list of choices before
to delegate
+     * to {@link #setTargetCRS(CoordinateReferenceSystem)} possibly with different axis order.
The typical
+     * scenario is {@link #apply(GridGeometry)} invoked with (<var>longitude</var>,
<var>latitude</var>)
+     * axis order, and this method swapping axes to standard (<var>latitude</var>,
<var>longitude</var>)
+     * axis order for coordinates display purpose.
+     *
+     * @param  crs  the new CRS (ignoring axis order), or {@code null} for {@link #objectiveCRS}.
+     * @return the reference system actually used for formatting coordinates. It may have
different axis order
+     *         and units than the specified CRS. This is the CRS that {@link CoordinateFormat#getDefaultCRS()}
+     *         will return a little bit later (pending completion of a background task).
+     */
+    private CoordinateReferenceSystem setReplaceableTargetCRS(CoordinateReferenceSystem crs)
{
+        if (crs != null) {
+            final ComparisonMode mode = systemChooser.duplicationCriterion.get();
+            for (final ReferenceSystem system : systemChooser.getItems()) {
+                if (Utilities.deepEquals(crs, system, mode)) {
+                    crs = (CoordinateReferenceSystem) system;       // Same CRS but possibly
different axis order.
+                    break;
+                }
+            }
+        }
+        if (crs != format.getDefaultCRS()) {
+            setTargetCRS(crs);
+        }
+        return crs;
     }
 
     /**
-     * Sets the coordinate reference system of the coordinates shown in this status bar.
+     * Sets the coordinate reference system of the position shown in this status bar.
      * The change may not appear immediately after method return; this method may use a
      * background thread for computing the coordinate operation.  That task may be long
      * the first time that it is executed, but should be fast on subsequent invocations.
      *
      * @param  crs  the new CRS, or {@code null} for {@link #objectiveCRS}.
      */
-    private void findFormatOperation(final CoordinateReferenceSystem crs) {
+    private void setTargetCRS(final CoordinateReferenceSystem crs) {
         if (crs != null && objectiveCRS != null && objectiveCRS != crs) {
             position.setTextFill(Styles.OUTDATED_TEXT);
-            final Envelope aoi = (areaOfInterest != null) ? areaOfInterest.get() : null;
+            final Envelope aoi = (systemChooser != null) ? systemChooser.areaOfInterest.get()
: null;
             BackgroundThreads.execute(new Task<MathTransform>() {
                 /**
                  * The new {@link StatusBar#objectiveToFormatCRS} value if successful.
@@ -583,7 +658,7 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
                     } catch (TransformException e) {
                         bbox = null;
                         Logging.recoverableException(Logging.getLogger(Modules.APPLICATION),
-                                StatusBar.class, "findFormatOperation", e);
+                                StatusBar.class, "setTargetCRS", e);
                     }
                     operation = CRS.findOperation(objectiveCRS, crs, bbox);
                     return MathTransforms.concatenate(localToObjectiveCRS, operation.getMathTransform());
@@ -649,7 +724,14 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
         position.setTextFill(Styles.NORMAL_TEXT);
         position.setMinWidth(0);
         setErrorMessage(null, null);
-        reformat();
+        if (isPositionVisible()) {
+            final double x = lastX;
+            final double y = lastY;
+            lastX = lastY = Double.NaN;
+            if (!Double.isNaN(x) && !Double.isNaN(y)) {
+                setLocalCoordinates(x, y);
+            }
+        }
     }
 
     /**
@@ -707,23 +789,6 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
     }
 
     /**
-     * Reformats the coordinates shown in {@link #position} using current {@link #lastX}
and {@link #lastY} values.
-     * This method should be invoked only when the caller knows that those values are still
valid. Note that those
-     * values may be invalid if {@link javafx.scene.Node#getTransforms()} changed even if
{@link #objectiveCRS} is
-     * the same.
-     */
-    private void reformat() {
-        if (isPositionVisible()) {
-            final double x = lastX;
-            final double y = lastY;
-            lastX = lastY = Double.NaN;
-            if (!Double.isNaN(x) && !Double.isNaN(y)) {
-                setLocalCoordinates(x, y);
-            }
-        }
-    }
-
-    /**
      * Resets {@link #localToFormatCRS} to its default value. This is invoked either when
the
      * target CRS is {@link #objectiveCRS}, or when an attempt to use another CRS failed.
      */
@@ -795,7 +860,7 @@ public class StatusBar extends Widget implements EventHandler<MouseEvent>
{
             actual   = conversion.getTargetDimensions();
             if (expected == actual) {
                 localToObjectiveCRS = conversion;
-                findFormatOperation(format.getDefaultCRS());                    // Recompute
`localToFormatCRS`.
+                updateLocalToFormatCRS();
                 return;
             }
         }
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 f25f675..e6130da 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
@@ -214,15 +214,19 @@ final class MenuSync extends SimpleObjectProperty<ReferenceSystem>
implements Ev
     public void handle(final ActionEvent event) {
         // 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;
-        if (crs != RecentReferenceSystems.OTHER) {
-            set(crs);
+        if (value == CHOOSER) {
+            action.changed(this, get(), RecentReferenceSystems.OTHER);
+        } else {
+            set((ReferenceSystem) value);
         }
     }
 
     /**
-     * Selects the specified reference system. This method is invoked by {@link RecentReferenceSystems}
-     * when the selected CRS changed, either programmatically or by user action.
+     * Selects the specified reference system. This method is invoked by {@link RecentReferenceSystems}
when the
+     * selected CRS changed, either programmatically or by user action. User-specified {@link
#action} is invoked,
+     * which will typically start a background thread for transforming data. This method
does nothing if the given
+     * reference system is same as current one; this is important both for avoiding infinite
loop and for avoiding
+     * to invoke the potentially costly {@link #action}.
      */
     @Override
     public void set(final ReferenceSystem system) {
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 f6ae41d..91f90b6 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
@@ -19,6 +19,7 @@ package org.apache.sis.gui.referencing;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Locale;
+import java.util.Objects;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleObjectProperty;
 import javafx.beans.value.ChangeListener;
@@ -26,6 +27,7 @@ import javafx.beans.value.ObservableValue;
 import javafx.beans.value.WritableValue;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
+import javafx.collections.transformation.FilteredList;
 import javafx.scene.control.ChoiceBox;
 import javafx.scene.control.MenuItem;
 import javafx.scene.control.Menu;
@@ -91,8 +93,18 @@ public class RecentReferenceSystems {
     private static final int NUM_OTHER_ITEMS = 1;
 
     /**
+     * Key for use with the {@linkplain Menu#getProperties() property map} for storing the
selected item.
+     * Used for providing the functionality of {@link javafx.scene.control.CheckBox#selectedProperty()}
+     * on controls that do not have an explicit selected property.
+     */
+    private static final String SELECTED_ITEM_KEY = "SelectedItem";
+
+    /**
      * A pseudo-reference system for the "Other…" choice. We use a null value because {@link
ChoiceBox}
      * seems to insist for inserting a null value in the items list when we remove the selected
item.
+     *
+     * <div class="note"><b>Maintenance note:</b> if this field is changed
to a non-null value,
+     * search also for usages of {@code Object::nonNull} predicate.</div>
      */
     static final ReferenceSystem OTHER = null;
 
@@ -177,10 +189,19 @@ public class RecentReferenceSystems {
      * The {@link #systemsOrCodes} elements with all codes or wrappers replaced by {@link
ReferenceSystem}
      * instances and duplicated values removed. This is the list given to JavaFX controls
that we build.
      * This list includes {@link #OTHER} as its last item.
+     *
+     * @see #updateItems()
      */
     private ObservableList<ReferenceSystem> referenceSystems;
 
     /**
+     * A filtered view of {@link #referenceSystems} without the {@link #OTHER} item.
+     *
+     * @see #getItems()
+     */
+    private ObservableList<ReferenceSystem> filteredSystems;
+
+    /**
      * {@code true} if the {@link #referenceSystems} list needs to be rebuilt from {@link
#systemsOrCodes} content.
      * This field shall be read and modified in a block synchronized on {@link #systemsOrCodes}.
      *
@@ -689,6 +710,21 @@ public class RecentReferenceSystems {
     }
 
     /**
+     * Returns all reference systems in the order they appear in JavaFX controls. The first
element
+     * is the {@link #setPreferred(boolean, ReferenceSystem) preferred} (or native) reference
system.
+     * All other elements are {@linkplain #addAlternatives(boolean, ReferenceSystem...) alternatives}.
+     *
+     * @return all reference systems in the order they appear in JavaFX controls.
+     */
+    @SuppressWarnings("ReturnOfCollectionOrArrayField")
+    public ObservableList<ReferenceSystem> getItems() {
+        if (filteredSystems == null) {
+            filteredSystems = new FilteredList<>(updateItems(), Objects::nonNull);
+        }
+        return filteredSystems;
+    }
+
+    /**
      * Returns all currently selected reference systems in the order they appear in JavaFX
controls.
      * This method collects selected values of all controls created by a {@code createXXX(…)}
method.
      * The returned list does not contain duplicated values.
@@ -775,11 +811,29 @@ next:       for (int i=0; i<count; i++) {
     public Menu createMenuItems(final ChangeListener<ReferenceSystem> action) {
         ArgumentChecks.ensureNonNull("action", action);
         final Menu menu = new Menu(Resources.forLocale(locale).getString(Resources.Keys.ReferenceSystem));
-        controlValues.add(new MenuSync(this, updateItems(), menu, new Listener(action)));
+        final MenuSync property = new MenuSync(this, updateItems(), menu, new Listener(action));
+        menu.getProperties().put(SELECTED_ITEM_KEY, property);
+        controlValues.add(property);
         return menu;
     }
 
     /**
+     * Returns the property for the selected value in a menu created by {@link #createMenuItems(ChangeListener)}.
+     *
+     * @param  menu  the menu, or {@code null} if none.
+     * @return the property for the selected value, or {@code null} if none.
+     */
+    public static ObjectProperty<ReferenceSystem> getSelectedProperty(final Menu menu)
{
+        if (menu != null) {
+            final Object property = menu.getProperties().get(SELECTED_ITEM_KEY);
+            if (property instanceof MenuSync) {
+                return (MenuSync) property;
+            }
+        }
+        return null;
+    }
+
+    /**
      * Invoked when an error occurred while filtering a {@link ReferenceSystem} instance.
      * The error may be a failure to convert an EPSG code to a {@link CoordinateReferenceSystem}
instance,
      * or an error during a CRS verification. Some errors may be normal, for example because
EPSG dataset


Mime
View raw message