Fix a bug in the positionning of the image when some gestures are applied while an image is computed in background thread.
This bug became more visible after the support of Cloud Optimized GeoTIFF through HTTP because of network transfer delays.
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/CoverageCanvas.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/CoverageCanvas.java
index 7d4a375..07ecc32 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/CoverageCanvas.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/CoverageCanvas.java
@@ -728,7 +728,7 @@
* point all remaining points are executed:
*
* <ol>
- * <li>Read a new coverage if zoom as changed more than some threshold value.</li>
+ * <li>Read a new coverage if zoom has changed more than some threshold value.</li>
* <li>Compute statistics on sample values (if needed).</li>
* <li>Stretch the color ramp (if requested).</li>
* <li>Resample the image and convert to integer values.</li>
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvas.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvas.java
index 5f5b946..78c3761 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvas.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvas.java
@@ -63,6 +63,7 @@
import org.apache.sis.referencing.operation.transform.MathTransforms;
import org.apache.sis.referencing.operation.transform.LinearTransform;
import org.apache.sis.referencing.cs.CoordinateSystems;
+import org.apache.sis.referencing.IdentifiedObjects;
import org.apache.sis.geometry.DirectPosition2D;
import org.apache.sis.geometry.Envelope2D;
import org.apache.sis.geometry.AbstractEnvelope;
@@ -87,7 +88,6 @@
import org.apache.sis.portrayal.PlanarCanvas;
import org.apache.sis.portrayal.RenderException;
import org.apache.sis.portrayal.TransformChangeEvent;
-import org.apache.sis.referencing.IdentifiedObjects;
import static org.apache.sis.internal.gui.LogHandler.LOGGER;
import static org.apache.sis.internal.util.StandardDateFormat.NANOS_PER_MILLISECOND;
@@ -1051,7 +1051,7 @@
* </ol>
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.1
+ * @version 1.4
* @since 1.1
*/
protected abstract static class Renderer {
@@ -1272,7 +1272,7 @@
renderedContentStamp = contentChangeCount;
final Renderer context = createRenderer();
if (context != null && context.initialize(floatingPane)) {
- final Task<?> worker = createWorker(context);
+ final RenderingTask<?> worker = createWorker(context);
assert renderingInProgress == null : renderingInProgress;
BackgroundThreads.execute(worker);
renderingInProgress = worker; // Set after we know that the task has been scheduled.
@@ -1291,15 +1291,15 @@
/**
* Creates the background task which will invoke {@link Renderer#render()} in a background thread.
- * The tasks must invoke {@link #renderingCompleted(Task)} in JavaFX thread after completion,
+ * The tasks must invoke {@link #renderingCompleted(RenderingTask)} in JavaFX thread after completion,
* either successful or not.
*
* <p><b>Note:</b> it is important that no other worker is in progress at the time this method is invoked
* ({@code assert renderingInProgress == null}), otherwise conflicts may happen when workers will update
* the {@code MapCanvas} fields after they completed their task.</p>
*/
- Task<?> createWorker(final Renderer renderer) {
- return new Task<Void>() {
+ RenderingTask<?> createWorker(final Renderer renderer) {
+ return new RenderingTask<Void>() {
/** Invoked in background thread. */
@Override protected Void call() throws Exception {
renderer.render();
@@ -1327,8 +1327,28 @@
* which is now integrated in the map. That transform will be removed from {@link #floatingPane} transforms.
* The {@link #transform} result is identity if no zoom, rotation or pan gesture has been applied since last
* rendering.
+ *
+ * <h4>Use case</h4>
+ * <p>Suppose that the {@link RenderingTask} has been started in response to some user gestures.
+ * For example, the user has zoomed on the map. The renderer has been initialized with a snapshot
+ * of this {@code MapCanvas} state at the time when the {@link Renderer} has been constructed.
+ * From this state, the renderer infers which data region to load at which resolution.</p>
+ *
+ * <p>Suppose that the rendering takes a long time, and during that time the user continues to do
+ * zoom and pan gestures. {@code MapCanvas} records those gestures in an {@link Affine} transform
+ * and will apply those changes on the image created by the <em>previous</em> rendering.
+ * For example if the user did a pan gesture of 100 pixels while the {@link Renderer} was working,
+ * then after the renderer finished to produce an image, that new image will also be translated by
+ * 100 pixels and a new call to {@link #repaint()} will happen later.</p>
+ *
+ * <p>Suppose that a zoom-in gesture caused the {@link Rendeder} to paint an image having a resolution
+ * twice finer than the resolution used in previous rendering. If the user does a translation of 100 pixels
+ * while this rendering is in progress, that "100 pixels" measurement is in units of the old rendering.
+ * It will need to be converted to 200 pixels after the rendering completed.</p>
+ *
+ * @param task the background task which has been completed, successfully or not.
*/
- final void renderingCompleted(final Task<?> task) {
+ final void renderingCompleted(final RenderingTask<?> task) {
assert Platform.isFxApplicationThread();
assert renderingInProgress == task : "Expected " + renderingInProgress + " but was " + task;
// Keep cursor unchanged if contents changed, because caller will invoke `repaint()` again.
@@ -1336,15 +1356,30 @@
restoreCursorAfterPaint();
}
renderingInProgress = null;
+ /*
+ * Display coordinates stored in this `MapCanvas` need to be converted to the
+ * new display coordinates, as expected by the new "objective to display" CRS.
+ */
final Point2D p = changeInProgress.transform(xPanStart, yPanStart);
xPanStart = p.getX();
yPanStart = p.getY();
try {
changeInProgress.invert();
- transform.prepend(changeInProgress);
+ transform.append(changeInProgress);
+ /*
+ * Note: intuitively one may expect `prepend(…)` instead of `append(…)` above.
+ * The use of `prepend(…)` would give a `transform` result which would be as if
+ * the transform was the identity transform at the time that rendering started,
+ * and all operations on it are gesture events that occurred while the renderer
+ * was working in background. But actually this is not quite correct.
+ * See the zoom-in discussion in "use case" section in method javadoc.
+ */
} catch (NonInvertibleTransformException e) {
unexpectedException("repaint", e);
}
+ /*
+ * At this point the rendering is completed. If some error occurred, report them.
+ */
isRendering.set(false);
final Throwable ex = task.getException();
if (ex != null) {
@@ -1372,11 +1407,11 @@
* This is especially useful when the first gesture event is a tiny change because the user just started
* panning or zooming.
*
- * <div class="note"><b>Design note:</b>
+ * <h4>Design note:</h4>
* using a thread for waiting seems a waste of resources, but a thread (likely this one) is going to be used
* for real after the waiting time is elapsed. That thread usually exists anyway in {@link BackgroundThreads}
* as an idle thread, and it is unlikely that other parts of this JavaFX application need that thread in same
- * time (if it happens, other threads will be created).</div>
+ * time (if it happens, other threads will be created).
*
* @see #requestRepaint()
*/
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvasAWT.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvasAWT.java
index ee90198..da79c83 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvasAWT.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/MapCanvasAWT.java
@@ -40,7 +40,6 @@
import javafx.scene.image.WritableImage;
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleObjectProperty;
-import javafx.concurrent.Task;
import javafx.util.Callback;
import org.apache.sis.internal.coverage.j2d.ColorModelFactory;
import org.apache.sis.internal.system.Configuration;
@@ -54,7 +53,7 @@
* controls by the user.
*
* @author Martin Desruisseaux (Geomatys)
- * @version 1.3
+ * @version 1.4
* @since 1.1
*/
public abstract class MapCanvasAWT extends MapCanvas {
@@ -354,7 +353,7 @@
* @see #requestRepaint()
*/
@Override
- final Task<?> createWorker(final MapCanvas.Renderer mc) {
+ final RenderingTask<?> createWorker(final MapCanvas.Renderer mc) {
assert Platform.isFxApplicationThread();
final Renderer context = (Renderer) mc;
if (!context.isValid(imageMargin.get(), buffer)) {
@@ -370,7 +369,7 @@
* previous resources that we can recycle, either because they have never been created yet or because
* they are not suitable anymore (for example because the image size changed).
*/
- private final class Creator extends Task<WritableImage> {
+ private final class Creator extends RenderingTask<WritableImage> {
/**
* The user-provided object which will perform the actual rendering.
* Its {@link Renderer#paint(Graphics2D)} method will be invoked in background thread.
@@ -467,7 +466,7 @@
* The Java2D volatile image will be rendered in background thread, then its content will be
* transferred to JavaFX image (through {@link BufferedImage} shared array) in JavaFX thread.
*/
- private final class Updater extends Task<VolatileImage> implements Callback<PixelBuffer<IntBuffer>, Rectangle2D> {
+ private final class Updater extends RenderingTask<VolatileImage> implements Callback<PixelBuffer<IntBuffer>, Rectangle2D> {
/**
* The user-provided object which will perform the actual rendering.
* Its {@link Renderer#paint(Graphics2D)} method will be invoked in background thread.
diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/map/RenderingTask.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/RenderingTask.java
new file mode 100644
index 0000000..140a642
--- /dev/null
+++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/map/RenderingTask.java
@@ -0,0 +1,42 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.sis.gui.map;
+
+import javafx.concurrent.Task;
+
+
+/**
+ * Base class of tasks executed in background thread for doing rendering.
+ * This is currently used only for type safety.
+ *
+ * @author Martin Desruisseaux (Geomatys)
+ * @version 1.4
+ *
+ * @param <V> type of value computed by the task.
+ *
+ * @see MapCanvas.Renderer
+ * @see MapCanvas#renderingCompleted(RenderingTask)
+ *
+ * @since 1.4
+ */
+abstract class RenderingTask<V> extends Task<V> {
+ /**
+ * Creates a new rendering task.
+ */
+ RenderingTask() {
+ }
+}
diff --git a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridExtent.java b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridExtent.java
index 35c0f58..a023268 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridExtent.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/coverage/grid/GridExtent.java
@@ -830,7 +830,7 @@
* The default implementation returns the median (or center) coordinates of this grid extent,
* but subclasses can override this method if another point is considered more representative.
*
- * <p>The {@code anchpr} argument tells {@linkplain GridGeometry#getGridToCRS(PixelInCell) which transform}
+ * <p>The {@code anchor} argument tells {@linkplain GridGeometry#getGridToCRS(PixelInCell) which transform}
* the caller intend to use for converting the grid coordinates to "real world" coordinates.
* With the default implementation, the coordinate values returned with {@code CELL_CORNER}
* are 0.5 cell units higher than the coordinate values returned with {@code CELL_CENTER}.
diff --git a/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/coverage/RenderingData.java b/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/coverage/RenderingData.java
index f239934..b02ccdb 100644
--- a/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/coverage/RenderingData.java
+++ b/core/sis-portrayal/src/main/java/org/apache/sis/internal/map/coverage/RenderingData.java
@@ -136,6 +136,7 @@
/**
* The pyramid level of {@linkplain #data} loaded by the {@linkplain #coverageLoader}.
+ * Value 0 is finest resolution.
*/
private int currentPyramidLevel;
@@ -393,7 +394,7 @@
domain = r.getImageGeometry(BIDIMENSIONAL);
xyDims = r.getXYDimensions();
}
- setImageSpace(domain, ranges, xyDims);
+ setImageSpace(domain, ranges, xyDims); // Implies `dataGeometry = domain`.
currentSlice = sliceExtent;
data = image;
/*
@@ -420,12 +421,32 @@
toOld = toNew.inverse();
}
}
- final MathTransform forward = concatenate(PixelInCell.CELL_CORNER, dataGeometry, old, toOld);
- final MathTransform inverse = concatenate(PixelInCell.CELL_CENTER, old, dataGeometry, toNew);
- cornerToObjective = MathTransforms.concatenate(forward, cornerToObjective);
- objectiveToCenter = MathTransforms.concatenate(objectiveToCenter, inverse);
+ /*
+ * `inverse` is the transform from new grid coordinates to old grid coordinates.
+ * `forward` is the converse, with the addition of half-pixel translation terms.
+ */
+ final MathTransform inverse = concatenate(PixelInCell.CELL_CORNER, dataGeometry, old, toOld);
+ final MathTransform forward = concatenate(PixelInCell.CELL_CENTER, old, dataGeometry, toNew);
+ cornerToObjective = MathTransforms.concatenate(inverse, cornerToObjective);
+ objectiveToCenter = MathTransforms.concatenate(objectiveToCenter, forward);
}
return true;
+ /*
+ * Note: the `forward` transform above is of particular interest and may be returned in a future version.
+ * It is the transform from new pixel coordinates to old pixel coordinates of the data before resampling
+ * (i.e. ignoring changes caused by user's zoom or pan gestures on the map). Typical values are:
+ *
+ * • An identity transform, meaning that the data changed but the new data uses the same pixel coordinates
+ * than the previous data. For example the user may have selected a new slice in a three-dimensional cube.
+ * • An affine transform represented by a diagonal matrix, i.e. with only scale factors and no translation.
+ * It happens when there is a change of resolution between the previous data and the new one, for example
+ * because a zoom change caused a change of pyramid level in `MultiResolutionCoverageLoader`.
+ * In such case the scale factors are typically 0.5 (after zoom-in) or 2 (after zoom out).
+ *
+ * That transform has already been applied to `RenderingData` internal state,
+ * but maybe some caller will need to apply that change to its own data.
+ * We wait to see if such need happens.
+ */
}
/**
