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: Do not fire "gridGeometry" property change anymore and add documentation explaining the policy.
Date Tue, 11 Feb 2020 12:21:48 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 ffe449c  Do not fire "gridGeometry" property change anymore and add documentation
explaining the policy.
ffe449c is described below

commit ffe449c15101b142d00a58aa0fbb7da65f2c2534
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Tue Feb 11 11:43:20 2020 +0100

    Do not fire "gridGeometry" property change anymore and add documentation explaining the
policy.
---
 .../java/org/apache/sis/internal/map/Canvas.java   | 93 ++++++++++++++--------
 1 file changed, 62 insertions(+), 31 deletions(-)

diff --git a/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/Canvas.java b/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/Canvas.java
index 1613ba3..b049d2d 100644
--- a/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/Canvas.java
+++ b/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/Canvas.java
@@ -199,17 +199,21 @@ public class Canvas extends Observable implements Localized {
     public static final String POINT_OF_INTEREST_PROPERTY = "pointOfInterest";
 
     /**
-     * The {@value} property name, used for notifications about changes in grid geometry.
+     * The {@value} property name.
      * The grid geometry is a synthetic property computed from other properties when requested.
      * The computed grid geometry may change every time that a {@value #OBJECTIVE_CRS_PROPERTY},
      * {@value #OBJECTIVE_TO_DISPLAY_PROPERTY}, {@value #DISPLAY_BOUNDS_PROPERTY} or
-     * {@value #POINT_OF_INTEREST_PROPERTY} property is changed, but a {@value} change event
-     * is send only when {@link #setGridGeometry(GridGeometry)} is explicitly invoked.
+     * {@value #POINT_OF_INTEREST_PROPERTY} property is changed. We do not (at this time)
fire
+     * {@value} change events because computing a new grid geometry for every changes of
above-cited
+     * properties would be costly. An alternative approach could be to fire {@value} event
only when
+     * {@link #setGridGeometry(GridGeometry)} is explicitly invoked, but it could be misleading
if
+     * it gives the false impression that the grid geometry did not changed because a listener
did
+     * not received an {@value} event.
      *
      * @see #getGridGeometry()
      * @see #setGridGeometry(GridGeometry)
      */
-    public static final String GRID_GEOMETRY_PROPERTY = "gridGeometry";
+    private static final String GRID_GEOMETRY_PROPERTY = "gridGeometry";
 
     /**
      * The coordinate reference system in which to transform all data before displaying.
@@ -729,18 +733,22 @@ public class Canvas extends Observable implements Localized {
     /**
      * Returns canvas properties (CRS, display bounds, conversion) encapsulated in a grid
geometry.
      * This is a convenience method for interoperability with grid coverage API.
-     * The set of {@link GridGeometry} dimensions includes all the dimensions of the objective
CRS,
+     * If {@link #setGridGeometry(GridGeometry)} has been invoked with a non-null value and
no other
+     * {@code Canvas} property changed since that method call, then this method returns that
value.
+     * Otherwise this method computes a grid geometry as described below.
+     *
+     * <p>The set of {@link GridGeometry} dimensions includes all the dimensions of
the objective CRS,
      * augmented with all (if possible) or some supplemental dimensions found in the point
of interest.
      * For example if the canvas manages only (<var>x</var>,<var>y</var>)
coordinates but the point of
      * interest includes also a <var>t</var> coordinate, then a third dimension
(which we call the
      * <cite>supplemental dimension</cite>) for <var>t</var> is added
to the CRS, {@link GridExtent}
-     * and "grid to CRS" transform of the returned grid geometry.
+     * and "grid to CRS" transform of the returned grid geometry.</p>
      *
      * <table>
      *   <caption>Canvas properties → grid geometry properties</caption>
      *   <tr>
      *     <th>Grid geometry element</th>
-     *     <th>Base dimensions</th>
+     *     <th>Display dimensions</th>
      *     <th>Supplemental dimensions</th>
      *   </tr><tr>
      *     <td>{@link GridGeometry#getCoordinateReferenceSystem()}</td>
@@ -748,12 +756,12 @@ public class Canvas extends Observable implements Localized {
      *     <td>Some of <code>{@linkplain #getPointOfInterest()}.getCoordinateReferenceSystem()</code></td>
      *   </tr><tr>
      *     <td>{@link GridGeometry#getExtent()}</td>
-     *     <td>{@link #getDisplayBounds()} rounded to enclosing integers</td>
+     *     <td>{@link #getDisplayBounds()} rounded to enclosing (floor and ceil) integers</td>
      *     <td>[0 … 0]</td>
      *   </tr><tr>
      *     <td>{@link GridGeometry#getGridToCRS(PixelInCell)}</td>
      *     <td>Inverse of {@link #getObjectiveToDisplay()}</td>
-     *     <td>Some {@linkplain #getPointOfInterest() point of interest} coordinates
as translation term.</td>
+     *     <td>Some {@linkplain #getPointOfInterest() point of interest} coordinates
as translation terms</td>
      *   </tr>
      * </table>
      *
@@ -763,8 +771,6 @@ public class Canvas extends Observable implements Localized {
      *
      * @return a grid geometry encapsulating canvas properties, including supplemental dimensions
if possible.
      * @throws RenderException if the grid geometry can not be computed.
-     *
-     * @see #GRID_GEOMETRY_PROPERTY
      */
     public GridGeometry getGridGeometry() throws RenderException {
         if (gridGeometry == null) try {
@@ -838,11 +844,16 @@ public class Canvas extends Observable implements Localized {
      * Sets canvas properties from the given grid geometry. This convenience method converts
the
      * coordinate reference system, "grid to CRS" transform and extent of the given grid
geometry
      * to {@code Canvas} properties. If the given value is different than the previous value,
then
-     * change events are sent to all listeners registered for the {@value #GRID_GEOMETRY_PROPERTY}
-     * property, with also potential change events for {@value #OBJECTIVE_CRS_PROPERTY},
-     * {@value #OBJECTIVE_TO_DISPLAY_PROPERTY}, {@value #DISPLAY_BOUNDS_PROPERTY} and
+     * change events are sent to all listeners registered for the {@value #OBJECTIVE_CRS_PROPERTY},
+     * {@value #OBJECTIVE_TO_DISPLAY_PROPERTY}, {@value #DISPLAY_BOUNDS_PROPERTY} and/or
      * {@value #POINT_OF_INTEREST_PROPERTY} properties.
      *
+     * <p>The value given to this method will be returned by {@link #getGridGeometry()}
as long as
+     * none of above cited properties is changed. If one of those properties changes (for
example
+     * if the user zooms or pans the map), then a new grid geometry will be computed. There
is no
+     * guarantees that the recomputed grid geometry will be similar to the grid geometry
specified
+     * to this method. For example the {@link GridExtent} in supplemental dimensions may
be different.</p>
+     *
      * @param  newValue  the grid geometry from which to get new canvas properties.
      * @throws RenderException if the given grid geometry can not be converted to canvas
properties.
      */
@@ -885,39 +896,59 @@ public class Canvas extends Observable implements Localized {
              */
             final TransformSeparator analyzer = new TransformSeparator(gridToCRS, coordinateOperationFactory.getMathTransformFactory());
             analyzer.addSourceDimensions(displayDimensions);
-            final LinearTransform           newObjToDisplay     = MathTransforms.tangent(analyzer.separate().inverse(),
newPOI);
-            final int[]                     objectiveDimensions = analyzer.getTargetDimensions();
-            final CoordinateReferenceSystem newObjectiveCRS     = CRS.reduce(crs, objectiveDimensions);
-            final MathTransform             dimensionSelect     = MathTransforms.linear(
+            final LinearTransform           newObjectiveToDisplay = MathTransforms.tangent(analyzer.separate().inverse(),
newPOI);
+            final int[]                     objectiveDimensions   = analyzer.getTargetDimensions();
+            final CoordinateReferenceSystem newObjectiveCRS       = CRS.reduce(crs, objectiveDimensions);
+            final MathTransform             dimensionSelect       = MathTransforms.linear(
                     Matrices.createDimensionSelect(newPOI.getDimension(), objectiveDimensions));
             /*
+             * At this point we are ready to commit the new values. Before doing so, copy
+             * the current property values in order to provide the old values to listeners.
+             */
+            final GeneralEnvelope           oldBounds             = new GeneralEnvelope(displayBounds);
+            final DirectPosition            oldPOI                = pointOfInterest;
+            final LinearTransform           oldObjectiveToDisplay = objectiveToDisplay;
+            final CoordinateReferenceSystem oldObjectiveCRS       = objectiveCRS;
+            /*
              * Set internal fields only after we successfully computed everything, in order
to have a
-             * "all or nothing" behavior. Notify listeners only after all properties have
been updated.
+             * "all or nothing" behavior.
              */
-            final GeneralEnvelope           oldBounds       = new GeneralEnvelope(displayBounds);
-            final DirectPosition            oldPOI          = pointOfInterest;
-            final LinearTransform           oldObjToDisplay = objectiveToDisplay;
-            final CoordinateReferenceSystem oldObjectiveCRS = objectiveCRS;
-            final GridGeometry              oldGrid         = gridGeometry;
-
             displayBounds.setEnvelope(newBounds);
             pointOfInterest       = newPOI;
-            objectiveToDisplay    = newObjToDisplay;
+            objectiveToDisplay    = newObjectiveToDisplay;
             objectiveCRS          = newObjectiveCRS;
             multidimToObjective   = dimensionSelect;
             augmentedObjectiveCRS = null;               // Will be recomputed when first
needed.
             axisTypes             = null;
             gridGeometry          = newValue;
-            if (!newBounds      .equals(oldBounds))       firePropertyChange(DISPLAY_BOUNDS_PROPERTY,
      oldBounds,       newBounds);
-            if (!newObjectiveCRS.equals(oldObjectiveCRS)) firePropertyChange(OBJECTIVE_CRS_PROPERTY,
       oldObjectiveCRS, newObjectiveCRS);
-            if (!newObjToDisplay.equals(oldObjToDisplay)) firePropertyChange(OBJECTIVE_TO_DISPLAY_PROPERTY,
oldObjToDisplay, newObjToDisplay);
-            if (!newPOI         .equals(oldPOI))          firePropertyChange(POINT_OF_INTEREST_PROPERTY,
   oldPOI,          newPOI);
-            /* Unconditional notification. */             firePropertyChange(GRID_GEOMETRY_PROPERTY,
       oldGrid,         newValue);
+            /*
+             * Notify listeners only after all properties have been updated. If a listener
throws an exception,
+             * other listeners will not be notified but this Canvas will not be corrupted
since all the work to
+             * do in this class is already completed.
+             */
+            fireIfChanged(DISPLAY_BOUNDS_PROPERTY,       oldBounds,             newBounds);
+            fireIfChanged(OBJECTIVE_CRS_PROPERTY,        oldObjectiveCRS,       newObjectiveCRS);
+            fireIfChanged(OBJECTIVE_TO_DISPLAY_PROPERTY, oldObjectiveToDisplay, newObjectiveToDisplay);
+            fireIfChanged(POINT_OF_INTEREST_PROPERTY,    oldPOI,                newPOI);
         } catch (IncompleteGridGeometryException | CannotEvaluateException | FactoryException
| TransformException e) {
             throw new RenderException(errors().getString(Errors.Keys.CanNotSetPropertyValue_1,
GRID_GEOMETRY_PROPERTY), e);
         }
     }
 
+    /**
+     * Fires a property change event if the old and new values are not equal.
+     * This method assumes that the new value is never null (but the old value can be null).
+     *
+     * @param  propertyName  name of the property that changed its value.
+     * @param  oldValue      the old property value (may be {@code null}).
+     * @param  newValue      the new property value, shall not be {@code null}.
+     */
+    private void fireIfChanged(final String propertyName, final Object oldValue, final Object
newValue) {
+        if (!newValue.equals(oldValue)) {
+            firePropertyChange(propertyName, oldValue, newValue);
+        }
+    }
+
     public Optional<GeographicBoundingBox> getGeographicArea() {
         return Optional.empty();        // TODO
     }


Mime
View raw message