Did a big refactoring on the MonochromeBitmapSource. I removed all the caching luminance calls and converted them to getting luminance data on demand. This saved another 33,000 function calls per rejected scan, good for another 15 ms savings. I also moved the luminance calls to BaseMonochromeBitmapSource and made them protected, to indicate that the decoders shouldn't use them.

Overall the recent optimizations took one rejected scan from 307 to 135 ms, which is definitely noticeable.

WARNING: I am not able to build the Bug or J2ME clients, but I believe they are correct.

git-svn-id: https://zxing.googlecode.com/svn/trunk@656 59b500cc-1b3d-0410-9834-0bbf25fbcc57

android/src/com/google/zxing/client/android/YUVMonochromeBitmapSource.java

@@ -63,17 +63,33 @@ final class YUVMonochromeBitmapSource extends BaseMonochromeBitmapSource {
    * @param y The y coordinate to fetch within crop
    * @return The luminance as an int, from 0-255
    */
-  public int getLuminance(int x, int y) {
+  protected int getLuminance(int x, int y) {
     return mYUVData[(y + mCrop.top) * mDataWidth + x + mCrop.left] & 0xff;
   }
 
-  // Nothing to do, since we have direct access to the mYUVData array.
-  public void cacheRowForLuminance(int y) {
-
+  protected int[] getLuminanceRow(int y, int[] row) {
+    int width = getWidth();
+    if (row == null || row.length < width) {
+      row = new int[width];
+    }
+    int offset = (y + mCrop.top) * mDataWidth + mCrop.left;
+    for (int x = 0; x < width; x++) {
+      row[x] = mYUVData[offset + x] & 0xff;
+    }
+    return row;
   }
 
-  public void cacheColumnForLuminance(int x) {
-
+  protected int[] getLuminanceColumn(int x, int[] column) {
+    int height = getHeight();
+    if (column == null || column.length < height) {
+      column = new int[height];
+    }
+    int offset = mCrop.top * mDataWidth + mCrop.left + x;
+    for (int y = 0; y < height; y++) {
+      column[y] = mYUVData[offset] & 0xff;
+      offset += mDataWidth;
+    }
+    return column;
   }
 
   /**

androidtest/src/com/google/zxing/client/androidtest/RGBMonochromeBitmapSource.java

@@ -70,16 +70,34 @@ public class RGBMonochromeBitmapSource extends BaseMonochromeBitmapSource {
     return mWidth;
   }
 
-  public int getLuminance(int x, int y) {
+  protected int getLuminance(int x, int y) {
     return mLuminances[y * mWidth + x] & 0xff;
   }
 
-  public void cacheRowForLuminance(int y) {
-
+  protected int[] getLuminanceRow(int y, int[] row) {
+    int width = mWidth;
+    if (row == null || row.length < width) {
+      row = new int[width];
+    }
+    int offset = y * width;
+    for (int x = 0; x < width; x++) {
+      row[x] = mLuminances[offset + x] & 0xff;
+    }
+    return row;
   }
 
-  public void cacheColumnForLuminance(int x) {
-
+  protected int[] getLuminanceColumn(int x, int[] column) {
+    int width = mWidth;
+    int height = mHeight;
+    if (column == null || column.length < height) {
+      column = new int[height];
+    }
+    int offset = x;
+    for (int y = 0; y < height; y++) {
+      column[y] = mLuminances[offset] & 0xff;
+      offset += width;
+    }
+    return column;
   }
 
 }

bug/src/com/google/zxing/client/bug/AWTImageMonochromeBitmapSource.java

@@ -63,19 +63,40 @@ public final class AWTImageMonochromeBitmapSource extends BaseMonochromeBitmapSo
    * See <code>com.google.zxing.client.j2me.LCDUIImageMonochromeBitmapSource</code> for more explanation
    * of the computation used in this method.
    */
-  public int getLuminance(int x, int y) {
-    int pixel = pixels[x * width + y];
+  protected int getLuminance(int x, int y) {
+    int pixel = pixels[y * width + x];
     return (((pixel & 0x00FF0000) >> 16) +
             ((pixel & 0x0000FF00) >>  7) +
              (pixel & 0x000000FF       )) >> 2;
   }
 
-  public void cacheRowForLuminance(int y) {
-    // do nothing; we are already forced to cache all pixels
+  protected int[] getLuminanceRow(int y, int[] row) {
+    if (row == null || row.length < width) {
+      row = new int[width];
+    }
+    int offset = y * width;
+    for (int x = 0; x < width; x++) {
+      int pixel = pixels[offset + x];
+      row[x] = (((pixel & 0x00FF0000) >> 16) +
+                ((pixel & 0x0000FF00) >>  7) +
+                 (pixel & 0x000000FF       )) >> 2;
+    }
+    return row;
   }
 
-  public void cacheColumnForLuminance(int x) {
-    // do nothing
+  protected int[] getLuminanceColumn(int x, int[] column) {
+    if (column == null || column.length < height) {
+      column = new int[height];
+    }
+    int offset = x;
+    for (int y = 0; y < height; y++) {
+      int pixel = pixels[offset];
+      column[y] = (((pixel & 0x00FF0000) >> 16) +
+                   ((pixel & 0x0000FF00) >>  7) +
+                    (pixel & 0x000000FF       )) >> 2;
+      offset += width;
+    }
+    return column;
   }
 
 }

core/src/com/google/zxing/MonochromeBitmapSource.java

@@ -23,6 +23,7 @@ import com.google.zxing.common.BitArray;
  * This unifies many possible representations, like AWT's <code>BufferedImage</code>.</p>
  *
  * @author srowen@google.com (Sean Owen)
+ * @author dswitkin@google.com (Daniel Switkin)
  */
 public interface MonochromeBitmapSource {
 
@@ -63,29 +64,6 @@ public interface MonochromeBitmapSource {
    */
   int getWidth();
 
-  /**
-   * Retrieves the luminance at the pixel x,y in the bitmap. This method is only used for estimating
-   * the black point and implementing getBlackRow() - it is not meant for decoding.
-   *
-   * @param x The x coordinate in the image.
-   * @param y The y coordinate in the image.
-   * @return The luminance value between 0 and 255.
-   */
-  int getLuminance(int x, int y);
-
-  /**
-   * Some implementations can be much more efficient by fetching an entire row of luminance data at
-   * a time. This method should be called once per row before calling getLuminance().
-   *
-   * @param y The row to cache.
-   */
-  void cacheRowForLuminance(int y);
-
-  /**
-   * Entirely analogous to {@link #cacheRowForLuminance(int)} but caches a column.
-   */
-  void cacheColumnForLuminance(int x);
-
   /**
    * <p>Estimates black point according to the given method, which is optionally parameterized by
    * a single int argument. For {@link BlackPointEstimationMethod#ROW_SAMPLING}, this

core/src/com/google/zxing/common/BaseMonochromeBitmapSource.java

@@ -31,6 +31,7 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
   private int blackPoint;
   private BlackPointEstimationMethod lastMethod;
   private int lastArgument;
+  private int[] luminances;
 
   protected BaseMonochromeBitmapSource() {
     blackPoint = 0x7F;
@@ -38,6 +39,15 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
     lastArgument = 0;
   }
 
+  private void initLuminances() {
+    if (luminances == null) {
+      int width = getWidth();
+      int height = getHeight();
+      int max = width > height ? width : height;
+      luminances = new int[max];
+    }
+  }
+
   public boolean isBlack(int x, int y) {
     return getLuminance(x, y) < blackPoint;
   }
@@ -49,15 +59,17 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
       row.clear();
     }
 
+    // Reuse the same int array each time
+    initLuminances();
+    luminances = getLuminanceRow(y, luminances);
+
     // If the current decoder calculated the blackPoint based on one row, assume we're trying to
     // decode a 1D barcode, and apply some sharpening.
-    // TODO: We may want to add a fifth parameter to request the amount of shapening to be done.
-    cacheRowForLuminance(y);
     if (lastMethod.equals(BlackPointEstimationMethod.ROW_SAMPLING)) {
-      int left = getLuminance(startX, y);
-      int center = getLuminance(startX + 1, y);
+      int left = luminances[startX];
+      int center = luminances[startX + 1];
       for (int x = 1; x < getWidth - 1; x++) {
-        int right = getLuminance(startX + x + 1, y);
+        int right = luminances[startX + x + 1];
         // Simple -1 4 -1 box filter with a weight of 2
         int luminance = ((center << 2) - left - right) >> 1;
         if (luminance < blackPoint) {
@@ -68,7 +80,7 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
       }
     } else {
       for (int x = 0; x < getWidth; x++) {
-        if (getLuminance(startX + x, y) < blackPoint) {
+        if (luminances[startX + x] < blackPoint) {
           row.set(x);
         }
       }
@@ -83,10 +95,13 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
       column.clear();
     }
 
-    cacheColumnForLuminance(x);
+    // Reuse the same int array each time
+    initLuminances();
+    luminances = getLuminanceColumn(x, luminances);
+
     // We don't handle "row sampling" specially here
     for (int y = 0; y < getHeight; y++) {
-      if (getLuminance(x, startY + y) < blackPoint) {
+      if (luminances[startY + y] < blackPoint) {
         column.set(y);
       }
     }
@@ -110,10 +125,10 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
         if (argument < 0 || argument >= height) {
           throw new IllegalArgumentException("Row is not within the image: " + argument);
         }
-        cacheRowForLuminance(argument);
+        initLuminances();
+        luminances = getLuminanceRow(argument, luminances);
         for (int x = 0; x < width; x++) {
-          int luminance = getLuminance(x, argument);
-          histogram[luminance >> LUMINANCE_SHIFT]++;
+          histogram[luminances[x] >> LUMINANCE_SHIFT]++;
         }
       } else {
         throw new IllegalArgumentException("Unknown method: " + method);
@@ -144,10 +159,37 @@ public abstract class BaseMonochromeBitmapSource implements MonochromeBitmapSour
 
   public abstract int getWidth();
 
-  public abstract int getLuminance(int x, int y);
+  /**
+   * Retrieves the luminance at the pixel x,y in the bitmap. This method is only used for estimating
+   * the black point and implementing getBlackRow() - it is not meant for decoding, hence it is not
+   * part of MonochromeBitmapSource itself, and is protected.
+   *
+   * @param x The x coordinate in the image.
+   * @param y The y coordinate in the image.
+   * @return The luminance value between 0 and 255.
+   */
+  protected abstract int getLuminance(int x, int y);
 
-  public abstract void cacheRowForLuminance(int y);
+  /**
+   * This is the main mechanism for retrieving luminance data. It is dramatically more efficient
+   * than repeatedly calling getLuminance(). As above, this is not meant for decoders.
+   *
+   * @param y The row to fetch
+   * @param row The array to write luminance values into. It is <b>strongly</b> suggested that you
+   *            allocate this yourself, making sure row.length >= getWidth(), and reuse the same
+   *            array on subsequent calls for performance. If you pass null, you will be flogged,
+   *            but then I will take pity on you and allocate a sufficient array internally.
+   * @return The array containing the luminance data. This is the same as row if it was usable.
+   */
+  protected abstract int[] getLuminanceRow(int y, int[] row);
 
-  public abstract void cacheColumnForLuminance(int x);
+  /**
+   * The same as getLuminanceRow(), but for columns.
+   *
+   * @param x The column to fetch
+   * @param column The array to write luminance values into. See above.
+   * @return The array containing the luminance data.
+   */
+  protected abstract int[] getLuminanceColumn(int x, int[] column);
 
 }

javame/src/com/google/zxing/client/j2me/LCDUIImageMonochromeBitmapSource.java

@@ -30,22 +30,13 @@ public final class LCDUIImageMonochromeBitmapSource extends BaseMonochromeBitmap
   private final Image image;
   private final int height;
   private final int width;
-  // For why this isn't final, see below
-  private int[] rgbRow;
-  private int[] rgbColumn;
   private final int[] pixelHolder;
-  private int cachedRow;
-  private int cachedColumn;
 
   public LCDUIImageMonochromeBitmapSource(Image image) {
     this.image = image;
     height = image.getHeight();
     width = image.getWidth();
-    rgbRow = new int[width];
-    rgbColumn = new int[height];
     pixelHolder = new int[1];
-    cachedRow = -1;
-    cachedColumn = -1;
   }
 
   public int getHeight() {
@@ -56,21 +47,10 @@ public final class LCDUIImageMonochromeBitmapSource extends BaseMonochromeBitmap
     return width;
   }
 
-  public int getLuminance(int x, int y) {
-
-    // Below, why the check for rgbRow being the right size? it should never change size
-    // or need to be reallocated. But bizarrely we have seen a but on Sun's WTK, and on
-    // some phones, where the array becomes zero-sized somehow. So we keep making sure the
-    // array is OK.
-    int pixel;
-    if (cachedRow == y && rgbRow.length == width) {
-      pixel = rgbRow[x];
-    } else if (cachedColumn == x && rgbColumn.length == height) {
-      pixel = rgbColumn[y];
-    } else {
-      image.getRGB(pixelHolder, 0, width, x, y, 1, 1);
-      pixel = pixelHolder[0];
-    }
+  // This is expensive and should be used very sparingly.
+  protected int getLuminance(int x, int y) {
+    image.getRGB(pixelHolder, 0, width, x, y, 1, 1);
+    int pixel = pixelHolder[0];
 
     // Instead of multiplying by 306, 601, 117, we multiply by 256, 512, 256, so that
     // the multiplies can be implemented as shifts.
@@ -89,25 +69,33 @@ public final class LCDUIImageMonochromeBitmapSource extends BaseMonochromeBitmap
              (pixel & 0x000000FF       )) >> 2;
   }
 
-  public void cacheRowForLuminance(int y) {
-    if (y != cachedRow) {
-      // See explanation above
-      if (rgbRow.length != width) {
-        rgbRow = new int[width];
-      }
-      image.getRGB(rgbRow, 0, width, 0, y, width, 1);
-      cachedRow = y;
+  // For efficiency, the RGB data and the luminance data share the same array.
+  protected int[] getLuminanceRow(int y, int[] row) {
+    if (row == null || row.length < width) {
+      row = new int[width];
     }
+    image.getRGB(row, 0, width, 0, y, width, 1);
+    for (int x = 0; x < width; x++) {
+      int pixel = row[x];
+      row[x] = (((pixel & 0x00FF0000) >> 16) +
+                ((pixel & 0x0000FF00) >>  7) +
+                 (pixel & 0x000000FF       )) >> 2;
+    }
+    return row;
   }
 
-  public void cacheColumnForLuminance(int x) {
-    if (x != cachedColumn) {
-      if (rgbColumn.length != height) {
-        rgbColumn = new int[height];
-      }
-      image.getRGB(rgbColumn, 0, 1, x, 0, 1, height);
-      cachedColumn = x;
+  protected int[] getLuminanceColumn(int x, int[] column) {
+    if (column == null || column.length < height) {
+      column = new int[height];
     }
+    image.getRGB(column, 0, 1, x, 0, 1, height);
+    for (int y = 0; y < height; y++) {
+      int pixel = column[y];
+      column[y] = (((pixel & 0x00FF0000) >> 16) +
+                   ((pixel & 0x0000FF00) >>  7) +
+                    (pixel & 0x000000FF       )) >> 2;
+    }
+    return column;
   }
 
-}
+}

javase/src/com/google/zxing/client/j2se/BufferedImageMonochromeBitmapSource.java

@@ -42,10 +42,6 @@ public final class BufferedImageMonochromeBitmapSource extends BaseMonochromeBit
   private final int top;
   private final int width;
   private final int height;
-  private int[] rgbRow;
-  private int[] rgbColumn;
-  private int cachedRow;
-  private int cachedColumn;
 
   /**
    * Creates an instance that uses the entire given image as a source of pixels to decode.
@@ -54,8 +50,6 @@ public final class BufferedImageMonochromeBitmapSource extends BaseMonochromeBit
    */
   public BufferedImageMonochromeBitmapSource(BufferedImage image) {
     this(image, 0, 0, image.getWidth(), image.getHeight());
-    rgbRow = new int[image.getWidth()];
-    cachedRow = -1;
   }
 
   /**
@@ -79,10 +73,6 @@ public final class BufferedImageMonochromeBitmapSource extends BaseMonochromeBit
     this.top = top;
     this.width = right - left;
     this.height = bottom - top;
-    rgbRow = new int[width];
-    rgbColumn = new int[height];
-    cachedRow = -1;
-    cachedColumn = -1;
   }
 
   /**
@@ -136,34 +126,40 @@ public final class BufferedImageMonochromeBitmapSource extends BaseMonochromeBit
    *
    * where R, G, and B are values in [0,1].
    */
-  public int getLuminance(int x, int y) {
-    int pixel;
-    if (cachedRow == y) {
-      pixel = rgbRow[x];
-    } else if (cachedColumn == x) {
-      pixel = rgbColumn[y];
-    } else {
-      pixel = image.getRGB(left + x, top + y);
-    }
-
+  protected int getLuminance(int x, int y) {
+    int pixel = image.getRGB(left + x, top + y);
     // Coefficients add up to 1024 to make the divide into a fast shift
     return (306 * ((pixel >> 16) & 0xFF) +
         601 * ((pixel >> 8) & 0xFF) +
         117 * (pixel & 0xFF)) >> 10;
   }
 
-  public void cacheRowForLuminance(int y) {
-    if (y != cachedRow) {
-      image.getRGB(left, top + y, width, 1, rgbRow, 0, width);
-      cachedRow = y;
+  protected int[] getLuminanceRow(int y, int[] row) {
+    if (row == null || row.length < width) {
+      row = new int[width];
     }
+    image.getRGB(left, top + y, width, 1, row, 0, width);
+    for (int x = 0; x < width; x++) {
+      int pixel = row[x];
+      row[x] = (306 * ((pixel >> 16) & 0xFF) +
+          601 * ((pixel >> 8) & 0xFF) +
+          117 * (pixel & 0xFF)) >> 10;
+    }
+    return row;
   }
 
-  public void cacheColumnForLuminance(int x) {
-    if (x != cachedColumn) {
-      image.getRGB(left + x, top, 1, height, rgbColumn, 0, 1);
-      cachedColumn = x;
+  protected int[] getLuminanceColumn(int x, int[] column) {
+    if (column == null || column.length < height) {
+      column = new int[height];
     }
+    image.getRGB(left + x, top, 1, height, column, 0, 1);
+    for (int y = 0; y < height; y++) {
+      int pixel = column[y];
+      column[y] = (306 * ((pixel >> 16) & 0xFF) +
+          601 * ((pixel >> 8) & 0xFF) +
+          117 * (pixel & 0xFF)) >> 10;
+    }
+    return column;
   }
 
 }