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 consolidation: - Remember when we failed to compute a tile. - Detect when a tile goes from having no writers to having one writer, or conversely. - Add documentation about behavior when image is also WritableRenderedImage.
Date Tue, 07 Jan 2020 15:56:46 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 e2a8c3d  ComputedImage consolidation: - Remember when we failed to compute a tile. - Detect when a tile goes from having no writers to having one writer, or conversely. - Add documentation about behavior when image is also WritableRenderedImage.
e2a8c3d is described below

commit e2a8c3de99f87c8787230353debff06a9f619f08
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Tue Jan 7 16:53:47 2020 +0100

    ComputedImage consolidation:
    - Remember when we failed to compute a tile.
    - Detect when a tile goes from having no writers to having one writer, or conversely.
    - Add documentation about behavior when image is also WritableRenderedImage.
---
 .../java/org/apache/sis/image/ComputedImage.java   | 350 ++++++--------------
 .../java/org/apache/sis/image/ComputedTiles.java   | 351 +++++++++++++++++++++
 .../java/org/apache/sis/image/PlanarImage.java     |  19 ++
 .../main/java/org/apache/sis/image/TileCache.java  |  11 +-
 .../java/org/apache/sis/util/collection/Cache.java |  22 +-
 5 files changed, 484 insertions(+), 269 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 88217b3..768a145 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,8 +16,6 @@
  */
 package org.apache.sis.image;
 
-import java.util.Map;
-import java.util.HashMap;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -32,14 +30,10 @@ 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;
 import org.apache.sis.coverage.grid.GridExtent;     // For javadoc
 
 
@@ -74,7 +68,7 @@ import org.apache.sis.coverage.grid.GridExtent;     // For javadoc
  * implementation assumes that source images occupy the same region as this {@code ComputedImage}:
  * all pixels at coordinates (<var>x</var>, <var>y</var>) in this {@code ComputedImage} depend on pixels
  * at the same (<var>x</var>, <var>y</var>) coordinates in the source images,
- * possibly expanded to neighborhood pixels as described in {@link #SOURCE_PADDING_PROPERTY}.
+ * possibly shifted or expanded to neighborhood pixels as described in {@link #SOURCE_PADDING_PROPERTY}.
  * If this assumption does not hold, then subclasses should override the
  * {@link #sourceTileChanged(RenderedImage, int, int)} method.</p>
  *
@@ -96,6 +90,19 @@ import org.apache.sis.coverage.grid.GridExtent;     // For javadoc
  *   <li>{@link #getMinTileY()} — the minimum tile index in the <var>y</var> direction.</li>
  * </ul>
  *
+ * <h2>Writable computed images</h2>
+ * {@code ComputedImage} can itself be a {@link WritableRenderedImage} if subclasses decide so.
+ * A writable computed image is an image which can retro-propagate changes of its values to the source images.
+ * This class provides {@link #hasTileWriters()}, {@link #getWritableTileIndices()}, {@link #isTileWritable(int, int)}
+ * and {@link #markTileWritable(int, int, boolean)} methods for making {@link WritableRenderedImage} implementations easier.
+ *
+ * <p>If this {@code ComputedImage} is writable, then it is subclass responsibility to manage synchronization between
+ * {@link #getTile(int, int) getTile(…)} method (e.g. with a {@linkplain java.util.concurrent.locks.ReadWriteLock#readLock() read lock}) and
+ * {@link WritableRenderedImage#getWritableTile getWritableTile}/{@link WritableRenderedImage#releaseWritableTile releaseWritableTile(…)}
+ * methods (e.g. with a {@linkplain java.util.concurrent.locks.ReadWriteLock#writeLock() write lock}).
+ * Users should invoke the {@code getWritableTile(…)} and {@code releaseWritableTile(…)} methods in
+ * {@code try ... finally} blocks for ensuring proper release of locks.</p>
+ *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
  * @since   1.1
@@ -126,214 +133,18 @@ public abstract class ComputedImage extends PlanarImage {
     public static final String SOURCE_PADDING_PROPERTY = "sourcePadding";
 
     /**
-     * Whether a tile in the cache is ready for use or needs to be recomputed because one if its sources
-     * changed its data. If the tile is checkout out for a write operation, the write operation will have
-     * precedence over the dirty state.
-     */
-    private enum TileStatus {
-        /** The tile, if present, is ready for use. */
-        VALID,
-
-        /** 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. */
-        WRITABLE
-    }
-
-    /**
-     * 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, 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 Map<TileCache.Key, TileStatus> cachedTiles;
-
-        /**
-         * 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.
-         */
-        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 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.
-                }
-            }
-        }
-
-        /**
-         * Sets the status of the specified tile, discarding any previous status.
-         */
-        final void setTileStatus(final TileCache.Key key, final TileStatus status) {
-            synchronized (cachedTiles) {
-                cachedTiles.put(key, status);
-            }
-        }
-
-        /**
-         * Returns the status of the tile at the specified indices.
-         * The main status of interest are:
-         * <ul>
-         *   <li>{@link TileStatus#DIRTY}    — if the tile needs to be recomputed.</li>
-         *   <li>{@link TileStatus#WRITABLE} — if the tile is currently checked out for writing.</li>
-         * </ul>
-         */
-        final TileStatus getTileStatus(final TileCache.Key key) {
-            synchronized (cachedTiles) {
-                return cachedTiles.get(key);
-            }
-        }
-
-        /**
-         * Adds in the given list the indices of all tiles which are checked out for writing.
-         * If the given list is {@code null}, then this method stops the search at the first
-         * tile checked out.
-         *
-         * @param  indices  the list where to add indices, or {@code null} if none.
-         * @return whether at least one tile is checked out for writing.
-         */
-        final boolean getWritableTileIndices(final List<Point> indices) {
-            synchronized (cachedTiles) {
-                for (final Map.Entry<TileCache.Key, TileStatus> entry : cachedTiles.entrySet()) {
-                    if (entry.getValue() == TileStatus.WRITABLE) {
-                        if (indices == null) return true;
-                        indices.add(entry.getKey().indices());
-                    }
-                }
-            }
-            return (indices != null) && !indices.isEmpty();
-        }
-
-        /**
-         * Marks all tiles in the given range of indices as in need of being recomputed.
-         * This method is invoked when some tiles of at least one source image changed.
-         * All arguments, including maximum values, are inclusive.
-         *
-         * @see ComputedImage#markDirtyTiles(Rectangle)
-         */
-        final void markDirtyTiles(final int minTileX, final int minTileY, final int maxTileX, final int maxTileY) {
-            synchronized (cachedTiles) {
-                for (int tileY = minTileY; tileY <= maxTileY; tileY++) {
-                    for (int tileX = minTileX; tileX <= maxTileX; tileX++) {
-                        final TileCache.Key key = new TileCache.Key(this, tileX, tileY);
-                        cachedTiles.replace(key, TileStatus.VALID, TileStatus.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 {@code 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) {
-                    target.sourceTileChanged(source, tileX, tileY);
-                } 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 and stops observing all sources.
-         *
-         * 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.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 allowing the garbage collector to release 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;
-            }
-        }
-    }
-
-    /**
      * Weak reference to this image, also used as a cleaner when the image is garbage-collected.
      * This reference is retained in {@link TileCache#GLOBAL}. Note that if that cache does not
-     * cache any tile for this image, then this {@link Cleaner} may be garbage-collected in same
-     * time than this image and its {@link Cleaner#dispose()} method may never be invoked.
+     * cache any tile for this image, then that {@link ComputedTiles} may be garbage-collected
+     * in same time than this image and its {@link ComputedTiles#dispose()} method may never be
+     * invoked.
      */
-    private final Cleaner reference;
+    private final ComputedTiles reference;
 
     /**
      * The sources of this image, or {@code null} if unknown. This array contains all sources.
-     * By contrast the {@link Cleaner#sources} array contains only the modifiable sources, for
-     * which we listen for changes.
+     * By contrast the {@link ComputedTiles#sources} array contains only the modifiable sources,
+     * for which we listen for changes.
      *
      * @see #getSource(int)
      */
@@ -392,14 +203,14 @@ public abstract class ComputedImage extends PlanarImage {
              */
             if (count != 0) {
                 if (count == sources.length) {
-                    sources = ws;               // The two arrays have the same content; share the same array.
+                    sources = ws;                   // The two arrays have the same content; share the same array.
                 } else {
                     ws = ArraysExt.resize(ws, count);
                 }
             }
         }
-        this.sources = sources;             // Note: null value does not have same meaning than empty array.
-        reference = new Cleaner(this, ws);  // Create cleaner last after all arguments have been validated.
+        this.sources = sources;                     // Note: null value does not have same meaning than empty array.
+        reference = new ComputedTiles(this, ws);    // Create cleaner last after all arguments have been validated.
     }
 
     /**
@@ -507,19 +318,18 @@ public abstract class ComputedImage extends PlanarImage {
      *
      * <ol>
      *   <li>If the requested tile is present in the cache and is not dirty, then that tile is returned immediately.</li>
-     *   <li>Otherwise if the requested tile is being computed in another thread, then this method blocks
-     *       until the other thread completed its work and returns its result. If the other thread failed
-     *       to compute the tile, an {@link ImagingOpException} is thrown.</li>
+     *   <li>Otherwise if the requested tile is being {@linkplain #computeTile computed} in another thread,
+     *       then this method blocks until the other thread completed its work and returns its result.
+     *       If the other thread failed to compute the tile, an {@link ImagingOpException} is thrown.</li>
      *   <li>Otherwise this method computes the tile and caches the result before to return it.
      *       If an error occurred, an {@link ImagingOpException} is thrown.</li>
      * </ol>
      *
      * <h4>Race conditions with write operations</h4>
-     * If this image implements the {@link WritableRenderedImage} interface, then a user may have acquired
-     * the tile for a write operation outside the {@link #computeTile computeTile(…)} method. In such case,
-     * there is no consistency guarantees on sample values: the tile returned by this method may show data
-     * in an unspecified stage during the write operation. This situation may be detected by checking if
-     * {@link #isTileWritable(int, int) isTileWritable(tileX, tileY)} returns {@code true}.
+     * If this image implements the {@link WritableRenderedImage} interface, then a user may acquire the same
+     * tile for a write operation after this method returned. In such case there is no consistency guarantees
+     * on sample values: the tile returned by this method may show data in an unspecified stage during the
+     * write operation.
      *
      * @param  tileX  the column index of the tile to get.
      * @param  tileY  the row index of the tile to get.
@@ -532,33 +342,37 @@ 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 || reference.getTileStatus(key) == TileStatus.DIRTY) {
+        if (tile == null || reference.isTileDirty(key)) {
             int min;
             ArgumentChecks.ensureBetween("tileX", (min = getMinTileX()), min + getNumXTiles() - 1, tileX);
             ArgumentChecks.ensureBetween("tileY", (min = getMinTileY()), min + getNumYTiles() - 1, tileY);
+            Exception error = null;
             final Cache.Handler<Raster> handler = cache.lock(key);
             try {
                 tile = handler.peek();
-                if (tile == null || reference.getTileStatus(key) == TileStatus.DIRTY) {
+                final boolean marked = reference.trySetComputing(key);              // May throw ImagingOpException.
+                if (marked || tile == null) {
                     final WritableRaster previous = (tile instanceof WritableRaster) ? (WritableRaster) tile : null;
-                    Exception cause = null;
-                    tile = null;
                     try {
                         tile = computeTile(tileX, tileY, previous);
-                    } catch (ImagingOpException e) {
-                        throw e;                            // Let that kind of exception propagate.
                     } catch (Exception e) {
-                        cause = e;
+                        tile = null;
+                        error = e;
                     }
-                    if (tile == null) {
-                        throw (ImagingOpException) new ImagingOpException(Resources.format(
-                                Resources.Keys.CanNotComputeTile_2, tileX, tileY)).initCause(cause);
+                    if (marked) {
+                        reference.endWrite(key);
                     }
-                    reference.setTileStatus(key, TileStatus.VALID);
                 }
             } finally {
                 handler.putAndUnlock(tile);     // Must be invoked even if an exception occurred.
             }
+            if (tile == null) {                 // Null in case of exception or if `computeTile(…)` returned null.
+                if (error instanceof ImagingOpException) {
+                    throw (ImagingOpException) error;
+                } else {
+                    throw (ImagingOpException) new ImagingOpException(key.error()).initCause(error);
+                }
+            }
         }
         return tile;
     }
@@ -604,13 +418,21 @@ public abstract class ComputedImage extends PlanarImage {
     }
 
     /**
-     * Returns whether any tile is checked out for writing.
-     * This method always returns {@code false} for read-only images, but may return {@code true}
-     * if this {@code ComputedImage} is also a {@link WritableRenderedImage}.
+     * Returns whether any tile is under computation or is checked out for writing.
+     * There is two reasons why this method may return {@code true}:
      *
-     * @return {@code true} if any tiles are checked out for writing; {@code false} otherwise.
+     * <ul>
+     *   <li>At least one {@link #computeTile(int, int, WritableRaster) computeTile(…)}
+     *       call is running in another thread.</li>
+     *   <li>There is at least one call to <code>{@linkplain #markTileWritable(int, int, boolean)
+     *       markTileWritable}(tileX, tileY, true)</code> call without matching call to
+     *       {@code markTileWritable(tileX, tileY, false)}. This second case may happen
+     *       if this {@code ComputedImage} is also a {@link WritableRenderedImage}.</li>
+     * </ul>
      *
-     * @see #markWritableTile(int, int, boolean)
+     * @return whether any tiles are under computation or checked out for writing.
+     *
+     * @see #markTileWritable(int, int, boolean)
      * @see WritableRenderedImage#hasTileWriters()
      */
     public boolean hasTileWriters() {
@@ -618,29 +440,38 @@ public abstract class ComputedImage extends PlanarImage {
     }
 
     /**
-     * Returns whether a tile is currently checked out for writing.
-     * This method always returns {@code false} for read-only images, but may return {@code true}
-     * if this {@code ComputedImage} is also a {@link WritableRenderedImage}.
-     *
-     * @param  tileX the X index of the tile.
-     * @param  tileY the Y index of the tile.
-     * @return {@code true} if specified tile is checked out for writing; {@code false} otherwise.
-     *
-     * @see #markWritableTile(int, int, boolean)
+     * Returns whether the specified tile is currently under computation or checked out for writing.
+     * There is two reasons why this method may return {@code true}:
+     *
+     * <ul>
+     *   <li><code>{@linkplain #computeTile(int, int, WritableRaster) computeTile}(tileX, tileY, …)</code>
+     *       is running in another thread.</li>
+     *   <li>There is at least one call to <code>{@linkplain #markTileWritable(int, int, boolean)
+     *       markTileWritable}(tileX, tileY, true)</code> call without matching call to
+     *       {@code markTileWritable(tileX, tileY, false)}. This second case may happen
+     *       if this {@code ComputedImage} is also a {@link WritableRenderedImage}.</li>
+     * </ul>
+     *
+     * @param  tileX the X index of the tile to check.
+     * @param  tileY the Y index of the tile to check.
+     * @return whether the specified tile is under computation or checked out for writing.
+     *
+     * @see #markTileWritable(int, int, boolean)
      * @see WritableRenderedImage#isTileWritable(int, int)
      */
     public boolean isTileWritable(final int tileX, final int tileY) {
-        return reference.getTileStatus(new TileCache.Key(reference, tileX, tileY)) == TileStatus.WRITABLE;
+        return reference.isTileWritable(new TileCache.Key(reference, tileX, tileY));
     }
 
     /**
-     * Returns an array of Point objects indicating which tiles are checked out for writing, or {@code null} if none.
-     * This method always returns {@code null} for read-only images, but may return a non-empty array
-     * if this {@code ComputedImage} is also a {@link WritableRenderedImage}.
+     * Returns the indices of all tiles under computation or checked out for writing, or {@code null} if none.
+     * This method lists all tiles for which the condition documented in {@link #isTileWritable(int, int)} is
+     * {@code true}.
      *
-     * @return an array containing the locations of tiles that are checked out for writing, or {@code null} if none.
+     * @return an array containing the indices of tiles that are under computation or checked out for writing,
+     *         or {@code null} if none.
      *
-     * @see #markWritableTile(int, int, boolean)
+     * @see #markTileWritable(int, int, boolean)
      * @see WritableRenderedImage#getWritableTileIndices()
      */
     public Point[] getWritableTileIndices() {
@@ -652,8 +483,9 @@ public abstract class ComputedImage extends PlanarImage {
     }
 
     /**
-     * Marks a tile as checkout out for writing. This method is provided for subclasses that also implement
-     * the {@link WritableRenderedImage} interface. This method can be used as below:
+     * Sets or clears whether a tile is checked out for writing.
+     * This method is provided for subclasses that implement the {@link WritableRenderedImage} interface.
+     * This method can be used as below:
      *
      * {@preformat java
      *     class MyImage extends ComputedImage implements WritableRenderedImage {
@@ -662,13 +494,13 @@ public abstract class ComputedImage extends PlanarImage {
      *         &#64;Override
      *         public WritableRaster getWritableTile(int tileX, int tileY) {
      *             WritableRaster raster = ...;             // Get the writable tile here.
-     *             markWritableTile(tileX, tileY, true);
+     *             markTileWritable(tileX, tileY, true);
      *             return raster;
      *         }
      *
      *         &#64;Override
      *         public void releaseWritableTile(int tileX, int tileY) {
-     *             markWritableTile(tileX, tileY, false);
+     *             markTileWritable(tileX, tileY, false);
      *             // Release the raster here.
      *         }
      *     }
@@ -681,9 +513,13 @@ public abstract class ComputedImage extends PlanarImage {
      * @see WritableRenderedImage#getWritableTile(int, int)
      * @see WritableRenderedImage#releaseWritableTile(int, int)
      */
-    protected void markWritableTile(final int tileX, final int tileY, final boolean writing) {
+    protected void markTileWritable(final int tileX, final int tileY, final boolean writing) {
         final TileCache.Key key = new TileCache.Key(reference, tileX, tileY);
-        reference.setTileStatus(key, writing ? TileStatus.WRITABLE : TileStatus.VALID);
+        if (writing) {
+            reference.startWrite(key);
+        } else {
+            reference.endWrite(key);
+        }
     }
 
     /**
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/ComputedTiles.java b/core/sis-feature/src/main/java/org/apache/sis/image/ComputedTiles.java
new file mode 100644
index 0000000..f88d493
--- /dev/null
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ComputedTiles.java
@@ -0,0 +1,351 @@
+/*
+ * 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.image;
+
+import java.util.Map;
+import java.util.HashMap;
+import java.util.List;
+import java.lang.ref.WeakReference;
+import java.awt.Point;
+import java.awt.image.TileObserver;
+import java.awt.image.ImagingOpException;
+import java.awt.image.WritableRenderedImage;
+import org.apache.sis.internal.system.ReferenceQueueConsumer;
+import org.apache.sis.util.Disposable;
+
+
+/**
+ * Weak reference to a {@link ComputedImage} image together with information about tile status.
+ * This class also contains necessary information for releasing resources when image is disposed.
+ * This class shall not contain any strong reference to the {@link ComputedImage}.
+ *
+ * <p>Despite the {@code ComputedTiles} class name, this class does not contain any reference
+ * to the tiles. Instead it contains keys for getting the tiles from {@link TileCache#GLOBAL}.
+ * Consequently this class "contains" the tiles only indirectly.</p>
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.1
+ * @since   1.1
+ * @module
+ */
+final class ComputedTiles extends WeakReference<ComputedImage> implements Disposable, TileObserver {
+    /**
+     * Whether a tile in the cache is ready for use or needs to be recomputed because one if its sources
+     * changed its data. Those values are stored in {@link #cachedTiles} map.
+     *
+     * <ul>
+     *   <li>{@code VALID} means that the tile, if presents, is ready for use. A tile may be non-existent in the cache
+     *       despite being marked {@code VALID} if the tile has been garbage-collected after it has been marked.</li>
+     *   <li>{@code DIRTY} means that the tile needs to be recomputed. If the tile is present, its data should be
+     *       discarded but its storage space will be reused.</li>
+     *   <li>{@code ERROR} means that the previous attempt to compute this tile failed.</li>
+     *   <li>All other values means that the tile has been checkout out for a write operation.
+     *       That value is incremented/decremented when the writable tile is acquired/released.
+     *       Write operation status have precedence over the dirty state.</li>
+     *   <li>{@code COMPUTING} is a special case of above point when calculation just started.</li>
+     * </ul>
+     */
+    private static final int VALID = 0, DIRTY = -1, ERROR = -2, COMPUTING = 1;
+
+    /**
+     * Indices of all cached tiles. Used for removing tiles from the cache when the image is disposed.
+     * Values can be {@link #ERROR}, {@link #DIRTY}, {@link #VALID} or counts of writers as unsigned
+     * integers (including {@link #COMPUTING} and {@link #VALID} as special cases).
+     *
+     * All accesses to this collection must be synchronized.
+     */
+    private final Map<TileCache.Key, Integer> cachedTiles;
+
+    /**
+     * All {@link ComputedImage#sources} that are writable, or {@code null} if none.
+     * This is used for removing tile observers when the {@link ComputedImage} is garbage-collected.
+     */
+    private WritableRenderedImage[] sources;
+
+    /**
+     * Creates a new weak reference to the given image and registers this {@link ComputedTiles}
+     * as a listener of all given sources. The listeners will be automatically removed when the
+     * {@link ComputedImage} is garbage collected.
+     *
+     * @param  image  the image for which to release tiles on garbage-collection.
+     * @param  ws     sources to observe for changes, or {@code null} if none.
+     */
+    @SuppressWarnings("ThisEscapedInObjectConstruction")
+    ComputedTiles(final ComputedImage image, final WritableRenderedImage[] ws) {
+        super(image, ReferenceQueueConsumer.QUEUE);
+        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.
+            }
+        }
+    }
+
+    /**
+     * Returns {@code true} if the given value is {@link #COMPUTING} or a greater unsigned value.
+     * Returns {@code false} if the value is null, {@link #VALID}, {@link #DIRTY} or {@link #ERROR}.
+     */
+    private static boolean isWritable(final Integer value) {
+        if (value == null) return false;
+        final int n = value;                        // Negative if we have more than Integer.MAX_VALUE writers.
+        return (n >= COMPUTING) || (n < ERROR);
+    }
+
+    /**
+     * Returns {@code true} if the specified tile is checked out for a write operation.
+     *
+     * @param  key  indices of the tile to check.
+     * @return whether the specified tile is checked out for a write operation.
+     */
+    final boolean isTileWritable(final TileCache.Key key) {
+        final Integer value;
+        synchronized (cachedTiles) {
+            value = cachedTiles.get(key);
+        }
+        return isWritable(value);
+    }
+
+    /**
+     * Returns {@code true} if the specified tile needs to be recomputed. An absent tile is considered as dirty.
+     * If previous attempt to compute the tile failed, then an {@link ImagingOpException} is thrown again.
+     *
+     * @param  key  indices of the tile to check.
+     * @return whether the specified tile needs to be recomputed.
+     * @throws ImagingOpException if we already tried and failed to compute the specified tile.
+     */
+    final boolean isTileDirty(final TileCache.Key key) {
+        final Integer value;
+        synchronized (cachedTiles) {
+            value = cachedTiles.get(key);
+        }
+        if (value != null) {
+            switch (value) {
+                case DIRTY: break;
+                case ERROR: throw new ImagingOpException(key.error());
+                default:    return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * If the specified tile is absent or {@link #DIRTY}, sets its status to {@link #COMPUTING} and
+     * returns {@code true}. Otherwise if there is no error, does nothing and returns {@code false}.
+     *
+     * @param  key  indices of the tile to compute if dirty.
+     * @return whether the specified tile was absent or dirty.
+     * @throws ImagingOpException if we already tried and failed to compute the specified tile.
+     */
+    final boolean trySetComputing(final TileCache.Key key) {
+        final Integer value;
+        synchronized (cachedTiles) {
+            value = cachedTiles.putIfAbsent(key, COMPUTING);
+            if (value == null || cachedTiles.replace(key, DIRTY, COMPUTING)) {
+                return true;
+            }
+        }
+        if (value == ERROR) {
+            throw new ImagingOpException(key.error());
+        }
+        return false;
+    }
+
+    /**
+     * Increments the count of writers for the specified tile.
+     * If the specified tile was marked dirty or in error, that previous status is discarded.
+     *
+     * @param  key  indices of the tile to mark writable.
+     * @return {@code true} if the tile goes from having no writers to having one writer.
+     * @throws ArithmeticException if too many writers.
+     */
+    final boolean startWrite(final TileCache.Key key) {
+        final Integer value;
+        synchronized (cachedTiles) {
+            value = cachedTiles.merge(key, COMPUTING, ComputedTiles::increment);
+        }
+        return value == COMPUTING;
+    }
+
+    /**
+     * Decrements the count of writers for the specified tile.
+     *
+     * @param  key  indices of the tile which was marked writable.
+     * @return {@code true} if the tile goes from having one writer to having no writers.
+     */
+    final boolean endWrite(final TileCache.Key key) {
+        final Integer value;
+        synchronized (cachedTiles) {
+            value = cachedTiles.merge(key, VALID, ComputedTiles::decrement);
+        }
+        return value == VALID;
+    }
+
+    /**
+     * If the value is {@link #VALID}, {@link #DIRTY} or {@link #ERROR}, sets it to {@link #COMPUTING}.
+     * Otherwise increments that value.
+     *
+     * @param  value      the value to increment.
+     * @param  computing  must be {@link #COMPUTING}.
+     * @return the incremented value.
+     */
+    private static Integer increment(final Integer value, final Integer computing) {
+        final int n = value;
+        switch (n) {
+            case VALID:
+            case DIRTY:
+            case ERROR:     return computing;
+            case ERROR - 1: throw new ArithmeticException();        // Unsigned integer overflow
+            default:        return n + 1;                           // case COMPUTING or greater
+        }
+    }
+
+    /**
+     * If the value is {@link #VALID}, {@link #DIRTY}, {@link #ERROR} or {@link #COMPUTING},
+     * sets it to {@link #VALID}. Otherwise decrements that value.
+     *
+     * @param  value  the value to decrement.
+     * @param  valid  must be {@link #VALID}.
+     * @return the decremented value.
+     */
+    private static Integer decrement(final Integer value, final Integer valid) {
+        final int n = value;
+        if (n >= ERROR && n <= COMPUTING) {     // Do not use the ternary operator here.
+            return valid;
+        } else {
+            return n - 1;
+        }
+    }
+
+    /**
+     * Adds in the given list the indices of all tiles which are checked out for writing.
+     * If the given list is {@code null}, then this method stops the search at the first
+     * writable tile.
+     *
+     * @param  indices  the list where to add indices, or {@code null} if none.
+     * @return whether at least one tile is checked out for writing.
+     */
+    final boolean getWritableTileIndices(final List<Point> indices) {
+        synchronized (cachedTiles) {
+            for (final Map.Entry<TileCache.Key, Integer> entry : cachedTiles.entrySet()) {
+                if (isWritable(entry.getValue())) {
+                    if (indices == null) return true;
+                    indices.add(entry.getKey().indices());
+                }
+            }
+        }
+        return (indices != null) && !indices.isEmpty();
+    }
+
+    /**
+     * Marks all tiles in the given range of indices as in need of being recomputed.
+     * This method is invoked when some tiles of at least one source image changed.
+     * All arguments, including maximum values, are inclusive.
+     *
+     * @see ComputedImage#markDirtyTiles(Rectangle)
+     */
+    final void markDirtyTiles(final int minTileX, final int minTileY, final int maxTileX, final int maxTileY) {
+        synchronized (cachedTiles) {
+            for (int tileY = minTileY; tileY <= maxTileY; tileY++) {
+                for (int tileX = minTileX; tileX <= maxTileX; tileX++) {
+                    final TileCache.Key key = new TileCache.Key(this, tileX, tileY);
+                    cachedTiles.replace(key, VALID, 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 {@code 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) {
+                target.sourceTileChanged(source, tileX, tileY);
+            } 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 `ComputedTiles` is still alive when
+                 * it should not. In any cases there is no point to continue observing the source.
+                 */
+                source.removeTileObserver(this);
+            }
+        }
+    }
+
+    /**
+     * Invoked when the {@link ComputedImage} has been garbage-collected. This method removes all cached
+     * tiles that were owned by the image and stops observing all sources.
+     *
+     * This method should not perform other cleaning work because it is not guaranteed to be invoked if this
+     * {@code ComputedTiles} is not registered as a {@link TileObserver} and if {@link TileCache#GLOBAL} does
+     * not contain any tile for the {@link ComputedImage}. 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.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 method is invoked when the {@link ComputedImage}
+     * is garbage collected. It may also be invoked for rolling back observer registrations if an error occurred
+     * during {@link ComputedTiles} construction. This method clears the {@link #sources} field immediately for
+     * allowing the garbage collector to release the sources in the event where this {@code ComputedTiles} 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;
+        }
+    }
+}
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java b/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java
index f7a4b5d..4c838d9 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/PlanarImage.java
@@ -23,6 +23,7 @@ import java.awt.image.IndexColorModel;
 import java.awt.image.SampleModel;
 import java.awt.image.Raster;
 import java.awt.image.WritableRaster;
+import java.awt.image.WritableRenderedImage;
 import java.awt.image.RenderedImage;
 import java.util.Vector;
 import org.apache.sis.util.Classes;
@@ -73,6 +74,24 @@ import org.apache.sis.internal.coverage.j2d.ColorModelFactory;
  * {@link #getData(Rectangle)} and {@link #copyData(WritableRaster)}
  * in terms of above methods.
  *
+ * <h2>Writable images</h2>
+ * Some subclasses may implement the {@link WritableRenderedImage} interface. If this image is writable, then the
+ * {@link WritableRenderedImage#getWritableTile getWritableTile(…)} and {@link WritableRenderedImage#releaseWritableTile
+ * releaseWritableTile(…)} methods <strong>must</strong> be invoked in {@code try ... finally} block like below:
+ *
+ * {@preformat java
+ *     WritableRenderedImage image = ...;
+ *     WritableRaster tile = image.getWritableTile(tileX, tileY);
+ *     try {
+ *         // Do some process on the tile.
+ *     } finally {
+ *         image.releaseWritableTile(tileX, tileY);
+ *     }
+ * }
+ *
+ * The reason is because some implementations may acquire and release synchronization locks in the
+ * {@code getWritableTile(…)} and {@code releaseWritableTile(…)} methods.
+ *
  * @author  Johann Sorel (Geomatys)
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/TileCache.java b/core/sis-feature/src/main/java/org/apache/sis/image/TileCache.java
index ce49d25..48f92a1 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/TileCache.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/TileCache.java
@@ -21,6 +21,8 @@ import java.awt.image.DataBuffer;
 import java.awt.image.Raster;
 import java.lang.ref.Reference;
 import org.apache.sis.util.collection.Cache;
+import org.apache.sis.internal.util.Numerics;
+import org.apache.sis.internal.feature.Resources;
 
 
 /**
@@ -68,7 +70,7 @@ final class TileCache extends Cache<TileCache.Key, Raster> {
         } catch (IllegalArgumentException e) {
             numBits *= Integer.SIZE;                // Conservatively assume 32 bits values.
         }
-        return (int) Math.min(Integer.MAX_VALUE, numBits / Byte.SIZE);
+        return Numerics.clamp(numBits / Byte.SIZE);
     }
 
     /**
@@ -108,6 +110,13 @@ final class TileCache extends Cache<TileCache.Key, Raster> {
         }
 
         /**
+         * Returns the error message when this tile can not be computed.
+         */
+        final String error() {
+            return Resources.format(Resources.Keys.CanNotComputeTile_2, tileX, tileY);
+        }
+
+        /**
          * Removes the raster associated to this key. This method is invoked
          * for all tiles in an image being disposed.
          */
diff --git a/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java b/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java
index fc9e340..469f183 100644
--- a/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java
+++ b/core/sis-utility/src/main/java/org/apache/sis/util/collection/Cache.java
@@ -70,7 +70,7 @@ import org.apache.sis.internal.system.ReferenceQueueConsumer;
  *   <li>Check if the value is already available in the map.
  *       If it is, return it immediately and we are done.</li>
  *   <li>Otherwise, get a lock and check again if the value is already available in the map
- *       (because the value could have been computed by an other thread between step 1 and
+ *       (because the value could have been computed by another thread between step 1 and
  *       the obtention of the lock). If it is, release the lock and we are done.</li>
  *   <li>Otherwise compute the value, store the result and release the lock.</li>
  * </ol>
@@ -775,7 +775,7 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
     /**
      * If a value is already cached for the given key, returns it. Otherwise returns {@code null}.
      * This method is similar to {@link #get(Object)} except that it doesn't block if the value is
-     * in process of being computed in an other thread; it returns {@code null} in such case.
+     * in process of being computed in another thread; it returns {@code null} in such case.
      *
      * @param  key  the key for which to get the cached value.
      * @return the cached value for the given key, or {@code null} if there is none.
@@ -864,7 +864,7 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
                 if (value == null) {
                     /*
                      * We succeed in adding the handler in the map (we know that because all our
-                     * map.put(...) or map.replace(...) operations are guaranteed to put non-null
+                     * map.put(…) or map.replace(…) operations are guaranteed to put non-null
                      * values). We are done. But before to leave, declare that we do not want to
                      * unlock in the finally clause (we want the lock to still active).
                      */
@@ -981,7 +981,7 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
         /**
          * If the value is already in the cache, returns it. Otherwise returns {@code null}.
          * This method should be invoked after the {@code Handler} creation in case a value
-         * has been computed in an other thread.
+         * has been computed in another thread.
          *
          * @return the value from the cache, or {@code null} if none.
          */
@@ -1004,12 +1004,12 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
 
     /**
      * A simple handler implementation wrapping an existing value. This implementation
-     * is used when the value has been fully computed in an other thread before this
+     * is used when the value has been fully computed in another thread before this
      * thread could start its work.
      */
     private final class Simple<V> implements Handler<V> {
         /**
-         * The result computed in an other thread.
+         * The result computed in another thread.
          */
         private final V value;
 
@@ -1034,7 +1034,7 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
          * <div class="note"><b>Implementation note:</b>
          * An alternative would have been to store the result in the map anyway.
          * But doing so is unsafe because we have no lock; we have no guarantee that nothing
-         * has happened in an other thread between {@code peek} and {@code putAndUnlock}.</div>
+         * has happened in another thread between {@code peek} and {@code putAndUnlock}.</div>
          */
         @Override
         public void putAndUnlock(final V result) throws IllegalStateException {
@@ -1130,9 +1130,9 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
         }
 
         /**
-         * A handler implementation used when the value is in process of being computed in an
-         * other thread. At the difference of the {@code Simple} handler, the computation is
-         * not yet completed, so this handler has to wait.
+         * A handler implementation used when the value is in process of being computed in another thread.
+         * At the difference of the {@code Simple} handler, the computation is not yet completed, so this
+         * handler has to wait.
          */
         final class Wait implements Handler<V> {
             /**
@@ -1149,7 +1149,7 @@ public class Cache<K,V> extends AbstractMap<K,V> implements ConcurrentMap<K,V> {
              * <div class="note"><b>Implementation note:</b>
              * An alternative would have been to store the result in the map anyway.
              * But doing so is unsafe because we have no lock; we have no guarantee that nothing
-             * has happened in an other thread between {@code peek} and {@code putAndUnlock}.</div>
+             * has happened in another thread between {@code peek} and {@code putAndUnlock}.</div>
              */
             @Override
             public void putAndUnlock(final V result) throws IllegalStateException {


Mime
View raw message