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 294b39e Stretch color ramp (if requested) before to resample the image, for simplifying `CoverageCanvas` a little bit. This is an apparent waste of CPU time because the resample operation should not need to be redone when only the ColorModel has changed, but `ImageProcessor` is capable to detect and optimize this case by itself. 294b39e is described below commit 294b39e912af5a5b0b2ce3496e5fc3a283b9a392 Author: Martin Desruisseaux AuthorDate: Wed Aug 5 00:59:43 2020 +0200 Stretch color ramp (if requested) before to resample the image, for simplifying `CoverageCanvas` a little bit. This is an apparent waste of CPU time because the resample operation should not need to be redone when only the ColorModel has changed, but `ImageProcessor` is capable to detect and optimize this case by itself. --- .../apache/sis/gui/coverage/CoverageCanvas.java | 101 ++++++++++++--------- .../sis/gui/coverage/ImagePropertyExplorer.java | 2 +- .../org/apache/sis/gui/coverage/RenderingData.java | 71 ++++++++------- 3 files changed, 99 insertions(+), 75 deletions(-) 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 6dd0398..42ce921 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 @@ -118,10 +118,17 @@ public class CoverageCanvas extends MapCanvasAWT { private RenderingData data; /** - * The {@link #data} resampled to a CRS which can easily be mapped to {@linkplain #getDisplayCRS() display CRS}. - * The different values are variants with color ramp changed. + * The {@link #data} with different operations applied on them. Currently the only supported operation is + * color ramp stretching. The coordinate system is the one of the original image (no resampling applied). */ - private final Map resampledImages; + private final Map derivedImages; + + /** + * Image resampled to a CRS which can easily be mapped to the {@linkplain #getDisplayCRS() display CRS}. + * May also include conversion to integer values for usage with index color model. + * This is the image which will be drawn in the canvas. + */ + private RenderedImage resampledImage; /** * The explorer to notify when the image shown in this canvas has changed. @@ -163,7 +170,7 @@ public class CoverageCanvas extends MapCanvasAWT { CoverageCanvas(final Locale locale) { super(locale); data = new RenderingData(); - resampledImages = new EnumMap<>(Stretching.class); + derivedImages = new EnumMap<>(Stretching.class); coverageProperty = new SimpleObjectProperty<>(this, "coverage"); sliceExtentProperty = new SimpleObjectProperty<>(this, "sliceExtent"); interpolationProperty = new SimpleObjectProperty<>(this, "interpolation", data.processor.getInterpolation()); @@ -175,12 +182,12 @@ public class CoverageCanvas extends MapCanvasAWT { /** * Completes initialization of this canvas for use with the returned property explorer. * The intent is to be notified when the image used for showing the coverage changed. - * This method may be removed in a future SIS version if we revisit this API before - * to make public. + * This method is invoked the first time that the "Properties" section in `CoverageControls` + * is being shown. */ final ImagePropertyExplorer createPropertyExplorer() { imageProperty = new ImagePropertyExplorer(getLocale(), fixedPane.backgroundProperty()); - imageProperty.setImage(resampledImages.get(data.selectedDerivative), getVisibleImageBounds()); + imageProperty.setImage(resampledImage, getVisibleImageBounds()); return imageProperty; } @@ -252,6 +259,7 @@ public class CoverageCanvas extends MapCanvasAWT { * @see GridCoverage#render(GridExtent) */ public final void setSliceExtent(final GridExtent sliceExtent) { + assert Platform.isFxApplicationThread(); sliceExtentProperty.set(sliceExtent); } @@ -274,6 +282,7 @@ public class CoverageCanvas extends MapCanvasAWT { * @see #interpolationProperty */ public final void setInterpolation(final Interpolation interpolation) { + assert Platform.isFxApplicationThread(); interpolationProperty.set(interpolation); } @@ -389,7 +398,8 @@ public class CoverageCanvas extends MapCanvasAWT { *

All arguments can be {@code null} for clearing the canvas.

*/ private void setRawImage(final RenderedImage image, final GridGeometry domain, final List ranges) { - resampledImages.clear(); + resampledImage = null; + derivedImages.clear(); data.setImage(image, domain, ranges); Envelope bounds = null; if (domain != null && domain.isDefined(GridGeometry.ENVELOPE)) { @@ -404,7 +414,7 @@ public class CoverageCanvas extends MapCanvasAWT { */ private void onInterpolationSpecified(final Interpolation newValue) { data.processor.setInterpolation(newValue); - resampledImages.clear(); + resampledImage = null; requestRepaint(); } @@ -425,7 +435,8 @@ public class CoverageCanvas extends MapCanvasAWT { * *
    *
  1. Compute statistics on sample values (if needed).
    2. - *
  2. Resample the image (if needed).
    3. + *
  3. Stretch the color ramp (if requested).
    4. + *
  4. Resample the image and convert to integer values.
    5. *
  5. Paint the image.
    6. *
*/ @@ -436,30 +447,38 @@ public class CoverageCanvas extends MapCanvasAWT { private final RenderingData data; /** - * The coordinate reference system in which to reproject the data. + * Value of {@link CoverageCanvas#getObjectiveCRS()} at the time this worker has been initialized. + * This the coordinate reference system in which to reproject the data, in "real world" units. */ private final CoordinateReferenceSystem objectiveCRS; /** - * The conversion from {@link #objectiveCRS} to the canvas display CRS. + * Value of {@link CoverageCanvas#getObjectiveToDisplay()} at the time this worker has been initialized. + * This is the conversion from {@link #objectiveCRS} to the canvas display CRS. + * Can be thought as a conversion from "real world" units to pixel units. */ private final LinearTransform objectiveToDisplay; /** - * Whether the {@linkplain RenderingData#resampleAndRecolor resampling operation} applied is different - * than the one used the last time that the image has been rendered, ignoring translations. - * Translations do not require new resampling operations because we can manage translations - * by changing {@link RenderedImage} coordinates. + * Value of {@link CoverageCanvas#getDisplayBounds()} at the time this worker has been initialized. + * This is the size and location of the display device, in pixel units. */ - private boolean resamplingChanged; + private final Envelope2D displayBounds; /** - * The resampled image after color ramp stretching and/or index color model applied. + * The {@link #data} image after color ramp stretching, before resampling is applied. + * May be {@code null} if not yet computed, in which case it will be computed by {@link #render()}. */ private RenderedImage recoloredImage; /** - * The filtered image with tiles computed in advance. The set of prefetched + * The {@link #recoloredImage} after resampling is applied. + * May be {@code null} if not yet computed, in which case it will be computed by {@link #render()}. + */ + private RenderedImage resampledImage; + + /** + * The resampled image with tiles computed in advance. The set of prefetched * tiles may differ at each rendering event. This image should not be cached * after rendering operation is completed. */ @@ -471,11 +490,6 @@ public class CoverageCanvas extends MapCanvasAWT { private AffineTransform resampledToDisplay; /** - * Size and location of the display device, in pixel units. - */ - private final Envelope2D displayBounds; - - /** * The resource from which the data has been read, or {@code null} if unknown. * This is used only for determining a target window for logging records. */ @@ -490,8 +504,9 @@ public class CoverageCanvas extends MapCanvasAWT { objectiveCRS = canvas.getObjectiveCRS(); objectiveToDisplay = canvas.getObjectiveToDisplay(); displayBounds = canvas.getDisplayBounds(); + recoloredImage = canvas.derivedImages.get(data.selectedDerivative); if (data.validateCRS(objectiveCRS)) { - recoloredImage = canvas.resampledImages.get(data.selectedDerivative); + resampledImage = canvas.resampledImage; } } @@ -513,7 +528,7 @@ public class CoverageCanvas extends MapCanvasAWT { /** * Invoked in background thread for resampling the image and stretching the color ramp. * This method performs some of the steps documented in class Javadoc, with possibility - * to skip the first step if the required source image is already resampled. + * to skip some steps for example if the required source image is already resampled. */ @Override @SuppressWarnings("PointlessBitwiseExpression") @@ -521,20 +536,24 @@ public class CoverageCanvas extends MapCanvasAWT { final Long id = LogHandler.loadingStart(originator); try { /* - * A new resampling is needed if the change compared to last rendering - * is anything else than identity or translation. + * Find whether resampling to apply is different than the resampling used last time that the image + * has been rendered, ignoring translations. Translations do not require new resampling operations + * because we can manage translations by changing `RenderedImage` coordinates. */ - resamplingChanged = (recoloredImage == null); + boolean resamplingChanged = (resampledImage == null); if (!resamplingChanged) { resampledToDisplay = data.getTransform(objectiveToDisplay); resamplingChanged = (resampledToDisplay.getType() & ~(AffineTransform.TYPE_IDENTITY | AffineTransform.TYPE_TRANSLATION)) != 0; } if (resamplingChanged) { - recoloredImage = data.resampleAndRecolor(objectiveCRS, objectiveToDisplay); + if (recoloredImage == null) { + recoloredImage = data.recolor(); + } + resampledImage = data.resampleAndConvert(recoloredImage, objectiveCRS, objectiveToDisplay); resampledToDisplay = data.getTransform(objectiveToDisplay); } - prefetchedImage = data.prefetch(recoloredImage, resampledToDisplay, displayBounds); + prefetchedImage = data.prefetch(resampledImage, resampledToDisplay, displayBounds); } finally { LogHandler.loadingStop(id); } @@ -565,22 +584,17 @@ public class CoverageCanvas extends MapCanvasAWT { */ private void cacheRenderingData(final Worker worker) { data = worker.data; - if (worker.resamplingChanged) { - /* - * If resampled image changed, then all derivative images (with stretched color ramp - * or other operation applied) are not valid anymore. We need to empty the cache. - */ - resampledImages.clear(); - } - resampledImages.put(data.selectedDerivative, worker.recoloredImage); + derivedImages.put(data.selectedDerivative, worker.recoloredImage); + resampledImage = worker.resampledImage; /* - * Notify the "Image properties" tab that the image changed. + * Notify the "Image properties" tab that the image changed. The `imageProperty` field is non-null + * only if the "Properties" section in `CoverageControls` has been shown at least once. */ if (imageProperty != null) { - imageProperty.setImage(worker.recoloredImage, worker.getVisibleImageBounds()); + imageProperty.setImage(resampledImage, worker.getVisibleImageBounds()); } if (statusBar != null) { - final Object value = worker.recoloredImage.getProperty(PlanarImage.POSITIONAL_ACCURACY_KEY); + final Object value = resampledImage.getProperty(PlanarImage.POSITIONAL_ACCURACY_KEY); Quantity accuracy = null; if (value instanceof Quantity[]) { for (final Quantity q : (Quantity[]) value) { @@ -598,7 +612,7 @@ public class CoverageCanvas extends MapCanvasAWT { /** * Returns the bounds of the image part which is currently shown. This method performs the same work - * than {@link Worker#getVisibleImageBounds()} is a less efficient way. It is used when no worker is + * than {@link Worker#getVisibleImageBounds()} in a less efficient way. It is used when no worker is * available. * * @see Worker#getVisibleImageBounds() @@ -621,6 +635,7 @@ public class CoverageCanvas extends MapCanvasAWT { final void setStyling(final Stretching selection) { if (data.selectedDerivative != selection) { data.selectedDerivative = selection; + resampledImage = null; requestRepaint(); } } diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/ImagePropertyExplorer.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/ImagePropertyExplorer.java index 07dd06d..899705b 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/ImagePropertyExplorer.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/ImagePropertyExplorer.java @@ -489,7 +489,7 @@ public class ImagePropertyExplorer extends Widget { * If {@link #updateOnChange} is true, then the tree view is updated. * Otherwise we will wait for the tree view to become visible before to update it. * - * @param newValue the new image. + * @param newValue the new image, or {@code null} if none. * @param visibleBounds image region which is currently visible, or {@code null} if unspecified. */ final void setImage(final RenderedImage newValue, final Rectangle visibleBounds) { diff --git a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/RenderingData.java b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/RenderingData.java index 49502f9..2d136f1 100644 --- a/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/RenderingData.java +++ b/application/sis-javafx/src/main/java/org/apache/sis/gui/coverage/RenderingData.java @@ -156,7 +156,7 @@ final class RenderingData implements Cloneable { private AffineTransform displayToObjective; /** - * Key of the currently selected alternative in {@link CoverageCanvas#resampledImages} map. + * Key of the currently selected alternative in {@link CoverageCanvas#derivedImages} map. */ Stretching selectedDerivative; @@ -227,14 +227,42 @@ final class RenderingData implements Cloneable { } /** - * Creates the resampled image, then optionally stretches the color map and applies an index color model. - * This method will compute the {@link MathTransform} steps from image coordinate system to display coordinate - * system if those steps have not already been computed. + * Stretch the color ramp of source image according the current value of {@link #selectedDerivative}. + * This method uses the original image as the source of statistics. It saves computation time + * (no need to recompute the statistics when the projection is changed) and provides more stable + * visual output when standard deviations are used for configuring the color ramp. * + * @return the given image with {@link #selectedDerivative} applied. + */ + final RenderedImage recolor() { + RenderedImage image = data; + if (selectedDerivative != Stretching.NONE) { + final Map modifiers = new HashMap<>(4); + if (statistics == null) { + statistics = processor.getStatistics(image, null); + } + modifiers.put("statistics", statistics); + if (selectedDerivative == Stretching.AUTOMATIC) { + modifiers.put("multStdDev", 3); + } + image = processor.stretchColorRamp(image, modifiers); + } + return image; + } + + /** + * Creates the resampled image, then optionally applies an index color model. + * This method will compute the {@link MathTransform} steps from image coordinate system + * to display coordinate system if those steps have not already been computed. + * + * @param recoloredImage the image computed by {@link #recolor()}. + * @param objectiveCRS value of {@link CoverageCanvas#getObjectiveCRS()}. + * @param objectiveToDisplay value of {@link CoverageCanvas#getObjectiveToDisplay()}. * @return image with operation applied and color ramp stretched. */ - final RenderedImage resampleAndRecolor(final CoordinateReferenceSystem objectiveCRS, - final LinearTransform objectiveToDisplay) throws TransformException + final RenderedImage resampleAndConvert(final RenderedImage recoloredImage, + final CoordinateReferenceSystem objectiveCRS, final LinearTransform objectiveToDisplay) + throws TransformException { if (changeOfCRS == null && objectiveCRS != null && dataGeometry.isDefined(GridGeometry.CRS)) { DefaultGeographicBoundingBox areaOfInterest = null; @@ -272,7 +300,7 @@ final class RenderingData implements Cloneable { * the result to 32 bit integer range). This is okay since only visible tiles will be created. * * TODO: if user pans the image close to integer range limit, we should create a new resampled image - * shifted to new location (i.e. clear `CoverageCanvas.resampledImages` for forcing this method + * shifted to new location (i.e. clear `CoverageCanvas.resampledImage` for forcing this method * to be invoked again). The intent is to move away from integer overflow situation. */ final LinearTransform inverse = objectiveToDisplay.inverse(); @@ -281,36 +309,17 @@ final class RenderingData implements Cloneable { final MathTransform displayToCenter = MathTransforms.concatenate(inverse, centerToObjective.inverse()); final PreferredSize bounds = (PreferredSize) Shapes2D.transform( MathTransforms.bidimensional(cornerToDisplay), - ImageUtilities.getBounds(data), new PreferredSize()); - /* - * Apply a map projection on the image, then convert the result to an index color model. - */ - RenderedImage resampledImage; - resampledImage = processor.resample(data, bounds, displayToCenter); - if (selectedDerivative != Stretching.NONE) { - final Map modifiers = new HashMap<>(4); - /* - * Select the original image as the source of statistics. It saves computation time (no need - * to recompute the statistics when the projection is changed) and provides more stable visual - * output when standard deviations are used for configuring the color ramp. - */ - if (statistics == null) { - statistics = processor.getStatistics(data, null); - } - modifiers.put("statistics", statistics); - if (selectedDerivative == Stretching.AUTOMATIC) { - modifiers.put("multStdDev", 3); - } - resampledImage = processor.stretchColorRamp(resampledImage, modifiers); - } + ImageUtilities.getBounds(recoloredImage), new PreferredSize()); /* - * Converts images of floating point values to integer values that we can use with IndexColorModel. + * Apply a map projection on the image, then convert the floating point results to integer values + * that we can use with IndexColorModel. * * TODO: if `colors` is null, instead than defaulting to `Colorizer.GRAYSCALE` we should get the colors * from the current ColorModel. This work should be done in Colorizer by converting the ranges of * sample values in source image to ranges of sample values in destination image, then query * ColorModel.getRGB(Object) for increasing integer values in that range. */ + RenderedImage resampledImage = processor.resample(recoloredImage, bounds, displayToCenter); if (CREATE_INDEX_COLOR_MODEL) { final ColorModelType ct = ColorModelType.find(resampledImage.getColorModel()); if (ct.isSlow || (processor.getCategoryColors() != null && ct.useColorRamp)) { @@ -324,7 +333,7 @@ final class RenderingData implements Cloneable { * Computes immediately, possibly using many threads, the tiles that are going to be displayed. * The returned instance should be used only for current rendering event; it should not be cached. * - * @param resampledImage the image computed by {@link #resampleAndRecolor resampleAndRecolor(…)}. + * @param resampledImage the image computed by {@link #resampleAndConvert resampleAndConvert(…)}. * @param resampledToDisplay the transform computed by {@link #getTransform(LinearTransform)}. * @param displayBounds size and location of the display device, in pixel units. * @return a temporary image with tiles intersecting the display region already computed.