Color substitution in a BufferedImage

Question

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 < h; row++) {
        for (int col = 0; col < w; col++) {
            alphaPixels[row][col] = ((RGBarray[i] >> 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"));
}

Answer 1

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(…));
}

Answer 2

Instead of shifting each and every pixel and testing against each input R, G, and B values, why not shift your input RGB (and implied A) once and then match that against the full four byte value at each pixel location?

Or if you can't make any assumptions about A you can still match the constructed RGB triplet instead of deconstructing, testing and reconstructing. The same goes for the value you want to set it to.