6
\$\begingroup\$

This is web exercise 3.1.14. from the book Computer Science An Interdisciplinary Approach by Sedgewick & Wayne:

Set pixel (i, j) to the color of the most frequent value among pixels with Manhattan distance W of (i, j) in the original image.

Here is my program:

import java.awt.Color;

public class OilPaintingFilter
{
    public static int findNumberOfPixels(int distance)
    {
        int numberOfPixels = 4;
        if (distance == 1) return numberOfPixels;
        int increment = 8;
        for (int i = 2; i <= distance; i++)
        {
            numberOfPixels += increment;
            increment += 4;
        }
        return numberOfPixels;
    }
    public static Color[] findNeighboringColors(Picture picture, int col, int row, int distance)
    {
        int width = picture.width();
        int height = picture.height();
        Color[] pixels = new Color[findNumberOfPixels(distance)+1];
        int counter = 0;
        for (int j = col-distance; j <= col+distance; j++)
        {
            for (int i = row-distance; i <= row+distance; i++)
            {
                if ((Math.abs(col-j) + Math.abs(row-i)) <= distance)
                {
                    pixels[counter] = picture.get(Math.abs(j)%width,Math.abs(i)%height);
                    counter++;
                }
            }
        }
        return pixels;
    }  
    public static Color findMostFrequentNeighbor(Color[] pixels)
    {
        int[] colors = new int[pixels.length];
        for (int i = 0; i < pixels.length; i++)
        {
            for (int j = 0; j < pixels.length; j++)
            {
                if (pixels[i].equals(pixels[j]))
                {
                    colors[i]++;
                }
            }
        }
        int max = 0;
        int index = 0;
        for (int i = 0; i < pixels.length; i++)
        {
            if (colors[i] > max)
            {
                max = colors[i];
                index = i;
            }
        }
        return pixels[index];
    }
    public static Picture applyPaintingFilter(Picture picture, int distance)
    {
        int width = picture.width();
        int height = picture.height();
        Picture newPicture = new Picture(width,height);
        for (int col = 0; col < width; col++)
        {
            for (int row = 0; row < height; row++)
            {
                Color newColor = findMostFrequentNeighbor(findNeighboringColors(picture, col, row, distance));
                newPicture.set(col,row,newColor);
            }
        }
        return newPicture;
    }
    public static void main(String[] args)
    {
        Picture picture = new Picture(args[0]);
        int distance = Integer.parseInt(args[1]);
        Picture newPicture = applyPaintingFilter(picture, distance);
        newPicture.show();
    }
}

Picture is a simple API written by the authors of the book. I checked my program and it works. Here is one instance of it:

Input (picture of a puffin taken from Pinterest):

enter image description here

with Manhattan distance = 10

Output:

enter image description here

Is there any way that I can improve my program?

Thanks for your attention.

\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

That's a pretty cool application and it's nice to see the output!

There are a few things I'd like to cover :

  • Let's say you were to print the output of findNumberOfPixels everytime you ran it, you'd see that it always outputs the same number. Why? Because distance never changes and your function doesn't rely on anything else. What this means is that instead of computing the "number of pixels" everytime you want to find the color for pixel intensity I(X,Y), you could call findNumberOfPixels only once and reuse the same result. With that, I also think that your function could be evaluated in "one shot", meaning that there's clearly an equation that could get you the number of pixels without iterating on anything.
  • The combination of findNeighboringColors and findMostFrequentNeighbor seem a little overkill. Although the code does what it's supposed to do, you iterate a lot of times to do something that is pretty simple. What if, instead of having a function that returns all the individual pixel intensities in the neighborhood and another function that finds the most frequent value, you had only one function that found the most repeated color in a neighborhood? You'd only need to change the code in findNeighboringColors to return say... a dictionary where key=color, value=count. Then you could pick the color that has the highest count (I'll leave you the exercise of implementing that function, but you pretty much have all the necessary tools in the findNeighboringColors function already!)
  • Finally, I was wondering why the bottom of your image is also blue. There's nothing wrong with it considering it looks pretty, but it's also not how the code is supposed to behave. I figured it was related to the usage of modulos : picture.get(Math.abs(j)%width,Math.abs(i)%height);. While this works, you could be interesed in a technique called padding that often comes back in computer vision when you have to deal with neighborhood. The idea is that before starting to compute your image, you "grow" it by a certain number of pixels (in your case, distance). You should then set these pixels to the closest value from the real image. This way, you wouldn't need to use modulo and the blue line at the bottom would be black-ish, like expected.

That's pretty much all, but I gotta say good job it looks really nice (both the output and the code)

\$\endgroup\$
1
  • \$\begingroup\$ Thank you very much. I appreciate your feedback. :) \$\endgroup\$ Commented Sep 10, 2020 at 13:57
2
\$\begingroup\$

Just my few cents...

There is no reason to treat distance == 1 differently:

    public static int findNumberOfPixels(int distance)
    {
        int numberOfPixels = 0;
        int increment = 4;
        for (int i = 1; i <= distance; i++)
        {
            numberOfPixels += increment;
            increment += 4;
        }
        return numberOfPixels;
    }

Furthermore, this method returns the same output for the same input every time. But you are calling it once for every pixel. It might be better to compute this value only once.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Also he can remove cycle inside this function, because it makes fine arithmetic progression numberOfPixels += lim * (increment + 2 * lim - 2); where lim is distance-1 \$\endgroup\$
    – Sugar
    Commented Sep 9, 2020 at 16:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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