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: Rename PropertyCalculator as AnnotatedImage (the properties are the annotations) and expose less implementation details. Allow sequential computation for the RenderedImage implementations that are not thread-safe. Add tests.
Date Fri, 28 Feb 2020 12:09:50 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 7514d35  Rename PropertyCalculator as AnnotatedImage (the properties are the annotations)
and expose less implementation details. Allow sequential computation for the RenderedImage
implementations that are not thread-safe. Add tests.
7514d35 is described below

commit 7514d35cb5a46b4be5c7dc7eb9bd4c240ea39c49
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Fri Feb 28 13:07:55 2020 +0100

    Rename PropertyCalculator as AnnotatedImage (the properties are the annotations) and expose
less implementation details.
    Allow sequential computation for the RenderedImage implementations that are not thread-safe.
Add tests.
---
 .../apache/sis/internal/gui/BackgroundThreads.java |   4 +-
 .../org/apache/sis/gui/coverage/GridViewApp.java   |   3 +-
 .../AnnotatedImage.java}                           | 196 ++++++++++++++-------
 .../java/org/apache/sis/image/ImageOperations.java |  26 ++-
 .../java/org/apache/sis/image/PixelIterator.java   |   5 +-
 .../org/apache/sis/image/StatisticsCalculator.java | 126 +++++++++----
 .../sis/internal/coverage/j2d/TileOpExecutor.java  |  10 +-
 .../apache/sis/image/StatisticsCalculatorTest.java |  94 ++++++++++
 .../java/org/apache/sis/image/TiledImageMock.java  |  23 +++
 .../apache/sis/test/suite/FeatureTestSuite.java    |   1 +
 10 files changed, 373 insertions(+), 115 deletions(-)

diff --git a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
index 9ea868e..de235b9 100644
--- a/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
+++ b/application/sis-javafx/src/main/java/org/apache/sis/internal/gui/BackgroundThreads.java
@@ -67,7 +67,9 @@ public final class BackgroundThreads extends AtomicInteger implements ThreadFact
      */
     @Override
     public Thread newThread(final Runnable r) {
-        return new Thread(Threads.SIS, r, "Application worker #" + incrementAndGet());
+        final Thread t = new Thread(Threads.SIS, r, "Application worker #" + incrementAndGet());
+        t.setPriority(Thread.NORM_PRIORITY - 1);      // Let JavaFX thread have higher priority.
+        return t;
     }
 
     /**
diff --git a/application/sis-javafx/src/test/java/org/apache/sis/gui/coverage/GridViewApp.java
b/application/sis-javafx/src/test/java/org/apache/sis/gui/coverage/GridViewApp.java
index 81508eb..19c2a82 100644
--- a/application/sis-javafx/src/test/java/org/apache/sis/gui/coverage/GridViewApp.java
+++ b/application/sis-javafx/src/test/java/org/apache/sis/gui/coverage/GridViewApp.java
@@ -39,7 +39,7 @@ public final strictfp class GridViewApp extends Application {
     /**
      * Size of the artificial tiles. Should be small enough so we can have many of them.
      * Width and height should be different in order to increase the chance to see bugs
-     * if some code confused them.
+     * if some code confuse them.
      */
     private static final int TILE_WIDTH = 10, TILE_HEIGHT = 15;
 
@@ -75,7 +75,6 @@ public final strictfp class GridViewApp extends Application {
      * have artificial errors in order to see the error controls.
      */
     private static TiledImageMock createImage() {
-        final Random random = new Random();
         final TiledImageMock image = new TiledImageMock(
                 DataBuffer.TYPE_USHORT, 1,
                 -50,                            // minX
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/PropertyCalculator.java
b/core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
similarity index 50%
rename from core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/PropertyCalculator.java
rename to core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
index 114ed01..86bbf28 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/PropertyCalculator.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/AnnotatedImage.java
@@ -14,16 +14,12 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.apache.sis.internal.coverage.j2d;
+package org.apache.sis.image;
 
 import java.util.Locale;
 import java.util.Vector;
 import java.util.logging.Level;
 import java.util.logging.LogRecord;
-import java.util.function.Supplier;
-import java.util.function.Consumer;
-import java.util.function.BiConsumer;
-import java.util.function.BinaryOperator;
 import java.util.stream.Collector;
 import java.awt.Image;
 import java.awt.Rectangle;
@@ -32,16 +28,16 @@ import java.awt.image.SampleModel;
 import java.awt.image.RenderedImage;
 import java.awt.image.Raster;
 import java.awt.image.WritableRaster;
-import java.awt.image.ImagingOpException;
 import org.apache.sis.util.ArraysExt;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.internal.system.Modules;
+import org.apache.sis.internal.coverage.j2d.TileOpExecutor;
 
 
 /**
- * An image which wraps an existing image unchanged, except for a property which is computed
- * on the fly when first requested. All methods delegate to the wrapped image except the
one
- * for getting the property value and {@link #getSources()}.
+ * An image which wraps an existing image unchanged, except for properties which are computed
+ * on the fly when first requested. All {@link RenderedImage} methods delegate to the wrapped
+ * image except {@link #getSources()} and the methods for getting the property names or values.
  *
  * <p>The name of the computed property is given by {@link #getComputedPropertyName()}.
  * In addition this method automatically creates another property with the same name
@@ -50,27 +46,15 @@ import org.apache.sis.internal.system.Modules;
  * The computation results are cached by this class.</p>
  *
  * <div class="note"><b>Design note:</b>
- * most non-abstract methods are final because {@link org.apache.sis.image.PixelIterator}
- * (among others) relies on the fact that it can unwrap this image and still get the same
- * pixel values.</div>
- *
- * This class implements various {@link java.util.function} interfaces for implementation
convenience
- * (would not be recommended for public API, but this is an internal class). Users should
not rely on
- * this fact. Compared to lambda functions, this is one less level of indirection and makes
stack traces
- * a little bit shorter to analyze in case of exceptions.
+ * most non-abstract methods are final because {@link PixelIterator} (among others) relies
+ * on the fact that it can unwrap this image and still get the same pixel values.</div>
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
- *
- * @param  <A>  type of the thread-local object (the accumulator) for holding intermediate
results during computation.
- *              This is usually the final type of the property value, but not necessarily.
- *
- * @since 1.1
+ * @since   1.1
  * @module
  */
-public abstract class PropertyCalculator<A> implements RenderedImage,
-        Supplier<A>, BinaryOperator<A>, BiConsumer<A, Raster>, Consumer<LogRecord>
-{
+abstract class AnnotatedImage implements RenderedImage {
     /**
      * The suffix to add to property name for errors that occurred during computation.
      */
@@ -79,7 +63,7 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
     /**
      * The source image from which to compute the property.
      */
-    public final RenderedImage source;
+    protected final RenderedImage source;
 
     /**
      * The computation result, or {@link Image#UndefinedProperty} if not yet computed.
@@ -94,20 +78,27 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
     private LogRecord errors;
 
     /**
-     * Creates a new calculator wrapping the given image.
+     * Whether parallel execution is authorized for the {@linkplain #source} image.
+     */
+    private final boolean parallel;
+
+    /**
+     * Creates a new annotated image wrapping the given image.
+     * The annotations are the additional properties computed by the subclass.
      *
-     * @param  source  the image to wrap.
+     * @param  source    the image to wrap for adding properties (annotations).
+     * @parma  parallel  whether parallel execution is authorized.
      */
-    protected PropertyCalculator(final RenderedImage source) {
-        this.source = source;
+    protected AnnotatedImage(final RenderedImage source, final boolean parallel) {
+        this.source   = source;
+        this.parallel = parallel;
         result = Image.UndefinedProperty;
     }
 
     /**
-     * Returns the source of this image. The default implementation
-     * returns {@link #source} in an vector of length 1.
+     * Returns the {@linkplain #source} of this image in an vector of length 1.
      *
-     * @return the source (usually only {@linkplain #source}) of this image.
+     * @return the unique {@linkplain #source} of this image.
      */
     @Override
     @SuppressWarnings("UseOfObsoleteCollectionType")
@@ -118,7 +109,7 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
     }
 
     /**
-     * If the property should be computed on a subset of the tiles,
+     * If the properties should be computed on a subset of the tiles,
      * the pixel coordinates of the region intersecting those tiles.
      * The default implementation returns {@code null}.
      *
@@ -143,7 +134,7 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
      * @return all recognized property names.
      */
     @Override
-    public final String[] getPropertyNames() {
+    public String[] getPropertyNames() {
         final String name = getComputedPropertyName();
         return ArraysExt.concatenate(source.getPropertyNames(), new String[] {name, name
+ ERRORS_SUFFIX});
     }
@@ -164,7 +155,7 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
     /**
      * Gets a property from this image or from its source. If the given name is for the property
      * to be computed by this class and if that property has not been computed before, then
this
-     * method starts computation now and caches the result.
+     * method invokes {@link #computeProperty()} and caches the result.
      *
      * @param  name  name of the property to get.
      * @return the property for the given name (may be {@code null}).
@@ -176,9 +167,22 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
             final boolean isProperty = cn.equals(name);
             if (isProperty || isErrorProperty(cn, name)) {
                 synchronized (this) {
-                    if (result == Image.UndefinedProperty) {
-                        final TileOpExecutor executor = new TileOpExecutor(source, getAreaOfInterest());
-                        result = executor.executeOnReadable(source, Collector.of(this, this,
this), this);
+                    if (result == Image.UndefinedProperty) try {
+                        result = computeProperty();
+                    } catch (Exception e) {
+                        result = null;
+                        if (errors != null) {
+                            errors.getThrown().addSuppressed(e);
+                        } else {
+                            /*
+                             * Stores the given exception in a log record. We use a log record
in order to initialize
+                             * the timestamp and thread ID to the values they had at the
time the first error occurred.
+                             */
+                            final LogRecord record = Errors.getResources((Locale) null).getLogRecord(
+                                                        Level.WARNING, Errors.Keys.CanNotCompute_1,
cn);
+                            record.setThrown(e);
+                            setError(record);
+                        }
                     }
                     return isProperty ? result : errors;
                 }
@@ -189,58 +193,103 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
 
     /**
      * Invoked by {@link TileOpExecutor} if an error occurred while processing tiles.
-     * This method should be invoked at most once.
+     * Can also be invoked by {@link #getProperty(String)} directly.
+     * This method shall be invoked at most once.
      *
      * @param  record  a description of the error that occurred.
      */
-    @Override
-    public final synchronized void accept(final LogRecord record) {
+    private synchronized void setError(final LogRecord record) {
         if (errors != null) {
             throw new IllegalStateException();      // Should never happen.
         }
         /*
-         * Completes the record with source identification as if the
-         * error occurred from above `getProperty(String)` method.
+         * Complete record with source identification as if the error occurred from
+         * above `getProperty(String)` method (this is always the case, indirectly).
          */
-        record.setSourceClassName(RenderedImage.class.getCanonicalName());
+        record.setSourceClassName(AnnotatedImage.class.getCanonicalName());
         record.setSourceMethodName("getProperty");
         record.setLoggerName(Modules.RASTER);
         errors = record;
     }
 
     /**
-     * Invoked for creating the object holding the information to be computed by a single
thread.
-     * This method will be invoked for each worker thread before the worker starts its execution.
+     * Invoked when the property needs to be computed. If the property can not be computed,
+     * then the result will be {@code null} and the exception thrown by this method will
be
+     * wrapped in a property of the same name with the {@value #ERRORS_SUFFIX} suffix.
      *
-     * @return a thread-local variable holding information computed by a single thread.
-     *         May be {@code null} is such objects are not needed.
+     * <p>The default implementation makes the following choice:</p>
+     * <ul class="verbose">
+     *   <li>If {@link #parallel} is {@code true} and the {@linkplain #getAreaOfInterest()
area of interest}
+     *       covers at least two tiles and {@link #collector()} returned a non-null value,
then this method
+     *       distributes the calculation of many threads using the functions provided by
the collector.
+     *       See {@link #collector()} Javadoc for more information.</li>
+     *   <li>Otherwise this method delegates to {@link #computeSequentially()}.</li>
+     * </ul>
+     *
+     * @return the property value (may be {@code null}).
+     * @throws Exception if an error occurred while computing the property.
      */
-    @Override
-    public abstract A get();
+    protected Object computeProperty() throws Exception {
+        if (parallel) {
+            final TileOpExecutor executor = new TileOpExecutor(source, getAreaOfInterest());
+            if (executor.isMultiTiled()) {
+                final Collector<? super Raster,?,?> collector = collector();
+                if (collector != null) {
+                    return executor.executeOnReadable(source, collector(), this::setError);
+                }
+            }
+        }
+        return computeSequentially();
+    }
 
     /**
-     * Invoked after a thread finished to process all its tiles and wants to combine its
result with the
-     * result of another thread. This method is invoked only if {@link #get()} returned a
non-null value.
-     * This method does not need to be thread-safe; synchronizations will be done by the
caller.
+     * Invoked when the property needs to be computed sequentially (all computations in current
thread).
+     * If the property can not be computed, then the result will be {@code null} and the
exception thrown
+     * by this method will be wrapped in a property of the same name with the {@value #ERRORS_SUFFIX}
suffix.
+     *
+     * <p>This method is invoked when this class does not support parallel execution
({@link #collector()}
+     * returned {@code null}), or when it is not worth to parallelize (image has only one
tile), or when
+     * the {@linkplain #source} image may be non-thread safe ({@link #parallel} is {@code
false}).</p>
      *
-     * @param  previous  the result of another thread (never {@code null}).
-     * @param  computed  the result computed by current thread (never {@code null}).
-     * @return combination of the two results. May be one of the {@code previous} or {@code
computed} instances.
+     * @return the property value (may be {@code null}).
+     * @throws Exception if an error occurred while computing the property.
      */
-    @Override
-    public abstract A apply(A previous, A computed);
+    protected abstract Object computeSequentially() throws Exception;
 
     /**
-     * Executes this operation on the given tile. This method may be invoked from any thread.
-     * If an exception occurs during computation, that exception will be logged or wrapped
in
-     * an {@link ImagingOpException} by the caller {@link TileOpExecutor}.
+     * Returns the function to execute for computing the property value, together with other
required functions
+     * (supplier of accumulator, combiner, finisher). Those functions allow multi-threaded
property calculation.
+     * This collector is used in a way similar to {@link java.util.stream.Stream#collect(Collector)}.
A typical
+     * approach is two define 3 private methods in the subclass as below (where <var>P</var>
is the type of the
+     * property to compute):
+     *
+     * {@preformat java
+     *     private P createAccumulator() {
+     *         // Create an object holding the information to be computed by a single thread.
+     *         // This is invoked for each worker thread before the worker starts its execution.
+     *     }
+     *
+     *     private static P combine(P previous, P computed) {
+     *         // Invoked after a thread finished to process all its tiles and
+     *         // wants to combine its result with the result of another thread.
+     *     }
+     *
+     *     private static void compute(P accumulator, Raster tile) {
+     *         // Perform the actual computation using one tile and update the accumulator
with the result.
+     *         // The accumulator may already contain data, which need to be augmented (not
overwritten).
+     *     }
      *
-     * @param  accumulator  the thread-local variable created by {@link #get()}. May be {@code
null}.
-     * @param  tile         the tile on which to perform a computation.
-     * @throws RuntimeException if the calculation failed.
+     *     &#64;Override
+     *     protected Collector<Raster,P,P> collector() {
+     *         return Collector.of(this::createAccumulator, MyClass::compute, MyClass::combine);
+     *     }
+     * }
+     *
+     * @return functions for multi-threaded computation of property value, or {@code null}
if unsupported.
      */
-    @Override
-    public abstract void accept(A accumulator, Raster tile);
+    protected Collector<? super Raster, ?, ?> collector() {
+        return null;
+    }
 
     /** Delegates to the wrapped image. */
     @Override public final ColorModel     getColorModel()            {return source.getColorModel();}
@@ -261,5 +310,16 @@ public abstract class PropertyCalculator<A> implements RenderedImage,
     @Override public final Raster         getData()                  {return source.getData();}
     @Override public final Raster         getData(Rectangle region)  {return source.getData(region);}
     @Override public final WritableRaster copyData(WritableRaster r) {return source.copyData(r);}
-    @Override public final String         toString()                 {return source.toString();}
+
+    /**
+     * Returns a string representation of this image for debugging purpose.
+     */
+    @Override
+    public String toString() {
+        final StringBuilder buffer = new StringBuilder(100).append(AnnotatedImage.class.getSimpleName()).append('[');
+        if (result != Image.UndefinedProperty) {
+            buffer.append("Cached ");
+        }
+        return buffer.append("[\"").append(getComputedPropertyName()).append("\" on ").append(source).append(']').toString();
+    }
 }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/ImageOperations.java b/core/sis-feature/src/main/java/org/apache/sis/image/ImageOperations.java
index 97956dd..71bae50 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/ImageOperations.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/ImageOperations.java
@@ -31,14 +31,32 @@ import org.apache.sis.util.ArgumentChecks;
  */
 public final class ImageOperations {
     /**
-     * The default set of operations in which failures to compute cause an exception to be
thrown.
+     * The set of operations with default configuration. Operations executed by this instance
+     * will be multi-threaded if possible, and failures to compute a value cause an exception
+     * to be thrown.
      */
-    public static final ImageOperations STRICT = new ImageOperations();
+    public static final ImageOperations DEFAULT = new ImageOperations(true);
+
+    /**
+     * The set of operations where all executions are constrained to a single thread.
+     * Only the caller thread is used, with no parallelization. Sequential operations
+     * may be useful for processing {@link RenderedImage} that may not be thread-safe.
+     * The error handling policy is the same than {@link #DEFAULT}.
+     */
+    public static final ImageOperations SEQUENTIAL = new ImageOperations(false);
+
+    /**
+     * Whether the operations can be executed in parallel.
+     */
+    private final boolean parallel;
 
     /**
      * Creates a new set of image operations.
+     *
+     * @param  parallel  whether the operations can be executed in parallel.
      */
-    private ImageOperations() {
+    private ImageOperations(final boolean parallel) {
+        this.parallel = parallel;
     }
 
     /**
@@ -49,7 +67,7 @@ public final class ImageOperations {
      */
     public Statistics[] statistics(final RenderedImage source) {
         ArgumentChecks.ensureNonNull("source", source);
-        final StatisticsCalculator calculator = new StatisticsCalculator(source);
+        final StatisticsCalculator calculator = new StatisticsCalculator(source, parallel);
         final Object property = calculator.getProperty(StatisticsCalculator.PROPERTY_NAME);
         if (property instanceof Statistics[]) {
             // TODO: check error condition.
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java b/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java
index d7faa55..a90d5c7 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/PixelIterator.java
@@ -36,7 +36,6 @@ import org.opengis.coverage.grid.SequenceType;
 import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.ArgumentChecks;
 import org.apache.sis.measure.NumberRange;
-import org.apache.sis.internal.coverage.j2d.PropertyCalculator;
 
 import static java.lang.Math.floorDiv;
 import static org.apache.sis.internal.util.Numerics.ceilDiv;
@@ -291,8 +290,8 @@ public abstract class PixelIterator {
          * also necessary for allowing the builder to recognize the {@link BufferedImage}
case.
          */
         private static RenderedImage unwrap(RenderedImage image) {
-            while (image instanceof PropertyCalculator) {
-                image = ((PropertyCalculator) image).source;
+            while (image instanceof AnnotatedImage) {
+                image = ((AnnotatedImage) image).source;
             }
             return image;
         }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/image/StatisticsCalculator.java
b/core/sis-feature/src/main/java/org/apache/sis/image/StatisticsCalculator.java
index 89a4d81..12b3ebc 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/image/StatisticsCalculator.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/image/StatisticsCalculator.java
@@ -18,32 +18,36 @@ package org.apache.sis.image;
 
 import java.awt.image.Raster;
 import java.awt.image.RenderedImage;
+import java.awt.image.ImagingOpException;
+import java.util.stream.Collector;
 import org.apache.sis.math.Statistics;
 import org.apache.sis.util.resources.Vocabulary;
-import org.apache.sis.internal.coverage.j2d.PropertyCalculator;
 
 
 /**
- * Computes statistics on all pixel values of an image.
+ * Computes statistics on all pixel values of an image. The results are stored in an array
+ * of {@link Statistics} objects (one per band) in a property named {@value #PROPERTY_NAME}.
+ * The statistics can be computed in parallel or sequentially for non thread-safe images.
  *
  * @author  Martin Desruisseaux (Geomatys)
  * @version 1.1
  * @since   1.1
  * @module
  */
-final class StatisticsCalculator extends PropertyCalculator<Statistics[]> {
+final class StatisticsCalculator extends AnnotatedImage {
     /**
      * Name of the property computed by this class.
      */
-    static final String PROPERTY_NAME = "statistics";
+    static final String PROPERTY_NAME = "org.apache.sis.image.statistics";
 
     /**
      * Creates a new calculator.
      *
-     * @param  image  the image for which to compute statistics.
+     * @param  image     the image for which to compute statistics.
+     * @parma  parallel  whether parallel execution is authorized.
      */
-    StatisticsCalculator(final RenderedImage image) {
-        super(image);
+    StatisticsCalculator(final RenderedImage image, final boolean parallel) {
+        super(image, parallel);
     }
 
     /**
@@ -55,29 +59,90 @@ final class StatisticsCalculator extends PropertyCalculator<Statistics[]>
{
     }
 
     /**
-     * Invoked for creating the objects holding the statistics to be computed by a single
thread.
-     * This method will be invoked for each worker threads.
+     * Creates the objects where to add sample values for computing statistics.
+     * We will have one accumulator for each band in the source image.
+     * This is used for both sequential and parallel executions.
      */
-    @Override
-    public Statistics[] get() {
-        final Statistics[] stats = new Statistics[source.getSampleModel().getNumBands()];
-        for (int i=0; i<stats.length; i++) {
+    private static Statistics[] createAccumulator(final int numBands) {
+        final Statistics[] stats = new Statistics[numBands];
+        for (int i=0; i<numBands; i++) {
             stats[i] = new Statistics(Vocabulary.formatInternational(Vocabulary.Keys.Band_1,
i));
         }
         return stats;
     }
 
     /**
-     * Invoked after a thread finished to process all its tiles and wants to combine its
statistics
-     * with the ones computed by another thread. This method does not need to be thread-safe;
-     * synchronizations will be done by the caller.
+     * Computes statistics using the given iterator and accumulates the result for all bands.
+     * This method is invoked in both sequential and parallel case. In the sequential case
it
+     * is invoked for the whole image; in the parallel case it is invoked for only one tile.
+     *
+     * @param accumulator  where to accumulate the statistics results.
+     * @param it           the iterator on a raster or on the whole image.
+     */
+    private static void compute(final Statistics[] accumulator, final PixelIterator it) {
+        double[] samples = null;
+        while (it.next()) {
+            samples = it.getPixel(samples);                 // Get values in all bands.
+            for (int i=0; i<accumulator.length; i++) {
+                accumulator[i].accept(samples[i]);
+            }
+        }
+    }
+
+    /**
+     * Computes statistics on the given image in a sequential way (everything computed in
current thread).
+     * This is used for testing purposes, or when the image has only one tile, or when the
implementation
+     * of {@link RenderedImage#getTile(int, int)} may be non thread-safe.
      *
-     * @param  previous  the statistics computed by another thread (never {@code null}).
-     * @param  computed  the statistics computed by current thread (never {@code null}).
-     * @return combination of the two results, stored in {@code previous} instances.
+     * @param  source  the image on which to compute statistics.
+     * @return statistics on the given image computed sequentially.
+     */
+    static Statistics[] computeSequentially(final RenderedImage source) {
+        final PixelIterator it = PixelIterator.create(source);
+        final Statistics[] accumulator = createAccumulator(it.getNumBands());
+        compute(accumulator, it);
+        return accumulator;
+    }
+
+    /**
+     * Computes the statistics on the whole image using a single thread. This method is invoked
when it is
+     * not worth to parallelize (image has only one tile), or when the source image may be
non-thread safe.
+     */
+    @Override
+    protected Object computeSequentially() {
+        return computeSequentially(source);
+    }
+
+    /**
+     * Returns the function to execute for parallel computation of statistics,
+     * together with other required functions (supplier of accumulator, combiner, finisher).
      */
     @Override
-    public Statistics[] apply(final Statistics[] previous, final Statistics[] computed) {
+    protected Collector<Raster, Statistics[], Statistics[]> collector() {
+        return Collector.of(this::createAccumulator, StatisticsCalculator::compute, StatisticsCalculator::combine);
+    }
+
+    /**
+     * Invoked for creating the object holding the information to be computed by a single
thread.
+     * This method will be invoked for each worker thread before the worker starts its execution.
+     *
+     * @return a thread-local variable holding information computed by a single thread.
+     *         May be {@code null} is such objects are not needed.
+     */
+    private Statistics[] createAccumulator() {
+        return createAccumulator(source.getSampleModel().getNumBands());
+    }
+
+    /**
+     * Invoked after a thread finished to process all its tiles and wants to combine its
result with the
+     * result of another thread. This method is invoked only if {@link #createAccumulator()}
returned a non-null value.
+     * This method does not need to be thread-safe; synchronizations will be done by the
caller.
+     *
+     * @param  previous  the result of another thread (never {@code null}).
+     * @param  computed  the result computed by current thread (never {@code null}).
+     * @return combination of the two results. May be one of the {@code previous} or {@code
computed} instances.
+     */
+    private static Statistics[] combine(final Statistics[] previous, final Statistics[] computed)
{
         for (int i=0; i<computed.length; i++) {
             previous[i].combine(computed[i]);
         }
@@ -85,20 +150,15 @@ final class StatisticsCalculator extends PropertyCalculator<Statistics[]>
{
     }
 
     /**
-     * Invoked for computing statistics on all pixel values in a raster.
+     * Executes this operation on the given tile. This method may be invoked from any thread.
+     * If an exception occurs during computation, that exception will be logged or wrapped
in
+     * an {@link ImagingOpException} by the caller.
      *
-     * @param  accumulator  where to store statistics.
-     * @param  tile         the tile for which to compute statistics.
+     * @param  accumulator  the thread-local variable created by {@link #createAccumulator()}.
+     * @param  tile         the tile on which to perform a computation.
+     * @throws RuntimeException if the calculation failed.
      */
-    @Override
-    public void accept(final Statistics[] accumulator, final Raster tile) {
-        final PixelIterator it = new PixelIterator.Builder().create(tile);
-        double[] samples = null;
-        while (it.next()) {
-            samples = it.getPixel(samples);         // Get values in all bands.
-            for (int i=0; i<samples.length; i++) {
-                accumulator[i].accept(samples[i]);
-            }
-        }
+    private static void compute(final Statistics[] accumulator, final Raster tile) {
+        compute(accumulator, new PixelIterator.Builder().create(tile));
     }
 }
diff --git a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
index f94e9f4..3d21ada 100644
--- a/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
+++ b/core/sis-feature/src/main/java/org/apache/sis/internal/coverage/j2d/TileOpExecutor.java
@@ -86,7 +86,7 @@ public class TileOpExecutor {
      * @param  aoi    region of interest, or {@code null} for the whole image.
      * @throws ArithmeticException if some tile indices are too large.
      */
-    protected TileOpExecutor(final RenderedImage image, final Rectangle aoi) {
+    public TileOpExecutor(final RenderedImage image, final Rectangle aoi) {
         if (aoi != null) {
             final int  tileWidth       = image.getTileWidth();
             final int  tileHeight      = image.getTileHeight();
@@ -105,10 +105,12 @@ public class TileOpExecutor {
     }
 
     /**
-     * Returns {@code true} if this executor will apply to two tiles or more.
-     * Returns {@code false} if it will apply to a single tile or no tile at all.
+     * Returns {@code true} if the region of interest covers at least two tiles.
+     * Returns {@code false} if the region of interest covers a single tile or no tile at
all.
+     *
+     * @return whether the operation will be executed on two tiles or more.
      */
-    private boolean isMultiTiled() {
+    public final boolean isMultiTiled() {
         /*
          * Following expression is negative if at least one (max - min) value is negative
          * (empty case), and 0 if all (max - min) values are zero (singleton case).
diff --git a/core/sis-feature/src/test/java/org/apache/sis/image/StatisticsCalculatorTest.java
b/core/sis-feature/src/test/java/org/apache/sis/image/StatisticsCalculatorTest.java
new file mode 100644
index 0000000..1bdc0a3
--- /dev/null
+++ b/core/sis-feature/src/test/java/org/apache/sis/image/StatisticsCalculatorTest.java
@@ -0,0 +1,94 @@
+/*
+ * 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.Random;
+import java.awt.image.DataBuffer;
+import org.apache.sis.math.Statistics;
+import org.apache.sis.test.TestCase;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+
+/**
+ * Tests {@link StatisticsCalculator}. This will also (indirectly) tests
+ * {@link org.apache.sis.internal.coverage.j2d.TileOpExecutor} with multi-threading.
+ *
+ * @author  Martin Desruisseaux (Geomatys)
+ * @version 1.1
+ * @since   1.1
+ * @module
+ */
+public final strictfp class StatisticsCalculatorTest extends TestCase {
+    /**
+     * Size of the artificial tiles. Should be small enough so we can have many of them.
+     * Width and height should be different in order to increase the chance to see bugs
+     * if some code confuse them.
+     */
+    private static final int TILE_WIDTH = 5, TILE_HEIGHT = 3;
+
+    /**
+     * Creates a dummy image for testing purpose. This image will contain many small tiles
+     * of two bands. The first bands has deterministic values and the second band contains
+     * random values.
+     */
+    private static TiledImageMock createImage() {
+        final TiledImageMock image = new TiledImageMock(
+                DataBuffer.TYPE_USHORT, 2,
+                +51,                            // minX
+                -72,                            // minY
+                TILE_WIDTH  * 27,               // width
+                TILE_HEIGHT * 19,               // height
+                TILE_WIDTH,
+                TILE_HEIGHT,
+                -3,                             // minTileX
+                +2);                            // minTileY
+        image.initializeAllTiles(0);
+        image.setRandomValues(1, new Random(), 1000);
+        image.validate();
+        return image;
+    }
+
+    /**
+     * Tests with parallel execution. The result of sequential execution is used as a reference.
+     */
+    @Test
+    public void testParallelExecution() {
+        final TiledImageMock image = createImage();
+        final Statistics[] expected = StatisticsCalculator.computeSequentially(image);
+        final Statistics[] actual = ImageOperations.DEFAULT.statistics(image);
+        for (int i=0; i<expected.length; i++) {
+            final Statistics e = expected[i];
+            final Statistics a = actual  [i];
+            assertEquals("minimum", e.minimum(), a.minimum(), STRICT);
+            assertEquals("maximum", e.maximum(), a.maximum(), STRICT);
+            assertEquals("sum",     e.sum(),     a.sum(),     STRICT);
+        }
+    }
+
+    /**
+     * Tests with random failures.
+     */
+    @Test
+    public void testWithFailures() {
+        final TiledImageMock image = createImage();
+        image.failRandomly(new Random());
+        final Statistics[] stats = ImageOperations.DEFAULT.statistics(image);
+        // TODO: clarify the policy on error handling.
+    }
+}
diff --git a/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java b/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java
index b96c187..690d2fe 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/image/TiledImageMock.java
@@ -217,6 +217,9 @@ public final strictfp class TiledImageMock extends PlanarImage implements
Writab
      *   <li><var>Y</var> is the <var>y</var> coordinate (row
index) of the sample value relative to current tile.</li>
      * </ul>
      *
+     * The "TXY" pattern holds if all values are less than 10. If some values are greater
than 10,
+     * then the sample values are a mix of above values resulting from arithmetic sums.
+     *
      * @param  band  band index where to set values. Other bands will be unmodified.
      */
     public void initializeAllTiles(final int band) {
@@ -239,6 +242,26 @@ public final strictfp class TiledImageMock extends PlanarImage implements
Writab
     }
 
     /**
+     * Initializes the sample values of all tiles to random values. The image must have been
+     * initialized by a call to {@link #initializeAllTiles(int)} before to invoke this method.
+     *
+     * @param  band       band index where to set values. Other bands will be unmodified.
+     * @param  generator  the random number generator to use for obtaining values.
+     * @param  upper      upper limit (exclusive) of random numbers to generate.
+     */
+    public void setRandomValues(final int band, final Random generator, final int upper)
{
+        for (final WritableRaster raster : tiles) {
+            final int x = raster.getMinX();
+            final int y = raster.getMinY();
+            for (int j=0; j<tileHeight; j++) {
+                for (int i=0; i<tileWidth; i++) {
+                    raster.setSample(x+i, y+j, band, generator.nextInt(upper));
+                }
+            }
+        }
+    }
+
+    /**
      * Returns the tile at the given location in tile coordinates.
      * This method verifies that no writable raster have been acquired. Actually this conditions
is not part of
      * {@link WritableRenderedImage} contract, since a readable and writable rasters can
be used in same time.
diff --git a/core/sis-feature/src/test/java/org/apache/sis/test/suite/FeatureTestSuite.java
b/core/sis-feature/src/test/java/org/apache/sis/test/suite/FeatureTestSuite.java
index 5367c9d..7fe6dd4 100644
--- a/core/sis-feature/src/test/java/org/apache/sis/test/suite/FeatureTestSuite.java
+++ b/core/sis-feature/src/test/java/org/apache/sis/test/suite/FeatureTestSuite.java
@@ -80,6 +80,7 @@ import org.junit.runners.Suite;
     org.apache.sis.image.ComputedImageTest.class,
     org.apache.sis.image.DefaultIteratorTest.class,
     org.apache.sis.image.LinearIteratorTest.class,
+    org.apache.sis.image.StatisticsCalculatorTest.class,
     org.apache.sis.coverage.CategoryTest.class,
     org.apache.sis.coverage.CategoryListTest.class,
     org.apache.sis.coverage.SampleDimensionTest.class,


Mime
View raw message