TerrainGrid and Terrain adjustHeight problems

Hello, it’s me again,



I got terrain edition to work. It’s fast and very cool to be able to edit the terrain in real time in game ^.^



There’s just a last problem. Everything works perfectly as long as the camera uses the default grids… at (0,0,0).

If I move the camera enough to trigger a change of grids displayed, terrain editing suddenly stops working. I see the ray hit the terrain at the correct gridCell and localPoint, My heightMaps in memory are updated correctly, but TerrainGrid.adjustHeight() does nothing (at all, nowhere!).



Should I offset the list of locations based on the Event GridMoved or something ?



Also, for my tests I bind a key to a full Terrain refresh : I delete the current one, and recreate a new one. See the difference :



Original Terrain with adjustHeight() applied :





Terrain after a full refresh





See how the lightning is different ?

Somehow it seems to reapply the texture when you refresh the terrain. It seems you are using the HeightBasedTerrain material, where the textures are mapped by height. You can set the last textures endpoint to a heigher value, and then it should look as you expect.



The only thing the grid takes care of, is changing its sub-quads. In all other cases the calls are handled by the undelying TerrainQuad class. I’ll dig into this, and see what might cause the problem.

Thank you for your kind answer,



If I move far away from the freshly adjusted Quad (to make sure it is removed from cache), then return : the texture is then “fixed”, as in the second picture. Something is done during the Quad’s creation which isn’t updated with adjustHeight().



I will patiently wait until the clever people find the fixex. I gave it a try, but I’m not experienced enough yet ^.^

I’m sorry but the adjustHeight() bug in TerrainGrid is really blocking my development.

I understand you don’t have the time to fix the bug, so if you don’t mind, I will give it another try…

I wouldn’t mind some tips about how the algorithm works, Sploreg ?

When you say your “heightmaps in memory are updated correctly”, is this when you edit?

Also, when you are editing, are you editing one grid cell, or are you also editing other grid cells and it only turns bad when the camera moves?

I have a copy of the heightmap ( float[] ) in memory. My algorithm updates its heights correctly.

I can edit up to four Terrains at the same time.

I added this chunk of code in TerrainGrid and it works :

[java] @Override

public void adjustHeight(List<Vector2f> xz, List<Float> height) {

Vector3f currentGridLocation = getCurrentCell().mult( getLocalScale() ).multLocal( quadSize );

for ( Vector2f vect : xz )

{

vect.x -= currentGridLocation.x;

vect.y -= currentGridLocation.z;

}

super.adjustHeight( xz, height );

}

[/java]

Why are you overriding the adjustHeight method?

TerrainGrid is a TerrainQuad, and it will handle the height adjustment for you.

Because it’s not working if I don’t do this :slight_smile:

I don’t know why, I just know the Override above fixes the problem.

It is possible there is a bug in some of the global vars that are used to look up the location of the point on the cell; that kind of looks like the culprit.

Possibly, lemme know if you find a fix ^.^

Thanks for your help.

1. In TerrainGrid, I had the occasional Exception with the executor thread :

[java] public void run() {

for …

for …

if ( ( heightMapGrid != null ) && ( material != null ) && ( cache.get(temp) == null) ) {

[…]

[/java]

I had to add the ( heightMapGrid and material != null ) checks to prevent them from occuring. It’s a minor bug since it solves itself with the next executor iteration.



2. In TerrainQuad, I think I discovered a memory leak. If you try to destroy a Quad from the 3D world, the executor will always remain in memory. Try to create and destroy many Quads, you will fill your RAM up to an Out of memory error.

I implemented this method to fix this (in TerrainGrid) :

[java]public void destroyGrid()

{

try

{

executor.shutdownNow();

executor.awaitTermination( 1, TimeUnit.SECONDS );

listeners.clear();

setMaterial( null );

detachAllChildren();

removeFromParent();

cache.clear();

heightMapGrid = null;

} catch ( InterruptedException e )

{

e.printStackTrace();

}

}[/java]

  1. having used TerrainGrid as intended, the executor runs only after the heightMapGrid and material was set and you call initialize. So this fix might be just hiding a bug somewhere else.


  2. if you found a bug in TerrainQuad, why are you destroying the grid? The memory should be ok though, as max 16 quads are in memory at one time. How long does it take for you to get an OutOfMemoryException? I mean how many grid change does it take?

1. Yes I agree, It certainly hides a (minor) bug somewhere else. I set the material before calling initialize, and the heightMapGrid is loaded automatically through the events. I will investigate on that one…



2. I destroy and rebuild a fresh grid to force a full texture refresh, to fix the texture bug I pointed out in my first post (see both screenshots).



The JVM doesn’t garbage collect the TerrainQuad object : it still has a live executor Thread. If you shutdown the executor properly, the JVM will garbage collect the Quad.



If I use .removeParent() instead of my method. A grid of 513 will fill my memory in about 10 cycles.





PS: Tell me if my ‘bug reports’ are helping or not. I don’t want to be a bother :wink:

Bug reports are always, definitely, wanted :slight_smile: Without them we would often never know they are there.

The executor in TerrainQuad for LOD will have to clear itself with shutdown() if it is being removed from memory. Even just being removed from the active quad.

Simple fix though, I had the quad create the executor when the variable was defined, so whenever a quad was created there would be an executor, even if it was never used! (sub-quads do not end up using the executor).

I will change it to not create the executor service unless it knows it needs it, and that should only be in the very root TerrainQuad, in if using TerrainGrid, in the top TerrainGrid.



EDIT: it’s in SVN now

1 Like

Thank you Splored.



Your fix helps a lot, I don’t have Out of memory Exceptions anymore.



On a side note :

I see you removed RawArrayHeightGrid saying it’s redundant . but redundant of which object ?

GeoMap has some similarities, but it uses a FloatBuffer instead of a float[], and it doesn’t extends AbstractHeightmap.

oh yes sorry I was going to post about that.

RawArrayHeightmap is exactly the same as RawHeightmap, just don’t call load() on RawHeightmap (it will blow away the height array you pass in). You don’t even need a heightmap class for using terrain, just pass in the float array of height values, that’s all it wants is a raw array of heights. Heightmap classes are just tools for importing or generating those height arrays.



GeoMap is not a heightmap, they are very separate. Heightmaps produce a float array of height values that terrain uses. When the terrain is created it splits up the height data and in the end passes it to a GeoMap that will produce a mesh for the terrain. GeoMap is a very internal class.

I see. The original reason I created this object is HeightMapGrid.

[java] public HeightMap getHeightMapAt(Vector3f location);[/java]

And if you look TerrainGrid’s code :

[java]HeightMap heightMapAt = heightMapGrid.getHeightMapAt(temp);

TerrainQuad q = new TerrainQuad( […] heightMapAt == null ? null : heightMapAt.getHeightMap(), […] );[/java]

Maybe it would be simplier and/or more logical to just return the float[] in HeightMapGrid

You could still use RawHeightMap with the array constructor to pack your data. It was designed this way for some outdated reasons, so I can just as well change that to an array. I’ll see, what if that would work. :slight_smile:

heightmapGrid is internal to TerrainGrid, you shouldn’t be accessing it outside. Why do you need it?

and you can just call getHeightMapAt(Vector3f location).getHeightMap(), that will return the underlying float[].