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 <[email protected]>
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.");}
}