StrongJoshua
StrongJoshua

Reputation: 995

Converting Colors

I'm trying to write a method for my game that will take an image, the hex value for the old color, and the hex value for the new color and then convert all pixels of the old color to the new color. Right now, the method paints the entire image the new color, as if the if statement is not working at all. This the method:

private void convertColors(BufferedImage img, int oldColor, int newColor)
{
    Graphics g = img.getGraphics();
    g.setColor(new Color(newColor));
    Color old = new Color(oldColor);

    for(int x = 0; x < img.getWidth(); x++)
    {
        for(int y = 0; y < img.getHeight(); y++)
        {
            Color tmp = new Color(img.getRGB(x, y));
            if(tmp.equals(old));
            {
                System.out.println("Temp=" + tmp.toString() + "Old=" + old.toString() + "New=" + g.getColor().toString());
                g.fillRect(x, y, 1, 1);
            }
        }
    }
    g.dispose();
}

*The hex for oldColor is 0xFFFFFF (white) and for newColor is 0xFF0000 (red).

Using the println method I get these kind of results:

    Temp=java.awt.Color[r=0,g=0,b=0]Old=java.awt.Color[r=255,g=255,b=255]New=java.awt.Color[r=255,g=0,b=0]
Temp=java.awt.Color[r=255,g=255,b=255]Old=java.awt.Color[r=255,g=255,b=255]New=java.awt.Color[r=255,g=0,b=0]

The scond line looks correct, the temp color and the old are the same, but that is obviously not the case with the first. I have also tried creating a new BufferedImage and copy over the pixels but that leaves the same result... Does the equals method not work as I think it does or does this entire method just not work and there's a better way to do this? Thanks for you help in advance.

Upvotes: 0

Views: 817

Answers (3)

StrongJoshua
StrongJoshua

Reputation: 995

I got it to work; this is the working convertColors method:

    private BufferedImage convertColors(BufferedImage img, int oldColor, int newColor)
{
    int [] pixels = new int [img.getWidth() * img.getHeight()];
    img.getRGB(0, 0, img.getWidth(), img.getHeight(), pixels, 0, img.getWidth());
    Color old = new Color(oldColor);
    Color newC = new Color(newColor);

    for(int x = 0; x < img.getWidth(); x++)
    {
        for(int y = 0; y < img.getHeight(); y++)
        {
            Color tmp = new Color(pixels[x + y * img.getWidth()]);
            if(tmp.equals(old))
            {
                pixels[x + y * img.getWidth()] = newC.getRGB();
            }
        }
    }
    img.setRGB(0, 0, img.getWidth(), img.getHeight(), pixels, 0, img.getWidth());

    return newImg;
}

Upvotes: -1

luk2302
luk2302

Reputation: 57114

Just remove the ; after if(tmp.equals(old)).
Otherwise you compare the colors and do nothing after the comparsion and always choose the new color.

Besides that you need to reorganize your code a little bit to make it slightly more efficient:

Graphics g = img.getGraphics();
g.setColor(new Color(newColor));

for(int x = 0; x < img.getWidth(); x++) {
    for(int y = 0; y < img.getHeight(); y++) {
        if(img.getRGB(x, y) == oldColor) {//check if pixel color matches old color
            g.fillRect(x, y, 1, 1);//fill the pixel with the right color
        }
    }
}
g.dispose();



Just because i was interested in the topic: relying on image filters you could do all that via:

class ColorSwapFilter extends RGBImageFilter {
  int newColor, oldColor;
  public ColorSwapFilter(int newColor, int oldColor) {
    canFilterIndexColorModel = true;
    this.newColor = newColor;
    this.oldColor = oldColor;
  }

  @Override
  public int filterRGB(int x, int y, int rgb) {
    return rgb == oldColor ? newColor : oldColor;
  }
}

which should be called via

BufferedImage img;//your image
ColorSwapFilter filter = new ColorSwapFilter(...,...);//your colors to be swapped.
ImageProducer producer = img.getSource();
producer = new FilteredImageSource(producer, filter);
Image im = Toolkit.getDefaultToolkit().createImage(producer);

Upvotes: 4

Kon
Kon

Reputation: 10810

You have a semicolon direct after your if statement if(tmp.equals(old));

This esentially tells Java that your if statement has only one command associated with it, and that command is a single semicolon, effectively meaning "do nothing". If you remove it, it will restore the condition on the block of code beneath it, which right now is just running every time regardless of the condition.

Upvotes: 3

Related Questions