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 <[email protected]>
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
}