ImageBasedHeightMap has a bug!

Hi monkeys,



while playing with the class I discovered that the extracted color values from the BufferedImage are labeled wrong.



I have a RGBA-TGA where water is colored with (0,0,255). But inspecting the extracted values I see that the variable “red” holds the 255f. So there’s something not right with RGB<->BGR color conversion.



Also I would ask whether the class could expose its ImageConverter to inhering classes (i.e. be a protected member) because the class is useful but some would need a different loading approach.



Cheers!

Hoi,



another push at this.



Interesting is also that here:

[java]

BufferedImage colorBufferedImage = ImageConverter.toBufferedImage(

colorImage, BufferedImage.TYPE_3BYTE_BGR);

boolean hasAlpha = colorBufferedImage.getColorModel().hasAlpha();









int bytesPerPixel = 3;

int blueBase = 0;

if (hasAlpha) {

bytesPerPixel = 4;

blueBase = 1;

}

[/java]



Which doesn’t make sense because we created our BufferedImage to be BGR, thus having no alpha field.

Ok haha, my last shot at it :smiley:



I fixed it to the point where its completely correct in my sense. I checked it with the desired output and it works flawlessly. All senseless statements and branches have been removes and I post my version for someone who encounters the same peculiar symptom:

[java]

public boolean load(boolean flipX, boolean flipY) {



BufferedImage colorBufferedImage = ImageConverter.toBufferedImage(

colorImage, BufferedImage.TYPE_3BYTE_BGR);



int imageWidth = colorBufferedImage.getWidth();

int imageHeight = colorBufferedImage.getHeight();



if (imageWidth != imageHeight) {

throw new RuntimeException("imageWidth: " + imageWidth

  • " != imageHeight: " + imageHeight);

    }



    size = imageWidth;



    byte data[] = (byte[]) colorBufferedImage.getRaster().getDataElements(

    0, 0, imageWidth, imageHeight, null);





    heightData = new float[(imageWidth * imageHeight)];



    int index = 0;

    if (flipY) {

    for (int h = 0; h < imageHeight; ++h) {

    if (flipX) {

    for (int w = imageWidth - 1; w >= 0; --w) {

    int baseIndex = (h * imageWidth * 3) + (w * 3);

    float blue = data[baseIndex];

    float green = data[baseIndex+1];

    float red = data[baseIndex+2];

    heightData[index++] = calculateHeight(red,green,blue);

    }

    } else {

    for (int w = 0; w < imageWidth; ++w) {

    int baseIndex = (h * imageWidth * 3) + (w * 3);

    float blue = data[baseIndex];

    float green = data[baseIndex+1];

    float red = data[baseIndex+2];

    heightData[index++] = calculateHeight(red,green,blue);



    }

    }

    }

    } else {

    for (int h = imageHeight - 1; h >= 0; --h) {

    if (flipX) {

    for (int w = imageWidth - 1; w >= 0; --w) {

    int baseIndex = (h * imageWidth * 3) + (w * 3);

    float blue = data[baseIndex];

    float green = data[baseIndex+1];

    float red = data[baseIndex+2];

    heightData[index++] = calculateHeight(red,green,blue);

    }

    } else {

    for (int w = 0; w < imageWidth; ++w) {

    int baseIndex = (h * imageWidth * 3) + (w * 3);

    float blue = data[baseIndex];

    float green = data[baseIndex+1];

    float red = data[baseIndex+2];

    heightData[index++] = calculateHeight(red,green,blue);

    }

    }

    }

    }









    return true;

    }



    // Just a function to do a custom heightmap calculation, change for custom needs

    public float calculateHeight(float r, float g, float b) {



    float grayscale = ((r + g + b) * dampen / 3f) - b * 0.05f;



    if (b > 250.0 || grayscale < -0.5f) {

    return -0.5f;

    } else {

    return grayscale;

    }



    }[/java]



    Cheers!

Thanks for the submission @Dodikles. I just committed a change to ImageBasedHeightmap that exposes the private members as protected now. I tried your changes and it causes odd results for TerrainTest. High heightmap values seem to sink. If you can fix that I will commit the change.

You are right with the alpha check, hasAlpha definitely will always be false.

Hi Sploreg,



thanks for the change.



I looked into this and saw that sometimes there were entries bigger than 127 in the data[] buffer. Because Java knows no unsigned types, entries will overflow and jump to -128…



Just add in the calculateHeight a case check and it will look like the following:

[java]

public float calculateHeight(float r, float g, float b) {





if (r < 0f) {

r = (256f + r);

}

if (g < 0f) {

g = (256f + g);

}

if (b < 0f) {

b = (256f + b);

}



return (float) ((0.299 * r + 0.587 * g + 0.114 * b) * dampen);





}

[/java]



Tested it and looks as expected. :slight_smile:

When you do the conversion, e.g.:

[java]float blue = data[baseIndex];

float green = data[baseIndex+1];

float red = data[baseIndex+2];[/java]

You are doing a signed conversion so it will be between -128 and 127,

mask it with 0xFF to convert to unsigned:

[java]float blue = (data[baseIndex] & 0xff);[/java]

Yes, to be precise you convert the signed byte to a signed integer and then return the lower 8 bit by masking, thus clipping the sign bit (just to remind everyone that Java knows no unsigned types :slight_smile: ).



BTW, is the addition of non-square terrain coming soon (i.e. maybe before Alpha-4)? I mean that you can have sth. like 2PatchSize X 4PatchSize…



Also I wanted to say a big THANK YOU to the team for your dedicated and awesome work! :wink:

No plans yet to make the terrain non-square. In the future I would like an abstraction of it that will allow for streaming in the terrain patches and loading them as needed (picture google maps and how it loads tiles). Then that could be non-square.

There’s still a lot to fix before then though :slight_smile: