5
\$\begingroup\$

I'm building an image filtering application, and I'm new to the Java frameworks that I need to do so. Right now, I can apply a simple 3x3 kernel blur to my thing by overwriting the RGB values of the destination picture pixel by pixel using the RGB averages from the original photo. I'm trying to increase the speed at which I can perform these filter calculations so there's less lag in my app.

I've done some surface level optimization, but if there's anything you see that could better utilize the cache or an entirely different framework that's faster than BufferedImage.getRGB(), I'd love to hear!

public void boxBlur(ImageView iv) {
       for (int height = 0; height < sourceImage.getHeight(); height++) {
           for (int width = 0; width < sourceImage.getWidth(); width++) {
               int[][] kernel = new int[][]{ // Contruct RGB array to manipulate. Edges are accounted for in construction
                   {(height == 0 || width == 0) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width - 1, height - 1), (height == 0) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width, height - 1), (height == 0 || width >= sourceImage.getWidth() - 1) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width + 1, height - 1)},
                   {(width == 0) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width - 1, height), sourceImage.getRGB(width, height), (width == sourceImage.getWidth() - 1) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width + 1, height)},
                   {(height >= sourceImage.getHeight() - 1 || width == 0) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width - 1, height + 1), (height >= sourceImage.getHeight() - 1) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width, height + 1), (height >= sourceImage.getHeight() - 1 || width >= sourceImage.getWidth() - 1) ? sourceImage.getRGB(width, height) : sourceImage.getRGB(width + 1, height + 1)}
               };
               int redAvg = 0, blueAvg = 0, greenAvg = 0;
               for (int i[] : kernel) {
                   for (int j : i) {
                       redAvg += getRed(j); // Get bitwise value from RGB int
                       greenAvg += getGreen(j);
                       blueAvg += getBlue(j);
                   }
               }
               redAvg /= 9; // 9 is size of kernel
               greenAvg /= 9;
               blueAvg /= 9;
               destinationImage.setRGB(width, height, 65536 * redAvg + 256 * greenAvg + blueAvg); // setRGB() takes the integer value of an rgb color
           }
       }
       iv.setImage(updateDisplay());
   }
\$\endgroup\$
  • 3
    \$\begingroup\$ Welcome to Code Review! I personally come from a C++/Python background where OpenCV is one of the most used image processing libraries. AFAIK, there are also bindings for Java. You might have to invest a little bit of your time to build them yourself if you cannot find prebuilt binaries including them. Fortunately, there are a few tutorial such as this one which can help you to get started. \$\endgroup\$ – AlexV Nov 7 '19 at 17:10
  • 1
    \$\begingroup\$ Have you looked at ConvolveOp. It does this all for you in one or two lines (except it will leave border pixels unchanged). Sending you back to SO : stackoverflow.com/questions/29295929/java-blur-image \$\endgroup\$ – drekbour Nov 8 '19 at 21:10
6
\$\begingroup\$

Throwaway memory

I haven't done any analysis on the code but based on my experience the worst part is the allocation of throwaway memory inside the innermost loop. Instead of allocating a 3x3 array 9 million times and throwing it to the garbage colletor immediately, allocate it once at the start of the method and reuse it in the loop.

int[][] kernel = new int[][] { ... }

Repeated calculations

Second obvious thing is the repeating of same mathematical operations. You calculate the right and bottom border several times inside the innermost loop even though the result never changes during the image processing. Calculate these at the start of the method and store the results to local variables:

sourceImage.getHeight() - 1
sourceImage.getWidth() - 1

Sacrifice readability for performance

After solving the obvious parts, you can start breaking good coding conventions. The only reason you have those mile long unreadable ternary operations is to handle the edge cases that affect 0.1% of your data. Take out the easy code inside the edges that covers 99.9% of your running time, that doesn't need any bounds checking, and make it as fast as possible. Then make a separate loop that handle only the edges.

Bad variable naming

The choice of loop variables are bad. Width and height are constants of the image that refer to the right and bottom border. You are using these names to refer to single pixels inside the image. The correct names here would be x and y. This makes following your code extremely difficult.

Refer to the good old StackOverflow-side of this site for more info about accessing the color values directly:

https://stackoverflow.com/questions/37779515/how-can-i-convert-an-imageview-to-byte-array-in-android-studio

\$\endgroup\$
  • \$\begingroup\$ I'm pretty sure all the getWidth()-1s etc will be optimised away anyway but boiling it down to w and h might well help it be more readable. \$\endgroup\$ – drekbour Nov 8 '19 at 20:08
  • 1
    \$\begingroup\$ I doubt it. The return value of getWidth() is calculated in View.java and it's components are "protected". Maybe they get optimized at runtime, but not at the compiler. \$\endgroup\$ – TorbenPutkonen Nov 8 '19 at 20:27
4
\$\begingroup\$

As you specifically ask for a different framework: you reinvented the wheel here.

The base libraries of java already contain everything necessary to do this operation in a few lines of code while utilizing highly optimized vendor code.

For a blur operation, create a kernel of 1/9 in 3x3, e.g.

     float oneNinth = 1f / 9f;
     Kernel kernel = new Kernel(3, 3, new float[] {
        oneNinth, oneNinth, oneNinth,
        oneNinth, oneNinth, oneNinth,
        oneNinth, oneNinth, oneNinth });

then create a ConvolveOp and apply it to the source and destination images:

     BufferedImageOp op = new ConvolveOp(kernel);
     op.filter(srcImage, dstImage);

I am sure you'll find lots of examples out there.

\$\endgroup\$
  • \$\begingroup\$ I knew of ConvolveOp but wanted to fact-check that comment about "optimised vendor code". It certainly does lead down to a class named ImagingLib which hooks into native calls. \$\endgroup\$ – drekbour Nov 9 '19 at 20:51
  • \$\begingroup\$ Yes exactly. And this should use the graphics card if possible on the underlying system. \$\endgroup\$ – mtj Nov 10 '19 at 8:26
2
\$\begingroup\$
  1. As has been pointed out, the code allocates a new int[3][3] for each pixel. Do this once and reuse.

  2. Use int[9] kernel instead of int[3][3]. This will remove the need for the inner j loop when summing.

  3. sourceImage.getRGB(x-1, y-1, 3, 3, kernel, 0, 3) will read 9 pixels at once though it won't handle borders without similar conditions to yours.

  4. Memory is cheap so get the whole RGB image, once only (sourceImage.getRGB(0,0,w,h,pixels,0,w)) then work directly with array-access only.

  5. Consider writing this as a ForkJoin task to parallelise the processing. Though all tasks can share the same pixels array, any image-task can be split into two sub-tasks along its longest edge (like folding a towel)

\$\endgroup\$
  • \$\begingroup\$ I did a bit of experimenting with multithreaded image processing on Android some time ago and found that it had a degrading effect on a four core processor. I guess most modern devices have more now, but it's a thing to keep in mind. \$\endgroup\$ – TorbenPutkonen Nov 9 '19 at 6:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.