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: ComputedImage tracks changes in its WritableRenderedImage sources in order to know which tiles to recompute.
Date Sun, 05 Jan 2020 16:51:29 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 22ea241  ComputedImage tracks changes in its WritableRenderedImage sources in order
to know which tiles to recompute.
22ea241 is described below

commit 22ea241f174c5e19b53786b7164aa6bbf247d938
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Sun Jan 5 17:50:53 2020 +0100

    ComputedImage tracks changes in its WritableRenderedImage sources in order to know which
tiles to recompute.
---
 .../java/org/apache/sis/image/ComputedImage.java   | 248 ++++++++++++++++++---
 .../coverage/j2d/BandedSampleConverter.java        |  13 +-
 .../org/apache/sis/image/ComputedImageTest.java    |   2 +-
 .../java/org/apache/sis/image/TiledImageMock.java  |   3 +-
 4 files changed, 222 insertions(+), 44 deletions(-)

diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java
index aa6f19c..b6ba21f 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ComputedImage.java
@@ -16,21 +16,25 @@
  */
 package org.apache.sis.image;
 
-import java.util.Set;
-import java.util.HashSet;
+import java.util.Map;
+import java.util.HashMap;
 import java.util.Arrays;
 import java.util.Vector;
 import java.awt.Point;
 import java.awt.image.Raster;
 import java.awt.image.WritableRaster;
-import java.awt.image.ImagingOpException;
+import java.awt.image.WritableRenderedImage;
 import java.awt.image.RenderedImage;
 import java.awt.image.SampleModel;
+import java.awt.image.TileObserver;
+import java.awt.image.ImagingOpException;
 import java.lang.ref.WeakReference;
 import org.apache.sis.internal.system.ReferenceQueueConsumer;
 import org.apache.sis.internal.feature.Resources;
+import org.apache.sis.internal.util.Numerics;
 import org.apache.sis.util.collection.Cache;
 import org.apache.sis.util.ArgumentChecks;
+import org.apache.sis.util.ArraysExt;
 import org.apache.sis.util.Disposable;
 
 
@@ -43,9 +47,10 @@ import org.apache.sis.util.Disposable;
  *
  * <p>Subclasses need to implement at least the following methods:</p>
  * <ul>
- *   <li>{@link #getWidth()}           — the image width in pixels.</li>
- *   <li>{@link #getHeight()}          — the image height in pixels.</li>
- *   <li>{@link #computeTile(int,int)} — invoked when a requested tile is not in
the cache.</li>
+ *   <li>{@link #getWidth()}  — the image width in pixels.</li>
+ *   <li>{@link #getHeight()} — the image height in pixels.</li>
+ *   <li>{@link #computeTile(int, int, WritableRaster)} — invoked when a
+ *       requested tile is not in the cache or needs to be updated.</li>
  * </ul>
  *
  * <p>If pixel coordinates or tile indices do not start at zero,
@@ -66,24 +71,74 @@ import org.apache.sis.util.Disposable;
  */
 public abstract class ComputedImage extends PlanarImage {
     /**
+     * Whether a tile in the cache is ready for use or needs to be recomputed
+     * because one if its sources changed its data.
+     */
+    private enum TileStatus {
+        /** The tile is ready for use. */
+        STORED,
+
+        /** The tile needs to be recomputed because at least one source changed its data.
*/
+        DIRTY,
+
+        /** The tile has been checked out for a write operation. */
+        CHECKED,
+
+        /** The tile needs to be recomputed, but it is also checked for write operation by
someone else. */
+        CHECKED_AND_DIRTY;
+
+        /** Remapping function for calls to {@link Map#merge(Object, Object, java.util.function.BiFunction)}.
*/
+        static TileStatus merge(final TileStatus oldValue, TileStatus newValue) {
+            if (newValue == DIRTY && oldValue == CHECKED) {
+                newValue = CHECKED_AND_DIRTY;
+            }
+            return newValue;
+        }
+    }
+
+    /**
      * Weak reference to the enclosing image together with necessary information for releasing
resources
      * when image is disposed. This class shall not contain any strong reference to the enclosing
image.
      */
     // MUST be static
-    private static final class Cleaner extends WeakReference<ComputedImage> implements
Disposable {
+    private static final class Cleaner extends WeakReference<ComputedImage> implements
Disposable, TileObserver {
         /**
          * Indices of all cached tiles. Used for removing tiles from the cache when the image
is disposed.
          * All accesses to this collection must be synchronized. This field has to be declared
here because
          * {@link Cleaner} is not allowed to keep a strong reference to the enclosing {@link
ComputedImage}.
          */
-        private final Set<TileCache.Key> cachedTiles;
+        private final Map<TileCache.Key, TileStatus> cachedTiles;
 
         /**
-         * Creates a new weak reference to the given image.
+         * All {@link ComputedImage#sources} that are writable, or {@code null} if none.
+         * This is used for removing tile observers when the enclosing image is garbage-collected.
          */
-        Cleaner(final ComputedImage image) {
+        private WritableRenderedImage[] sources;
+
+        /**
+         * Creates a new weak reference to the given image and registers this {@link Cleaner}
+         * as a listener of all given sources. The listeners will be automatically removed
+         * when the enclosing image is garbage collected.
+         *
+         * @param  image  the enclosing image for which to release tiles on garbage-collection.
+         * @param  ws     sources to observe for changes, or {@code null} if none.
+         */
+        @SuppressWarnings("ThisEscapedInObjectConstruction")
+        Cleaner(final ComputedImage image, final WritableRenderedImage[] ws) {
             super(image, ReferenceQueueConsumer.QUEUE);
-            cachedTiles = new HashSet<>();
+            cachedTiles = new HashMap<>();
+            sources = ws;
+            if (ws != null) {
+                int i = 0;
+                try {
+                    while (i < ws.length) {
+                        WritableRenderedImage source = ws[i++];     // `i++` must be before
`addTileObserver(…)` call.
+                        source.addTileObserver(this);
+                    }
+                } catch (RuntimeException e) {
+                    unregister(ws, i, e);                           // `unregister(…)`
will rethrow the given exception.
+                }
+            }
         }
 
         /**
@@ -92,23 +147,110 @@ public abstract class ComputedImage extends PlanarImage {
          */
         final void addTile(final TileCache.Key key) {
             synchronized (cachedTiles) {
-                cachedTiles.add(key);
+                cachedTiles.put(key, TileStatus.STORED);
+            }
+        }
+
+        /**
+         * Returns {@code true} if the specified tile needs to be recomputed.
+         */
+        final boolean isDirty(final TileCache.Key key) {
+            final TileStatus status;
+            synchronized (cachedTiles) {
+                status = cachedTiles.get(key);
+            }
+            return (status == TileStatus.DIRTY) || (status == TileStatus.CHECKED_AND_DIRTY);
+        }
+
+        /**
+         * Invoked when a source is changing the content of one of its tile.
+         * This method is interested only in events fired after the change is done.
+         * The tiles that depend on the modified tile are marked in need to be recomputed.
+         *
+         * @param source          the image that own the tile which is about to be updated.
+         * @param tileX           the <var>x</var> index of the tile that is
being updated.
+         * @param tileY           the <var>y</var> index of the tile that is
being updated.
+         * @param willBeWritable  if true, the tile is grabbed for writing; otherwise it
is being released.
+         */
+        @Override
+        public void tileUpdate(final WritableRenderedImage source, int tileX, int tileY,
final boolean willBeWritable) {
+            if (!willBeWritable) {
+                final ComputedImage target = get();
+                if (target != null) {
+                    final long sourceWidth  = source.getTileWidth();
+                    final long sourceHeight = source.getTileHeight();
+                    final long targetWidth  = target.getTileWidth();
+                    final long targetHeight = target.getTileHeight();
+                    final long tx           = tileX * sourceWidth  + source.getTileGridXOffset()
- target.getTileGridXOffset();
+                    final long ty           = tileY * sourceHeight + source.getTileGridYOffset()
- target.getTileGridYOffset();
+                    final int  maxTileX     = Numerics.clamp(Math.floorDiv(tx + sourceWidth
 - 1, targetWidth));
+                    final int  maxTileY     = Numerics.clamp(Math.floorDiv(ty + sourceHeight
- 1, targetHeight));
+                    final int  minTileX     = Numerics.clamp(Math.floorDiv(tx, targetWidth));
+                    final int  minTileY     = Numerics.clamp(Math.floorDiv(ty, targetHeight));
+                    synchronized (cachedTiles) {
+                        for (tileY = minTileY; tileY <= maxTileY; tileY++) {
+                            for (tileX = minTileX; tileX <= maxTileX; tileX++) {
+                                final TileCache.Key key = new TileCache.Key(this, tileX,
tileY);
+                                cachedTiles.merge(key, TileStatus.DIRTY, TileStatus::merge);
+                            }
+                        }
+                    }
+                } else {
+                    /*
+                     * Should not happen, unless maybe the source invoked this method before
`dispose()`
+                     * has done its work. Or maybe we have a bug in our code and this `Cleaner`
is still
+                     * alive but should not. In any cases there is no point to continue observing
the source.
+                     */
+                    source.removeTileObserver(this);
+                }
             }
         }
 
         /**
          * Invoked when the enclosing image has been garbage-collected. This method removes
all cached tiles
-         * that were owned by the enclosing image. This method should not perform other cleaning
work than
-         * removing cached tiles because it is not guaranteed to be invoked if {@link TileCache#GLOBAL}
-         * does not contain any tile for the enclosing image (because there would be nothing
preventing
-         * this weak reference to be garbage collected before {@code dispose()} is invoked).
+         * that were owned by the enclosing image and unregister all tile observers.
+         *
+         * This method should not perform other cleaning work because it is not guaranteed
to be invoked if
+         * this {@code Cleaner} is not registered as a {@link TileObserver} and if {@link
TileCache#GLOBAL}
+         * does not contain any tile for the enclosing image. The reason is because there
would be nothing
+         * preventing this weak reference to be garbage collected before {@code dispose()}
is invoked.
+         *
+         * @see ComputedImage#dispose()
          */
         @Override
         public void dispose() {
             synchronized (cachedTiles) {
-                cachedTiles.forEach(TileCache.Key::dispose);
+                cachedTiles.keySet().forEach(TileCache.Key::dispose);
                 cachedTiles.clear();
             }
+            final WritableRenderedImage[] ws = sources;
+            if (ws != null) {
+                unregister(ws, ws.length, null);
+            }
+        }
+
+        /**
+         * Stops observing writable sources for modifications. This methods is invoked when
the enclosing
+         * image is garbage collected. It may also be invoked for rolling back observer registrations
if
+         * an error occurred during {@link Cleaner} construction. This method clears the
{@link #sources}
+         * field immediately for letting the garbage collector to collect the sources in
the event where
+         * this {@code Cleaner} would live longer than expected.
+         *
+         * @param  ws       a copy of {@link #sources}. Can not be null.
+         * @param  i        index after the last source to stop observing.
+         * @param  failure  if this method is invoked because an exception occurred, that
exception.
+         */
+        private void unregister(final WritableRenderedImage[] ws, int i, RuntimeException
failure) {
+            sources = null;                     // Let GC to its work in case of error in
this method.
+            while (--i >= 0) try {
+                ws[i].removeTileObserver(this);
+            } catch (RuntimeException e) {
+                if (failure == null) failure = e;
+                else failure.addSuppressed(e);
+            }
+            if (failure != null) {
+                throw failure;
+            }
         }
     }
 
@@ -143,11 +285,10 @@ public abstract class ComputedImage extends PlanarImage {
 
     /**
      * Creates an initially empty image with the given sample model.
-     * The default tile size will be the width and height of the given sample model.
-     *
-     * <div class="note"><b>Note:</b>
-     * the restriction about sample model size matching tile size is for reducing the amount
-     * of memory consumed by {@link #createTile(int, int)}.</div>
+     * The default tile size will be the width and height of the given sample model
+     * (this default setting minimizes the amount of memory consumed by {@link #createTile(int,
int)}).
+     * This constructor automatically registers a {@link TileObserver}
+     * for all sources that are {@link WritableRenderedImage} instances.
      *
      * @param  sampleModel  the sample model shared by all tiles in this image.
      * @param  sources      sources of this image (may be an empty array), or a null array
if unknown.
@@ -155,14 +296,33 @@ public abstract class ComputedImage extends PlanarImage {
     protected ComputedImage(final SampleModel sampleModel, RenderedImage... sources) {
         ArgumentChecks.ensureNonNull("sampleModel", sampleModel);
         this.sampleModel = sampleModel;
+        /*
+         * Verify the `sources` argument validity and opportunistically collect all writable
sources
+         * in a separated array. If at the end it appears that the two arrays have the same
content,
+         * the same array will be shared by this `ComputedImage` and its `TileObserver`.
+         */
+        WritableRenderedImage[] ws = null;
         if (sources != null) {
             sources = sources.clone();
+            int count = 0;
             for (int i=0; i<sources.length; i++) {
-                ArgumentChecks.ensureNonNullElement("sources", i, sources[i]);
+                final RenderedImage source = sources[i];
+                ArgumentChecks.ensureNonNullElement("sources", i, source);
+                if (source instanceof WritableRenderedImage) {
+                    if (ws == null) {
+                        ws = new WritableRenderedImage[sources.length - i];
+                    }
+                    ws[count++] = (WritableRenderedImage) source;
+                }
+            }
+            if (count == sources.length) {
+                sources = ws;                   // The two arrays have the same content;
share the same one.
+            } else {
+                ws = ArraysExt.resize(ws, count);
             }
         }
         this.sources = sources;             // Note: null value does not have same meaning
than empty array.
-        reference = new Cleaner(this);      // Create cleaner last after all arguments have
been validated.
+        reference = new Cleaner(this, ws);  // Create cleaner last after all arguments have
been validated.
     }
 
     /**
@@ -256,17 +416,19 @@ public abstract class ComputedImage extends PlanarImage {
         final TileCache.Key key = new TileCache.Key(reference, tileX, tileY);
         final Cache<TileCache.Key,Raster> cache = TileCache.GLOBAL;
         Raster tile = cache.peek(key);
-        if (tile == null) {
+        if (tile == null || reference.isDirty(key)) {
             int min;
             ArgumentChecks.ensureBetween("tileX", (min = getMinTileX()), min + getNumXTiles()
- 1, tileX);
             ArgumentChecks.ensureBetween("tileY", (min = getMinTileY()), min + getNumYTiles()
- 1, tileY);
             final Cache.Handler<Raster> handler = cache.lock(key);
             try {
                 tile = handler.peek();
-                if (tile == null) {
+                if (tile == null || reference.isDirty(key)) {
+                    final WritableRaster previous = (tile instanceof WritableRaster) ? (WritableRaster)
tile : null;
                     Exception cause = null;
+                    tile = null;
                     try {
-                        tile = computeTile(tileX, tileY);
+                        tile = computeTile(tileX, tileY, previous);
                     } catch (ImagingOpException e) {
                         throw e;                            // Let that kind of exception
propagate.
                     } catch (Exception e) {
@@ -286,19 +448,33 @@ public abstract class ComputedImage extends PlanarImage {
     }
 
     /**
-     * Invoked when a tile need to be computed. This method is invoked by {@link #getTile(int,
int)}
-     * when the requested tile is not in the cache. The returned tile will be automatically
cached.
+     * Invoked when a tile need to be computed or updated. This method is invoked by {@link
#getTile(int, int)}
+     * when the requested tile is not in the cache, or when a writable source notified us
that its data changed.
+     * The returned tile will be automatically cached.
+     *
+     * <p>A typical implementation is as below:</p>
+     * {@preformat java
+     *     &#64;Override
+     *     protected Raster computeTile(int tileX, int tileY, WritableRaster tile) {
+     *         if (tile == null) {
+     *             tile = createTile(tileX, tileY);
+     *         }
+     *         // Do calculation here and write results in tile.
+     *         return tile;
+     *     }
+     * }
      *
-     * @param  tileX  the column index of the tile to compute.
-     * @param  tileY  the row index of the tile to compute.
-     * @return computed tile for the given indices (can not be null).
+     * @param  tileX     the column index of the tile to compute.
+     * @param  tileY     the row index of the tile to compute.
+     * @param  previous  if the tile already exists but needs to be updated, the tile to
update. Otherwise {@code null}.
+     * @return computed tile for the given indices. May be the {@code previous} tile after
update but can not be null.
      * @throws Exception if an error occurred while computing the tile.
      */
-    protected abstract Raster computeTile(int tileX, int tileY) throws Exception;
+    protected abstract Raster computeTile(int tileX, int tileY, WritableRaster previous)
throws Exception;
 
     /**
      * Creates an initially empty tile at the given tile grid position.
-     * This is a helper method for {@link #computeTile(int, int)} implementations.
+     * This is a helper method for {@link #computeTile(int, int, WritableRaster)} implementations.
      *
      * @param  tileX  the column index of the tile to create.
      * @param  tileY  the row index of the tile to create.
@@ -312,8 +488,8 @@ public abstract class ComputedImage extends PlanarImage {
     }
 
     /**
-     * Advises this image that its tiles will no longer be requested.
-     * This method removes all tiles from the cache.
+     * Advises this image that its tiles will no longer be requested. This method removes
all
+     * tiles from the cache and stops observation of {@link WritableRenderedImage} sources.
      * This image should not be used anymore after this method call.
      *
      * <p><b>Note:</b> keep in mind that this image may be referenced as
a source of other images.
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BandedSampleConverter.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BandedSampleConverter.java
index 4f87725..c488436 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BandedSampleConverter.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/BandedSampleConverter.java
@@ -44,7 +44,7 @@ import org.apache.sis.util.Workaround;
  *   <li>Calculation is performed on {@code float} or {@code double} numbers.</li>
  * </ul>
  *
- * Subclasses may relax those restrictions at the cost of more complex {@link #computeTile(int,
int)}
+ * Subclasses may relax those restrictions at the cost of more complex {@link #computeTile(int,
int, WritableRaster)}
  * implementation. Those restrictions may also be relaxed in future versions of this class.
  *
  * @author  Martin Desruisseaux (Geomatys)
@@ -171,14 +171,17 @@ public final class BandedSampleConverter extends ComputedImage {
     /**
      * Computes the tile at specified indices.
      *
-     * @param  tileX  the column index of the tile to compute.
-     * @param  tileY  the row index of the tile to compute.
+     * @param  tileX   the column index of the tile to compute.
+     * @param  tileY   the row index of the tile to compute.
+     * @param  target  if the tile already exists but needs to be updated, the tile to update.
Otherwise {@code null}.
      * @return computed tile for the given indices (can not be null).
      * @throws TransformException if an error occurred while converting a sample value.
      */
     @Override
-    protected Raster computeTile(final int tileX, final int tileY) throws TransformException
{
-        final WritableRaster target = createTile(tileX, tileY);
+    protected Raster computeTile(final int tileX, final int tileY, WritableRaster target)
throws TransformException {
+        if (target == null) {
+            target = createTile(tileX, tileY);
+        }
         Transferer.create(getSource(0), target).compute(converters);
         return target;
     }
diff --git a/core/sis-feature/src/test/java/org/apache/sis/image/ComputedImageTest.java b/core/sis-feature/src/test/java/org/apache/sis/image/ComputedImageTest.java
index 2c6e613..b4d34f5 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/image/ComputedImageTest.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/image/ComputedImageTest.java
@@ -60,7 +60,7 @@ public final strictfp class ComputedImageTest extends TestCase {
             @Override public ColorModel getColorModel() {return getSource(0).getColorModel();}
             @Override public int        getWidth()      {return getSource(0).getWidth();}
             @Override public int        getHeight()     {return getSource(0).getHeight();}
-            @Override protected Raster  computeTile(final int tileX, final int tileY) {
+            @Override protected Raster  computeTile(final int tileX, final int tileY, WritableRaster
previous) {
                 final int tw = getTileWidth();
                 final int th = getTileHeight();
                 return getSource(0).getData(new Rectangle(tileX * tw, tileY * th, tw, th));
diff --git a/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java b/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java
index 9db5992..0df0cf8 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java
@@ -288,11 +288,10 @@ public final strictfp class TiledImageMock extends PlanarImage implements
Writab
     }
 
     /**
-     * Unsupported since we do not need tile observers for the tests.
+     * Ignored since we do not need tile observers for the tests.
      */
     @Override
     public void addTileObserver(TileObserver to) {
-        throw new UnsupportedOperationException();
     }
 
     /**


Mime
View raw message