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: Add a `clearErrorFlags` method and add tests.
Date Tue, 07 Jan 2020 19:37:41 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 0d36b92  Add a `clearErrorFlags` method and add tests.
0d36b92 is described below

commit 0d36b925a4e0bebd2ddd8af807def88d98f06a64
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Tue Jan 7 20:37:16 2020 +0100

    Add a `clearErrorFlags` method and add tests.
---
 .../java/org/apache/sis/image/ComputedImage.java   | 50 ++++++++---
 .../java/org/apache/sis/image/ComputedTiles.java   | 42 ++++++----
 .../main/java/org/apache/sis/image/TileCache.java  |  6 +-
 .../org/apache/sis/internal/feature/Resources.java |  5 ++
 .../sis/internal/feature/Resources.properties      |  1 +
 .../sis/internal/feature/Resources_fr.properties   |  1 +
 .../org/apache/sis/image/ComputedImageTest.java    | 98 +++++++++++++++++++++-
 7 files changed, 174 insertions(+), 29 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 768a145..af6b96d 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
@@ -35,6 +35,7 @@ import org.apache.sis.util.collection.Cache;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.util.ArraysExt;
 import org.apache.sis.coverage.grid.GridExtent;     // For javadoc
+import org.apache.sis.internal.feature.Resources;
 
 
 /**
@@ -360,7 +361,7 @@ public abstract class ComputedImage extends PlanarImage {
                         error = e;
                     }
                     if (marked) {
-                        reference.endWrite(key);
+                        reference.endWrite(key, error == null);
                     }
                 }
             } finally {
@@ -370,7 +371,7 @@ public abstract class ComputedImage extends PlanarImage {
                 if (error instanceof ImagingOpException) {
                     throw (ImagingOpException) error;
                 } else {
-                    throw (ImagingOpException) new ImagingOpException(key.error()).initCause(error);
+                    throw (ImagingOpException) new ImagingOpException(key.error(Resources.Keys.CanNotComputeTile_2)).initCause(error);
                 }
             }
         }
@@ -394,6 +395,14 @@ public abstract class ComputedImage extends PlanarImage {
      *     }
      * }
      *
+     * <h4>Error handling</h4>
+     * If this method throws an exception or return {@code null}, then {@link #getTile(int,
int) getTile(…)}
+     * will set an error flag on the tile and throw an {@link ImagingOpException} with the
exception thrown
+     * by {@code computeTile(…)} as its cause. Future invocations of {@code getTile(tileX,
tileY)} with the
+     * same tile indices will cause an {@link ImagingOpException} to be thrown immediately
without invocation
+     * of {@code compute(tileX, tileY)}. If the cause of the error has been fixed, then users
should invoke
+     * {@link #clearErrorFlags(Rectangle)} for allowing the tile to be computed again.
+     *
      * @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}.
@@ -509,16 +518,19 @@ public abstract class ComputedImage extends PlanarImage {
      * @param  tileX    the X index of the tile to acquire or release.
      * @param  tileY    the Y index of the tile to acquire or release.
      * @param  writing  {@code true} for acquiring the tile, or {@code false} for releasing
it.
+     * @return {@code true} if the tile goes from having no writers to having one writer
+     *         (may happen if {@code writing} is {@code true}), or from having one writer
+     *         to no writers (may happen if {@code writing} is {@code false}).
      *
      * @see WritableRenderedImage#getWritableTile(int, int)
      * @see WritableRenderedImage#releaseWritableTile(int, int)
      */
-    protected void markTileWritable(final int tileX, final int tileY, final boolean writing)
{
+    protected boolean markTileWritable(final int tileX, final int tileY, final boolean writing)
{
         final TileCache.Key key = new TileCache.Key(reference, tileX, tileY);
         if (writing) {
-            reference.startWrite(key);
+            return reference.startWrite(key);
         } else {
-            reference.endWrite(key);
+            return reference.endWrite(key, true);
         }
     }
 
@@ -535,11 +547,29 @@ public abstract class ComputedImage extends PlanarImage {
      * for them.</p>
      *
      * @param  tiles  indices of tiles to mark as dirty.
+     * @return {@code true} if at least one tile has been marked dirty.
+     */
+    protected boolean markDirtyTiles(final Rectangle tiles) {
+        return reference.markDirtyTiles(tiles.x, tiles.y,
+                          Math.addExact(tiles.x, tiles.width  - 1),
+                          Math.addExact(tiles.y, tiles.height - 1), false);
+    }
+
+    /**
+     * Clears the error status of all tiles in the given range of indices.
+     * Those tiles will be marked as dirty and recomputed next time the the
+     * {@link #getTile(int, int)} method is invoked.
+     * The status of valid tiles is unchanged by this method call.
+     *
+     * @param  tiles  indices of tiles for which to clear the error status.
+     * @return {@code true} if at least one tile got its error flag cleared.
+     *
+     * @see #computeTile(int, int, WritableRaster)
      */
-    protected void markDirtyTiles(final Rectangle tiles) {
-        reference.markDirtyTiles(tiles.x, tiles.y,
-                   Math.addExact(tiles.x, tiles.width  - 1),
-                   Math.addExact(tiles.y, tiles.height - 1));
+    protected boolean clearErrorFlags(final Rectangle tiles) {
+        return reference.markDirtyTiles(tiles.x, tiles.y,
+                          Math.addExact(tiles.x, tiles.width  - 1),
+                          Math.addExact(tiles.y, tiles.height - 1), true);
     }
 
     /**
@@ -568,7 +598,7 @@ public abstract class ComputedImage extends PlanarImage {
         reference.markDirtyTiles(Numerics.clamp(Math.floorDiv(tx - (b == null ? 0 : b.left),
targetWidth)),
                                  Numerics.clamp(Math.floorDiv(ty - (b == null ? 0 : b.top),
 targetHeight)),
                                  Numerics.clamp(Math.floorDiv(tx + (b == null ? 0 : b.right)
 + sourceWidth  - 1, targetWidth)),
-                                 Numerics.clamp(Math.floorDiv(ty + (b == null ? 0 : b.bottom)
+ sourceHeight - 1, targetHeight)));
+                                 Numerics.clamp(Math.floorDiv(ty + (b == null ? 0 : b.bottom)
+ sourceHeight - 1, targetHeight)), false);
     }
 
     /**
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
index f88d493..d123c0e 100644
--- 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
@@ -24,6 +24,7 @@ import java.awt.Point;
 import java.awt.image.TileObserver;
 import java.awt.image.ImagingOpException;
 import java.awt.image.WritableRenderedImage;
+import org.apache.sis.internal.feature.Resources;
 import org.apache.sis.internal.system.ReferenceQueueConsumer;
 import org.apache.sis.util.Disposable;
 
@@ -142,7 +143,7 @@ final class ComputedTiles extends WeakReference<ComputedImage> implements
Dispos
         if (value != null) {
             switch (value) {
                 case DIRTY: break;
-                case ERROR: throw new ImagingOpException(key.error());
+                case ERROR: throw new ImagingOpException(key.error(Resources.Keys.TileErrorFlagSet_2));
                 default:    return false;
             }
         }
@@ -166,7 +167,7 @@ final class ComputedTiles extends WeakReference<ComputedImage> implements
Dispos
             }
         }
         if (value == ERROR) {
-            throw new ImagingOpException(key.error());
+            throw new ImagingOpException(key.error(Resources.Keys.TileErrorFlagSet_2));
         }
         return false;
     }
@@ -180,9 +181,9 @@ final class ComputedTiles extends WeakReference<ComputedImage> implements
Dispos
      * @throws ArithmeticException if too many writers.
      */
     final boolean startWrite(final TileCache.Key key) {
-        final Integer value;
+        Integer value = COMPUTING;                          // Do the boxing outside synchronized
block.
         synchronized (cachedTiles) {
-            value = cachedTiles.merge(key, COMPUTING, ComputedTiles::increment);
+            value = cachedTiles.merge(key, value, ComputedTiles::increment);
         }
         return value == COMPUTING;
     }
@@ -190,15 +191,17 @@ final class ComputedTiles extends WeakReference<ComputedImage>
implements Dispos
     /**
      * Decrements the count of writers for the specified tile.
      *
-     * @param  key  indices of the tile which was marked writable.
+     * @param  key      indices of the tile which was marked writable.
+     * @param  success  whether the operation should be considered successful.
      * @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;
+    final boolean endWrite(final TileCache.Key key, final boolean success) {
+        final int status = success ? VALID : ERROR;
+        Integer value = status;                             // Do the boxing outside synchronized
block.
         synchronized (cachedTiles) {
-            value = cachedTiles.merge(key, VALID, ComputedTiles::decrement);
+            value = cachedTiles.merge(key, value, ComputedTiles::decrement);
         }
-        return value == VALID;
+        return value == status;
     }
 
     /**
@@ -222,16 +225,16 @@ final class ComputedTiles extends WeakReference<ComputedImage>
implements Dispos
 
     /**
      * If the value is {@link #VALID}, {@link #DIRTY}, {@link #ERROR} or {@link #COMPUTING},
-     * sets it to {@link #VALID}. Otherwise decrements that value.
+     * sets that value to {@link #VALID} or {@link #ERROR}. Otherwise decrements that value.
      *
-     * @param  value  the value to decrement.
-     * @param  valid  must be {@link #VALID}.
+     * @param  value   the value to decrement.
+     * @param  status  {@link #VALID} or {@link #ERROR}.
      * @return the decremented value.
      */
-    private static Integer decrement(final Integer value, final Integer valid) {
+    private static Integer decrement(final Integer value, final Integer status) {
         final int n = value;
         if (n >= ERROR && n <= COMPUTING) {     // Do not use the ternary operator
here.
-            return valid;
+            return status;
         } else {
             return n - 1;
         }
@@ -262,17 +265,24 @@ final class ComputedTiles extends WeakReference<ComputedImage>
implements Dispos
      * This method is invoked when some tiles of at least one source image changed.
      * All arguments, including maximum values, are inclusive.
      *
+     * @param  error  {@code false} for marking valid tiles as dirty, or {@code true} for
marking tiles in error.
+     * @return {@code true} if at least one tile got its status updated.
+     *
      * @see ComputedImage#markDirtyTiles(Rectangle)
      */
-    final void markDirtyTiles(final int minTileX, final int minTileY, final int maxTileX,
final int maxTileY) {
+    final boolean markDirtyTiles(final int minTileX, final int minTileY, final int maxTileX,
final int maxTileY, final boolean error) {
+        final Integer search = error ? ERROR : VALID;
+        final Integer dirty  = DIRTY;
+        boolean updated = false;
         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);
+                    updated |= cachedTiles.replace(key, search, dirty);
                 }
             }
         }
+        return updated;
     }
 
     /**
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 48f92a1..f381bd3 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
@@ -111,9 +111,11 @@ final class TileCache extends Cache<TileCache.Key, Raster> {
 
         /**
          * Returns the error message when this tile can not be computed.
+         *
+         * @param  key  {@link Resources.Keys#CanNotComputeTile_2} or {@link Resources.Keys#TileErrorFlagSet_2}.
          */
-        final String error() {
-            return Resources.format(Resources.Keys.CanNotComputeTile_2, tileX, tileY);
+        final String error(final short key) {
+            return Resources.format(key, tileX, tileY);
         }
 
         /**
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java
index 97d152c..a221314 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.java
@@ -331,6 +331,11 @@ public final class Resources extends IndexedResourceBundle {
         public static final short PropertyNotFound_2 = 16;
 
         /**
+         * Tile ({0}, {1}) has the error flag set.
+         */
+        public static final short TileErrorFlagSet_2 = 68;
+
+        /**
          * Too many qualitative categories.
          */
         public static final short TooManyQualitatives = 48;
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties
b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties
index 23f473b..e611708 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources.properties
@@ -71,6 +71,7 @@ OptionalLibraryNotFound_2         = The {0} optional library is not available.
G
 OutOfIteratorDomain_2             = The ({0,number}, {1,number}) pixel coordinate is outside
iterator domain.
 PropertyAlreadyExists_2           = Property \u201c{1}\u201d already exists in feature \u201c{0}\u201d.
 PropertyNotFound_2                = No property named \u201c{1}\u201d has been found in \u201c{0}\u201d
feature.
+TileErrorFlagSet_2                = Tile ({0}, {1}) has the error flag set.
 TooManyQualitatives               = Too many qualitative categories.
 UnavailableGeometryLibrary_1      = The {0} geometry library is not available in current
runtime environment.
 UnconvertibleGridCoordinate_2     = Can not convert grid coordinate {1} to type \u2018{0}\u2019.
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties
b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties
index ba6fac4..972d7c8 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/feature/Resources_fr.properties
@@ -77,6 +77,7 @@ OutOfIteratorDomain_2             = La coordonn\u00e9e pixel ({0,number},
{1,num
 PointOutsideCoverageDomain_1      = Le point ({0}) est en dehors du domaine de la couverture
de donn\u00e9es.
 PropertyAlreadyExists_2           = La propri\u00e9t\u00e9 \u00ab\u202f{1}\u202f\u00bb existe
d\u00e9j\u00e0 dans l\u2019entit\u00e9 \u00ab\u202f{0}\u202f\u00bb.
 PropertyNotFound_2                = Aucune propri\u00e9t\u00e9 nomm\u00e9e \u00ab\u202f{1}\u202f\u00bb
n\u2019a \u00e9t\u00e9 trouv\u00e9e dans l\u2019entit\u00e9 \u00ab\u202f{0}\u202f\u00bb.
+TileErrorFlagSet_2                = La tuile ({0}, {1}) est marqu\u00e9e comme ayant une
erreur.
 TooManyQualitatives               = Trop de cat\u00e9gories qualitatives.
 UnavailableGeometryLibrary_1      = La biblioth\u00e8que de g\u00e9om\u00e9tries {0} n\u2019est
pas disponible dans l\u2019environnement d\u2019ex\u00e9cution actuel.
 UnconvertibleGridCoordinate_2     = Ne peut pas convertir la coordonn\u00e9e de grille {1}
vers le type \u2018{0}\u2019.
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 b4d34f5..7870728 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
@@ -16,16 +16,20 @@
  */
 package org.apache.sis.image;
 
+import java.awt.Point;
 import java.awt.Rectangle;
 import java.awt.image.Raster;
 import java.awt.image.WritableRaster;
 import java.awt.image.BufferedImage;
 import java.awt.image.ColorModel;
+import java.awt.image.ImagingOpException;
+import java.util.function.Consumer;
 import org.apache.sis.test.DependsOn;
 import org.apache.sis.test.TestCase;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
+import static org.opengis.test.Assert.assertInstanceOf;
 import static org.apache.sis.test.FeatureAssert.assertValuesEqual;
 
 
@@ -46,9 +50,21 @@ public final strictfp class ComputedImageTest extends TestCase {
     private static final int TILE_WIDTH = 3, TILE_HEIGHT = 2;
 
     /**
+     * Tile indices used in some methods performing their tests on only one tile.
+     */
+    private static final int TILE_X = 1, TILE_Y = 2;
+
+    /**
+     * Additional code to invoke during {@link ComputedImage#computeTile(int, int, WritableRaster)}
execution,
+     * or {@code null} if none.
+     */
+    private Consumer<ComputedImage> onComputeTile;
+
+    /**
      * Creates an image to test. The {@link ComputedImage} tiles are simply sub-regions of
a {@link BufferedImage}.
+     * If {@link #onComputeTile} is non-null, it will be invoked every time that a tile is
computed.
      */
-    private static ComputedImage createImage() {
+    private ComputedImage createImage() {
         final BufferedImage source = new BufferedImage(TILE_WIDTH * 2, TILE_HEIGHT * 4, BufferedImage.TYPE_USHORT_GRAY);
         final WritableRaster raster = source.getRaster();
         for (int y=raster.getHeight(); --y >= 0;) {
@@ -61,6 +77,8 @@ public final strictfp class ComputedImageTest extends TestCase {
             @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, WritableRaster
previous) {
+                final Consumer<ComputedImage> f = onComputeTile;
+                if (f != null) f.accept(this);
                 final int tw = getTileWidth();
                 final int th = getTileHeight();
                 return getSource(0).getData(new Rectangle(tileX * tw, tileY * th, tw, th));
@@ -113,4 +131,82 @@ public final strictfp class ComputedImageTest extends TestCase {
             {70, 71, 72, 73, 74, 75}
         });
     }
+
+    /**
+     * Tests {@link ComputedImage#hasTileWriters()}, {@link ComputedImage#isTileWritable(int,
int)}
+     * and {@link ComputedImage#getWritableTileIndices()}.
+     */
+    @Test
+    public void testHasTileWriters() {
+        onComputeTile = ComputedImageTest::verifyNoWrite;
+        final ComputedImage image = createImage();
+        verifyWritableTiles(image, (Point[]) null);
+        /*
+         * During execution of ComputedImage.computeTile(1, 2),
+         * the writable tile indices should be set to (1, 2).
+         */
+        onComputeTile = ComputedImageTest::verifyWriting;
+        final Raster tile = image.getTile(TILE_X, TILE_Y);
+        /*
+         * After tile computation we should be back to no writable tile.
+         */
+        verifyWritableTiles(image, (Point[]) null);
+        onComputeTile = ComputedImageTest::verifyNoWrite;
+        assertSame(tile, image.getTile(TILE_X, TILE_Y));
+    }
+
+    /**
+     * Callback method invoked during {@code ComputedImage.computeTile(…)} execution.
+     */
+    private static void verifyWriting(final ComputedImage image) {verifyWritableTiles(image,
new Point(TILE_X, TILE_Y));}
+    private static void verifyNoWrite(final ComputedImage image) {verifyWritableTiles(image,
(Point[]) null);}
+
+    /**
+     * Asserts that the writable tiles indices are the expected ones.
+     * If non-null, the given indices shall contain (1,2) and shall not contain (2,1).
+     */
+    private static void verifyWritableTiles(final ComputedImage image, final Point... expected)
{
+        assertFalse      ("isTileWritable",                   image.isTileWritable(2, 1));
+        assertEquals     ("isTileWritable", null != expected, image.isTileWritable(TILE_X,
TILE_Y));
+        assertEquals     ("hasTileWriters", null != expected, image.hasTileWriters());
+        assertArrayEquals("getWritableTileIndices", expected, image.getWritableTileIndices());
+    }
+
+    /**
+     * Verifies that a tile that failed to compute will not be computed again, unless we
mark it as dirty.
+     */
+    @Test
+    public void testErrorFlag() {
+        onComputeTile = ComputedImageTest::makeError;
+        final ComputedImage image = createImage();
+        try {
+            image.getTile(TILE_X, TILE_Y);
+            fail("Computation should have failed.");
+        } catch (ImagingOpException e) {
+            assertInstanceOf("cause", IllegalStateException.class, e.getCause());
+        }
+        /*
+         * Ask again for the same tile. ComputedTile should have set a flag for remembering
+         * that the computation of this tile failed, and should not try to compute it again.
+         */
+        onComputeTile = ComputedImageTest::notInvoked;
+        try {
+            image.getTile(TILE_X, TILE_Y);
+            fail("Computation should have failed.");
+        } catch (ImagingOpException e) {
+            assertNull("cause", e.getCause());
+        }
+        /*
+         * Clearing the error flag should allow to compute the tile again.
+         */
+        onComputeTile = null;
+        assertTrue(image.clearErrorFlags(new Rectangle(TILE_X, TILE_Y, 1, 1)));
+        assertNotNull(image.getTile(TILE_X, TILE_Y));
+    }
+
+    /**
+     * Callback method invoked during {@code ComputedImage.computeTile(…)} execution.
+     */
+    private static void makeError (final ComputedImage image) {throw new IllegalStateException("Testing
an error");}
+    private static void notInvoked(final ComputedImage image) {fail("Should not be invoked.");}
 }


Mime
View raw message