6
\$\begingroup\$

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

Write a program that takes the name of a grayscale image file as a command-line argument and uses StdDraw to plot a histogram of the frequency of occurrence of each of the 256 grayscale intensities.

Here is my program:

import java.awt.Color;

public class ImageToHistogram
{
    public static double getIntensity(Color color) // this method produces monochrome luminance
    {
        int r = color.getRed();
        int g = color.getGreen();
        int b = color.getBlue();
        return 0.299*r + 0.587*g + 0.114*b;
    }
    public static double[][] convertPictureToArray(Picture picture)
    {
        int width = picture.width();
        int height = picture.height();
        double[][] pixels = new double[width][height];      
        for (int col = 0; col < width; col++)
        {
            for (int row = 0; row < height; row++)
            {
                Color color = picture.get(col,row);
                pixels[col][row] = getIntensity(color);
            }
        }
        return pixels;
    }
    public static void drawHistogram(double[][] a)
    {
        int m = a.length;
        int n = a[0].length;
        int pixels = m*n;
        int[] histogram = new int[256];
        StdDraw.setPenColor(StdDraw.BOOK_BLUE);
        for (int i = 0; i < m; i++)
        {
            for (int j = 0; j < n; j++)
            {
                int intensity = (int) a[i][j];
                histogram[intensity]++;
            }
        }
        StdDraw.setXscale(0,256);
        StdDraw.setYscale(0,pixels/20); // obtained "20" by experimentation
        StdDraw.enableDoubleBuffering();
        for (int i = 0; i < 256; i++)
        {
            StdDraw.filledRectangle(i+0.5,histogram[i]/2,0.5,histogram[i]/2);
        }
        StdDraw.show();
    }
    public static void main(String[] args)
    {
        Picture picture = new Picture(args[0]);
        double[][] a = convertPictureToArray(picture);
        drawHistogram(a);
    }
}

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

Input (Grayscale picture of Coleen Gray taken from Wikipedia):

enter image description here

Output:

enter image description here

I also tested the extreme cases of pure black and pure white images.

Is there any way that I can improve my program?

Thanks for your attention.

Clarification:

I used the preview of the image provided by Wikipedia with resolution of 800 by 933 and not the original image with resolution of 2000 by 2333.

\$\endgroup\$
15
  • 2
    \$\begingroup\$ The histogram doesn't look correct. Did you crop it and only show the left (=dark) part? There's no bright highlight in the picture, so the histogram should be completely flat on the right part. Also, the left part of the histogram should be smoother, without those discrete peaks with large spaces in between. You can use dcode.fr/image-histogram for comparison. Here's the histogram in Photoshop : i.imgur.com/VI3PrcO.png \$\endgroup\$ Sep 3 '20 at 12:34
  • 1
    \$\begingroup\$ Thanks for the answer. This histogram looks much better indeed. I also used the 800x933 in Photoshop, though. Resizing an image shouldn't change the overall shape of the histogram, so there must be another explanation. Are you sure the above histogram is for the shown picture? \$\endgroup\$ Sep 3 '20 at 12:57
  • 1
    \$\begingroup\$ I don't think so. With integers, you should still have 256 distinct values, so the large valleys cannot be explained by double -> int . And the overall shape should still be similar to imgur.com/986pdcK . There's a problem, I just don't know where. \$\endgroup\$ Sep 3 '20 at 13:10
  • 1
    \$\begingroup\$ Yes, some small variations would appear. But not huge peaks with 8 empty values in between, and the global shape wouldn't change much. I don't know how else to tell you : the above histogram is not correct for the above portrait. Here are the histogram values I get with Python and the 800x933 portrait : pastebin.com/raw/FErFeY2G. For reference : np.histogram(plt.imread('ColeenGray.jpg').ravel(), bins=256, range=(0, 256))[0] Every shade of grey between 0 and 238 is present at least once, so there shouldn't be any valley in the histogram, only empty values on the right part. \$\endgroup\$ Sep 3 '20 at 13:52
  • 2
6
\$\begingroup\$

JPG vs PNG

Somehow, your code seems to produce wrong results with the above JPG. When used with the same image converted to PNG, the histogram looks much better, and much closer to what Photoshop displays.

ImageIO.read(file) seems to not return the correct RGB values with the above JPG. It is outside of your code though, and seems to be called by Picture class.

As mentioned by @Nayuki, it seems to be a known bug/feature:

Double -> int

getIntensity returns a double.

For example, getIntensity(new Color(1, 1, 1)) returns 0.9999999999999999, which is very close to 1.0 but strictly smaller than 1.0.

When you cast this double to an integer in int intensity = (int) a[i][j], it gets converted to 0, which isn't desired.

With your current code, the first histogram bins look like this:

[37509, 2429, 0, 3981, 0, 1763, 1757, 4131, 0, 2016, 4718, 0, 6498, 0, 3526, 7824, ...

If you replace the line with int intensity = (int) Math.round(a[i][j]);, the first bins look like this:

[32317, 5192, 2429, 2327, 1654, 1763, 1757, 2034, 2097, 2016, 2230, 2488, 2947, 3551, 3526, 3683, 4141, ...

Which should produce a smoother and more correct diagram.

As mentioned by @MarkRansom:

the goal should be to redistribute the values in a way that preserves their overall distribution.

Rounding the doubles should work fine in the above case, because the values are very close to integers. In the general case, truncating might be preferred.

\$\endgroup\$
10
  • 2
    \$\begingroup\$ I can't thank you enough. I appreciate all the time that you put into solving this problem. :) \$\endgroup\$ Sep 3 '20 at 15:38
  • 2
    \$\begingroup\$ @KhashayarBaghizadeh: My pleasure. It's always funny to play detective. Thanks for staying with me and trying to understand what the problem was. \$\endgroup\$ Sep 3 '20 at 17:38
  • \$\begingroup\$ round is rarely how you want to convert from floating to integer, because it results in an uneven distribution unless all the values are extremely close to integers already. \$\endgroup\$ Sep 3 '20 at 22:11
  • \$\begingroup\$ @MarkRansom interesting. Why should the distribution be uneven? What is the alternative you propose? In the above case, with a grayscale image, the values are basically integers +- epsilon. \$\endgroup\$ Sep 3 '20 at 22:17
  • 1
    \$\begingroup\$ Suppose your values were evenly distributed between 0.0 and 0.999999 and you wanted to convert them into integers 0-100: v2 = round(v1*100). 0 will result from an input of 0.0-0.005, while 1 will result from 0.005 to 0.015 - you're twice as likely to get 1 than 0. Use v2 = trunc(v1*101) instead. \$\endgroup\$ Sep 3 '20 at 23:20
4
\$\begingroup\$

I have some suggestions for your code.

Extract the expression to variables when used multiple times.

In your code, you can extract the similar expressions into variables; this will make the code shorter and easier to read.

Before

StdDraw.filledRectangle(i + 0.5, histogram[i] / 2, 0.5, histogram[i] / 2);

After

int half = histogram[i] / 2;
StdDraw.filledRectangle(i + 0.5, half, 0.5, half);

Replace the for loop with an enhanced 'for' loop

In your code, you don’t actually need the index provided by the loop, you can the enhanced version.

ImageToHistogram#drawHistogram

Before

for (int i = 0; i < m; i++) {
   for (int j = 0; j < n; j++) {
      int intensity = (int) a[i][j];
      histogram[intensity]++;
   }
}

After

for (double[] currentRow : a) {
   for (int j = 0; j < n; j++) {
      int intensity = (int) currentRow[j];
      histogram[intensity]++;
   }
}

For the rest it looks good!

\$\endgroup\$
3
\$\begingroup\$

Your review scope is extremely broad. I'm concentrating on the the code being completely procedural in a language that is object oriented.

The operation you perform on each pixel is independent of each other, so you should implement this as a series of operations performed on a stream of pixels.

  1. Write a Spliterator that allows you to process the RGB pixels of the Picture as a Java 8 IntStream (BTW, it's more common and convenient to process image data as simpe one dimensional array/stream of pixels where image rows follow each other instead of using an array of rows).
  2. Write a IntUnaryOperator that maps an RGB pixel into a intensity value and pass this to IntStream.map(IntUnaryOperator).
  3. Write a Collector that generates a histogram from a stream of integers.
  4. Write a method that shows a histogram, when given a Map<Integer, Integer>. This needs to go through the map to find the smallest and largest key as well as the largest value so you can scale the X and Y axis. You may be tempted to optimize this step to the ones above, but we're talking about a map with 255 entries. The code mess is not worth the performance gain.

When you put each step into an independent class that is not unnecessarily dependent on the domain of image processing, you make maintenance and unit testing much easier. Now you could take the histogram collector to any program and use it, for example, to generate a histogram on student grades for a programming test.

\$\endgroup\$
1
  • \$\begingroup\$ I truly appreciate your answer but I still don't know OOP. When I learn it, I will certainly come back to this answer to enhance my understanding. Thank you very much. \$\endgroup\$ Sep 3 '20 at 11:46
2
\$\begingroup\$

I myself found another way to improve my code.

Obtain the maximum for the y-coordinate of the histogram

I need to obtain the maximum for the y-coordinate of the histogram to avoid experimentation for each instance of the program.

Before:

    StdDraw.setXscale(0,256);
    StdDraw.setYscale(0,pixels/20); // obtained "20" by experimentation

After:

    int max = 0;
    for (int i = 0; i < 256; i++)
    {
        max = Math.max(max,histogram[i]);
    }
    StdDraw.setXscale(0,256);
    StdDraw.setYscale(0,max);
\$\endgroup\$

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.