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: LinearIterator.next() should avoid to compute tile indices at every calls when the tile did not changed.
Date Wed, 08 May 2019 17:16:15 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 1d503b1  LinearIterator.next() should avoid to compute tile indices at every calls
when the tile did not changed.
1d503b1 is described below

commit 1d503b16bde814cf899260e9c728281e9b211fd7
Author: Martin Desruisseaux <martin.desruisseaux@geomatys.com>
AuthorDate: Wed May 8 19:09:28 2019 +0200

    LinearIterator.next() should avoid to compute tile indices at every calls when the tile
did not changed.
---
 .../java/org/apache/sis/image/DefaultIterator.java | 95 ++++++++++++++--------
 .../java/org/apache/sis/image/LinearIterator.java  | 89 +++++++++++++-------
 .../java/org/apache/sis/image/PixelIterator.java   | 10 +++
 3 files changed, 133 insertions(+), 61 deletions(-)

diff --git a/core/sis-raster/src/main/java/org/apache/sis/image/DefaultIterator.java b/core/sis-raster/src/main/java/org/apache/sis/image/DefaultIterator.java
index b737d35..5bf0dab 100644
--- a/core/sis-raster/src/main/java/org/apache/sis/image/DefaultIterator.java
+++ b/core/sis-raster/src/main/java/org/apache/sis/image/DefaultIterator.java
@@ -31,6 +31,7 @@ import java.nio.FloatBuffer;
 import java.nio.DoubleBuffer;
 import org.opengis.coverage.grid.SequenceType;
 import org.apache.sis.internal.raster.Resources;
+import org.apache.sis.util.resources.Errors;
 import org.apache.sis.util.ArgumentChecks;
 
 
@@ -57,32 +58,33 @@ import org.apache.sis.util.ArgumentChecks;
 class DefaultIterator extends WritablePixelIterator {
     /**
      * Tile coordinate of {@link #currentRaster}.
+     * The {@code tileY >= tileUpperY} condition is used for detecting when we reached
iteration end.
      */
-    protected int tileX, tileY;
+    int tileX, tileY;
 
     /**
      * Current column index in current raster.
+     * The {@code x >= lowerX} condition is used for detecting if iteration started.
      */
-    protected int x;
+    int x;
 
     /**
      * Current row index in current raster.
      */
-    protected int y;
+    int y;
 
     /**
      * Bounds of the region traversed by the iterator in current raster.
      * When iteration reaches the upper coordinates, the iterator needs to move to next tile.
      */
-    protected int currentLowerX, currentUpperX, currentUpperY;
+    int currentLowerX, currentUpperX, currentUpperY;
 
     /**
      * Creates an iterator for the given region in the given raster.
      *
      * @param  input    the raster which contains the sample values to read.
      * @param  output   the raster where to write the sample values, or {@code null} for
read-only iterator.
-     * @param  subArea  the raster region where to perform the iteration, or {@code null}
-     *                  for iterating over all the raster domain.
+     * @param  subArea  the raster region where to perform the iteration, or {@code null}
for iterating over all the raster domain.
      * @param  window   size of the window to use in {@link #createWindow(TransferType)}
method, or {@code null} if none.
      */
     DefaultIterator(final Raster input, final WritableRaster output, final Rectangle subArea,
final Dimension window) {
@@ -99,8 +101,7 @@ class DefaultIterator extends WritablePixelIterator {
      *
      * @param  input    the image which contains the sample values to read.
      * @param  output   the image where to write the sample values, or {@code null} for read-only
iterator.
-     * @param  subArea  the image region where to perform the iteration, or {@code null}
-     *                  for iterating over all the image domain.
+     * @param  subArea  the image region where to perform the iteration, or {@code null}
for iterating over all the image domain.
      * @param  window   size of the window to use in {@link #createWindow(TransferType)}
method, or {@code null} if none.
      */
     DefaultIterator(final RenderedImage input, final WritableRenderedImage output, final
Rectangle subArea, final Dimension window) {
@@ -112,6 +113,14 @@ class DefaultIterator extends WritablePixelIterator {
         currentUpperY = lowerY;
         x = Math.decrementExact(lowerX);        // Set the position before first pixel.
         y = lowerY;
+        /*
+         * We need to ensure that `tileUpperY+1 > tileUpperY` will alway be true because
`tileY` may be equal
+         * to `tileUpperY` when the `if (++tileY >= tileUpperY)` statement is excuted
in the `next()` method.
+         * This is because `tileY` is used as a sentinel value for detecting when we reached
iteration end.
+         */
+        if (tileUpperY == Integer.MAX_VALUE) {
+            throw new ArithmeticException(Errors.format(Errors.Keys.IntegerOverflow_1, Integer.SIZE));
+        }
     }
 
     /**
@@ -150,6 +159,7 @@ class DefaultIterator extends WritablePixelIterator {
 
     /**
      * Returns the column (x) and row (y) indices of the current pixel.
+     * This implementation {@link #x} and {@link #tileY} for determining if the iteration
is valid.
      *
      * @return column and row indices of current iterator position.
      * @throws IllegalStateException if this method is invoked before the first call to {@link
#next()}
@@ -160,7 +170,7 @@ class DefaultIterator extends WritablePixelIterator {
         final short message;
         if (x < lowerX) {
             message = Resources.Keys.IterationNotStarted;
-        } else if (x >= upperX) {
+        } else if (tileY >= tileUpperY) {
             message = Resources.Keys.IterationIsFinished;
         } else {
             return new Point(x,y);
@@ -184,10 +194,12 @@ class DefaultIterator extends WritablePixelIterator {
             final int tx = Math.floorDiv(px - tileGridXOffset, tileWidth);
             final int ty = Math.floorDiv(py - tileGridYOffset, tileHeight);
             if (tx != tileX || ty != tileY) {
-                close();                                    // Release current writable raster,
if any.
+                close();                                            // Release current writable
raster, if any.
                 tileX = tx;
                 tileY = ty;
-                fetchTile();
+                if (fetchTile() > py || currentLowerX > px) {       // `fetchTile()`
must be before `currentLowerX`.
+                    throw new RasterFormatException(Resources.format(Resources.Keys.IncompatibleTile_2,
tileX, tileY));
+                }
             }
         }
         x = px;
@@ -195,7 +207,8 @@ class DefaultIterator extends WritablePixelIterator {
     }
 
     /**
-     * Moves the iterator to the next pixel.
+     * Moves the iterator to the next pixel. This default implementation moves to the next
tile only after
+     * all pixels in current tiles have been traversed, but subclasses may apply a different
strategy.
      *
      * @return {@code true} if the current pixel is valid, or {@code false} if there is no
more pixels.
      * @throws IllegalStateException if this iterator already reached end of iteration in
a previous call
@@ -207,26 +220,13 @@ class DefaultIterator extends WritablePixelIterator {
             if (++y >= currentUpperY) {                     // Strict equality (==) would
work, but use >= as a safety.
                 close();                                    // Release current writable raster,
if any.
                 if (++tileX >= tileUpperX) {                // Strict equality (==) would
work, but use >= as a safety.
-                    tileY = Math.incrementExact(tileY);     // 'incrementExact' because 'tileY
> tileUpperY' is allowed.
-                    if (tileY >= tileUpperY) {
-                        /*
-                         * Paranoiac safety: keep the x, y and tileX values before their
maximal values
-                         * in order to avoid overflow. The 'tileY' value is used for checking
if next()
-                         * is invoked again, in order to avoid a common misuse pattern. In
principle
-                         * 'tileY' needs to be compared only to 'tileUpperY', but we also
compare to
-                         * 'tileLowerY + 1' for handling the empty iterator case.
-                         */
-                        x =  currentUpperX - 1;
-                        y =  currentUpperY - 1;
-                        tileX = tileUpperX - 1;
-                        if (tileY > Math.max(tileUpperY, tileLowerY + 1)) {
-                            throw new IllegalStateException(Resources.format(Resources.Keys.IterationIsFinished));
-                        }
+                    if (++tileY >= tileUpperY) {
+                        endOfIteration();
                         return false;
                     }
                     tileX = tileLowerX;
                 }
-                fetchTile();
+                y = fetchTile();
             }
             x = currentLowerX;
         }
@@ -235,10 +235,15 @@ class DefaultIterator extends WritablePixelIterator {
 
     /**
      * Fetches from the image a tile for the current {@link #tileX} and {@link #tileY} coordinates.
-     * All fields prefixed by {@code current} are updated by this method. This method also
updates
-     * the {@link #y} field, but caller is responsible for updating the {@link #x} field.
+     * All fields prefixed by {@code current} are updated by this method. The caller is responsible
+     * for updating the {@link #x} and {@link #y} fields.
+     *
+     * <p>Note that there is no {@code currentLowerY} field in this {@code DefaultIterator}
class.
+     * Instead, the value that would be have been set to that field is returned by this method.</p>
+     *
+     * @return the {@link #y} value of the first row of new tile.
      */
-    void fetchTile() {
+    final int fetchTile() {
         currentRaster = null;
         if (destination != null) {
             destRaster = destination.getWritableTile(tileX, tileY);
@@ -252,12 +257,36 @@ class DefaultIterator extends WritablePixelIterator {
         final int minX = currentRaster.getMinX();
         final int minY = currentRaster.getMinY();
         currentLowerX  = Math.max(lowerX, minX);
-        y              = Math.max(lowerY, minY);
         currentUpperX  = Math.min(upperX, minX + tileWidth);
         currentUpperY  = Math.min(upperY, minY + tileHeight);
         if (currentRaster.getNumBands() != numBands) {
             throw new RasterFormatException(Resources.format(Resources.Keys.IncompatibleTile_2,
tileX, tileY));
         }
+        return Math.max(lowerY, minY);
+    }
+
+    /**
+     * Invoked when a call to {@link #next()} moved to the end of iteration. This method
sets fields to values
+     * that will allow {@link #moveTo(int,int)} and {@link #next()} to detect that we already
finished iteration.
+     */
+    final void endOfIteration() {
+        /*
+         * The `tileY` value is used for checking if next() is invoked again, in order to
avoid a
+         * common misuse pattern. In principle `tileY` needs to be compared only to `tileUpperY`,
+         * but we also compare to `tileLowerY + 1` for handling the empty iterator case.
+         */
+        final boolean error = tileY > Math.max(tileUpperY, tileLowerY + 1);
+        /*
+         * Paranoiac safety: keep the x, y and tileX variables before their limits
+         * in order to avoid overflow in the `if (++foo >= limit)` statements.
+         */
+        x =  currentUpperX - 1;
+        y =  currentUpperY - 1;
+        tileX = tileUpperX - 1;
+        tileY = tileUpperY;             // Sentinel value for detecting following error condition.
+        if (error) {
+            throw new IllegalStateException(Resources.format(Resources.Keys.IterationIsFinished));
+        }
     }
 
     /**
@@ -616,7 +645,7 @@ class DefaultIterator extends WritablePixelIterator {
      * This method does nothing if the iterator is read-only.
      */
     @Override
-    public void close() {
+    public final void close() {
         if (destination != null && destRaster != null) {
             destRaster = null;
             destination.releaseWritableTile(tileX, tileY);
diff --git a/core/sis-raster/src/main/java/org/apache/sis/image/LinearIterator.java b/core/sis-raster/src/main/java/org/apache/sis/image/LinearIterator.java
index c586550..6f2c699 100644
--- a/core/sis-raster/src/main/java/org/apache/sis/image/LinearIterator.java
+++ b/core/sis-raster/src/main/java/org/apache/sis/image/LinearIterator.java
@@ -22,13 +22,14 @@ import java.awt.image.Raster;
 import java.awt.image.RenderedImage;
 import java.awt.image.WritableRaster;
 import java.awt.image.WritableRenderedImage;
+import java.awt.image.RasterFormatException;
 import org.opengis.coverage.grid.SequenceType;
 import org.apache.sis.internal.raster.Resources;
 
 
 /**
- * Linear iterator used when common line y line iteration is requiered.
- * This iterator uses the {@link Raster} API for traversing the pixels of the image.
+ * Iterator for the {@link SequenceType#LINEAR} traversal order.
+ * This iterator behaves as is the while image was a single tile.
  * Calls to {@link #next()} move the current position by increasing the following values,
in order:
  *
  * <ol>
@@ -36,55 +37,87 @@ import org.apache.sis.internal.raster.Resources;
  *   <li>Row index in image (from top to bottom).</li>
  * </ol>
  *
+ * This class uses the {@link Raster} API for traversing the pixels of the image,
+ * i.e. it does not yet provide optimization for commonly used sample models.
+ *
  * @author  Johann Sorel (Geomatys)
+ * @author  Martin Desruisseaux (Geomatys)
  * @version 1.0
  * @since   1.0
  * @module
  */
 final class LinearIterator extends DefaultIterator {
-
-    LinearIterator(Raster input, WritableRaster output, Rectangle subArea, Dimension window)
{
+    /**
+     * Creates an iterator for the given region in the given raster.
+     *
+     * @param  input    the raster which contains the sample values to read.
+     * @param  output   the raster where to write the sample values, or {@code null} for
read-only iterator.
+     * @param  subArea  the raster region where to perform the iteration, or {@code null}
for iterating over all the raster domain.
+     * @param  window   size of the window to use in {@link #createWindow(TransferType)}
method, or {@code null} if none.
+     */
+    LinearIterator(final Raster input, final WritableRaster output, final Rectangle subArea,
final Dimension window) {
         super(input, output, subArea, window);
     }
 
+    /**
+     * Creates an iterator for the given region in the given image.
+     *
+     * @param  input    the image which contains the sample values to read.
+     * @param  output   the image where to write the sample values, or {@code null} for read-only
iterator.
+     * @param  subArea  the image region where to perform the iteration, or {@code null}
for iterating over all the image domain.
+     * @param  window   size of the window to use in {@link #createWindow(TransferType)}
method, or {@code null} if none.
+     */
     LinearIterator(final RenderedImage input, final WritableRenderedImage output, final Rectangle
subArea, final Dimension window) {
         super(input, output, subArea, window);
     }
 
+    /**
+     * Returns the order in which pixels are traversed.
+     */
     @Override
     public SequenceType getIterationOrder() {
         return SequenceType.LINEAR;
     }
 
+    /**
+     * Moves the iterator to the next pixel on the current row, or to the next row.
+     * This method behaves as if the whole image was a single tile.
+     *
+     * @return {@code true} if the current pixel is valid, or {@code false} if there is no
more pixels.
+     * @throws IllegalStateException if this iterator already reached end of iteration in
a previous call
+     *         to {@code next()}, and {@link #rewind()} or {@link #moveTo(int,int)} have
not been invoked.
+     */
     @Override
     public boolean next() {
-
-        if (++x >= upperX) {
-            //move to next line
-            x = lowerX;
-            if (++y >= upperY) {
-                x = lowerX;
-                return false;
+        if (++x >= currentUpperX) {                 // Move to next column, potentially
on a different tile.
+            if (x < upperX) {
+                close();                            // Must be invoked before `tileX` change.
+                tileX++;
+            } else {
+                x = lowerX;                         // Beginning of next row.
+                if (++y >= currentUpperY) {         // Move to next line.
+                    close();                        // Must be invoked before `tileY` change.
+                    if (++tileY >= tileUpperY) {
+                        endOfIteration();
+                        return false;
+                    }
+                } else if (tileX == tileLowerX) {
+                    return true;                    // Beginning of next row is in the same
tile.
+                }
+                close();                            // Must be invoked before `tileX` change.
+                tileX = tileLowerX;
             }
-        } else if (y >= upperY) {
-            x = lowerX;
-            //second time or more get in the next method, raise error
-            throw new IllegalStateException(Resources.format(Resources.Keys.IterationIsFinished));
-        }
-
-        if (image != null) {
-            final int tx = Math.floorDiv(x - tileGridXOffset, tileWidth);
-            final int ty = Math.floorDiv(y - tileGridYOffset, tileHeight);
-            if (tx != tileX || ty != tileY) {
-                close(); // Release current writable raster, if any.
-                tileX = tx;
-                tileY = ty;
-                int ry = y;
-                fetchTile();
-                y = ry; //y is changed by fetchTile method
+            /*
+             * At this point the (x,y) pixel coordinates have been updated and are inside
the domain of validity.
+             * We may need to change tile, either because we moved to the tile on the right
or because we start a
+             * new row (in which case we need to move to the leftmost tile). The only case
where we can skip tile
+             * change is when the image has only one tile width (in which case tileX == tileLowerX)
and the next
+             * row is still on the same tile.
+             */
+            if (fetchTile() > y) {
+                throw new RasterFormatException(Resources.format(Resources.Keys.IncompatibleTile_2,
tileX, tileY));
             }
         }
         return true;
     }
-
 }
diff --git a/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java b/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java
index 5499140..e85c9ba 100644
--- a/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java
+++ b/core/sis-raster/src/main/java/org/apache/sis/image/PixelIterator.java
@@ -290,6 +290,11 @@ public abstract class PixelIterator {
          */
         public PixelIterator create(final Raster data) {
             ArgumentChecks.ensureNonNull("data", data);
+            if (order == SequenceType.LINEAR) {
+                return new LinearIterator(data, null, subArea, window);
+            } else if (order != null) {
+                throw new IllegalStateException(Errors.format(Errors.Keys.UnsupportedType_1,
order));
+            }
             // TODO: check here for cases that we can optimize (after we ported corresponding
implementations).
             return new DefaultIterator(data, null, subArea, window);
         }
@@ -343,6 +348,11 @@ public abstract class PixelIterator {
         public WritablePixelIterator createWritable(final Raster input, final WritableRaster
output) {
             ArgumentChecks.ensureNonNull("input",  input);
             ArgumentChecks.ensureNonNull("output", output);
+            if (order == SequenceType.LINEAR) {
+                return new LinearIterator(input, output, subArea, window);
+            } else if (order != null) {
+                throw new IllegalStateException(Errors.format(Errors.Keys.UnsupportedType_1,
order));
+            }
             // TODO: check here for cases that we can optimize (after we ported corresponding
implementations).
             return new DefaultIterator(input, output, subArea, window);
         }


Mime
View raw message