HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Color substitution in a BufferedImage

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bufferedimagecolorsubstitution

Problem

The setPixelColor function below changes the color of pixels.
I need some suggestions to optimize this function.

Example:

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File("Images/Input-1.bmp"));
    setPixelColor(pic1, 34, 177, 76, 127, 127, 127);
}


This changes the left image into the right image.

private static void setPixelColor(BufferedImage imgBuf, int red, int green, int blue, int newRed, int newGreen, int newBlue) throws IOException {
    int[] RGBarray;
    int w;
    int h;

    //Declare color arrays
    int[][] alphaPixels;
    int[][] redPixels;
    int[][] greenPixels;
    int[][] bluePixels;

    w = imgBuf.getWidth();
    h = imgBuf.getHeight();

    alphaPixels = new int[h][w];
    redPixels = new int[h][w];
    greenPixels = new int[h][w];
    bluePixels = new int[h][w];

    RGBarray = imgBuf.getRGB(0, 0, w, h, null, 0, w);

    //Bit shift values into arrays
    int i = 0;
    for (int row = 0; row > 24) & 0xff);
            redPixels[row][col] = ((RGBarray[i] >> 16) & 0xff);
            greenPixels[row][col] = ((RGBarray[i] >> 8) & 0xff);
            bluePixels[row][col] = (RGBarray[i] & 0xff);
            i++;
        }
    }

    //Set the values back to integers using re-bit shifting
    for (int row = 0; row < h; row++) {
        for (int col = 0; col < w; col++) {
            if (redPixels[row][col] == red && greenPixels[row][col] == green && bluePixels[row][col] == blue) {
                int rgb = (alphaPixels[row][col] & 0xff) << 24 |
                        (redPixels[row][col] & newRed) << 16 |
                        (greenPixels[row][col] & newGreen) << 8 |
                        (bluePixels[row][col] & newBlue);
                imgBuf.setRGB(col, row, rgb);
            }
        }
    }

    //Write back image
    ImageIO.write(imgBuf, "bmp", new File("Images/Output2.bmp"));
}

Solution

I think that changeColor or substColor would be a more appropriate name for this function.

I don't see why the function should write out its result to a file. That's a violation of the Single Responsibility Principle. What if I want to perform multiple color substitutions before writing out the result? What if I want a different output filename, or a format other than BMP?

Avoid empty declarations, when you can declare and initialize values at the same time. It's more readable and less error-prone.

You don't care about the x-y coordinates of each pixel — only the color matters. So, there is no need to construct the 2-D arrays representing the red, green, and blue channels.

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}

Code Snippets

/**
 * Changes all pixels of an old color into a new color, preserving the
 * alpha channel.
 */
private static void changeColor(
        BufferedImage imgBuf,
        int oldRed, int oldGreen, int oldBlue,
        int newRed, int newGreen, int newBlue) {

    int RGB_MASK = 0x00ffffff;
    int ALPHA_MASK = 0xff000000;

    int oldRGB = oldRed << 16 | oldGreen << 8 | oldBlue;
    int toggleRGB = oldRGB ^ (newRed << 16 | newGreen << 8 | newBlue);

    int w = imgBuf.getWidth();
    int h = imgBuf.getHeight();

    int[] rgb = imgBuf.getRGB(0, 0, w, h, null, 0, w);
    for (int i = 0; i < rgb.length; i++) {
        if ((rgb[i] & RGB_MASK) == oldRGB) {
            rgb[i] ^= toggleRGB;
        }
    }
    imgBuf.setRGB(0, 0, w, h, rgb, 0, w);
}

public static void main(String[] args) throws IOException {
    BufferedImage pic1 = ImageIO.read(new File(…));
    changeColor(pic1, 34, 177, 76, 127, 127, 127);
    ImageIO.write(pic1, "bmp", new File(…));
}

Context

StackExchange Code Review Q#146609, answer score: 5

Revisions (0)

No revisions yet.