News:

Simutrans Forum Archive
A complete record of the old Simutrans Forum.

Allow load height map with high mountains

Started by jk271, November 13, 2015, 07:16:33 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

jk271

Simutrans let you create a random map with maximum mountain height of 32 tiles above 0.
It gives you sufficient range heigh levels.
However, if you load a height map, the highest mountain can only reach 20 tiles above 0.
It would be nice to be able to load height map fulfilling the whole range of available height levels.

The attached patch let you use all availble height levels with height maps.

kierongreen

There was an issue here with making sure that heighmaps could be loaded with single and double height paks (conversion code doubles heights for double height paks which could break maps if some limits weren't imposed).

Ters

One problem I think I see with this patch is that it means existing height maps will load differently. Since I don't load height maps, I don't have any opinion on how much of a problem that is.

jk271

Thank you for you feedback!

Quote from: kierongreen on November 13, 2015, 08:13:01 PM
There was an issue here with making sure that heighmaps could be loaded with single and double height paks (conversion code doubles heights for double height paks which could break maps if some limits weren't imposed).

I am not sure, which limits you mean. I consider limit of maximum heigh of 16 tiles above 0 on single height pakset (and 32 on double height pakset) and limit of signed char variable.

It led me to the following calculations:

The original code loads 0 from height file

single height pak:  (( 6 * 0 ) / 4 - 224 ) / 16 = -14
double height pak:  (( 6 * 0 ) / 4 - 224 ) / 8 = -28

The original code loads 255 from height file

single height pak:  (( 6 * 255 ) / 4 - 224 ) / 16 = 9
double height pak:  (( 6 * 255 ) / 4 - 224 ) / 8 = 19

Therefore, the current version can return values from -14 to 9 for single height pack and from -28 to 19 for double height pak.


The patched version loads 0 from file

single height pak:  (( 18 * 0 ) / 8 - 320 ) / 16 = -20
double height pak:  (( 18 * 0 ) / 8 - 320 ) / 8 = -40

The patched version loads 255 from file

single height pak:  (( 18 * 255 ) / 8 - 320 ) / 16 = 15
double height pak:  (( 18 * 255 ) / 8 - 320 ) / 8 = 31

The patched version can return values from -20 to 15 for single height pack and from -40 to 32 for double height pak.


In both cases, in current version and in patched version, the imported values are stored into signed char variable (range -128, 127).
The varible can store 31 as well as -40.
Everything below -20 in double height paksets and below -10 in single height paksets is seen as water.
As a result, I am not worried about -40.

Furthermore, both versions load 128 as height of -2 (the default sea level configuration in single height paksets) or as -4 in double height paksets.

Quote from: Ters on November 14, 2015, 10:43:17 AM
One problem I think I see with this patch is that it means existing height maps will load differently. Since I don't load height maps, I don't have any opinion on how much of a problem that is.

About compatibility:
If the height map is generated from a real terrain and it contains more detailed levels, than simutrans currently uses, you will get better result. I use maps rendered from data from SRTM http://srtm.csi.cgiar.org/. Each new height level available in simutrans significantly improves impression from the landscape, especially if the landscape is not flat, but it is not fully covered with 32-tiles high mountains.

If the height map is created by hand and uses only some height levels, you can get steeper hills on some places on the map.

Ters

Quote from: jk271 on November 14, 2015, 04:47:33 PM
About compatibility:
If the height map is generated from a real terrain and it contains more detailed levels, than simutrans currently uses, you will get better result.

But you will not be able to recreate your earlier maps, try a different strategy this time, and compare the results. Without tweaking the height map in an image editor, that is, which might be non-trivial due to rounding differences. My point is that sometimes better is less desirable than consistency, and with Simutrans, there are many points-of-view.

jk271

Quote from: Ters on November 14, 2015, 06:40:53 PM
But you will not be able to recreate your earlier maps, try a different strategy this time, and compare the results. Without tweaking the height map in an image editor, that is, which might be non-trivial due to rounding differences. My point is that sometimes better is less desirable than consistency, and with Simutrans, there are many points-of-view.

I have tried several maps from maps.simutrans.com. I have not found any hand-made one, that can employ all height levels of double height paksets and therefore might be broken due to the patch.
What about changing the height map import for double height paksets and letting the import for single height paksets untouched?

Ters

They don't need to employ all height levels. My concern is: if a person has a height map, and has used that to create one or more game maps, can that person create new game maps that are exactly the same using the same height map, after this code is changed? I think not, because the whole point of this patch seems to be to change the conversion between height map color and in-game terrain elevation. Is that a problem? That probably depends on who you ask. Or even when.

Vladki

What about a compatibility switch for loading old height maps?
I would really like the height maps tu use as much levels as possible.

Ters

Quote from: Vladki on November 15, 2015, 07:11:33 PM
What about a compatibility switch for loading old height maps?

That's a possibility if the ability to load old height maps as they always have been is an issue.

jk271

Quote from: Vladki on November 15, 2015, 07:11:33 PM
What about a compatibility switch for loading old height maps?
I would really like the height maps tu use as much levels as possible.

This suggestion solves the problem. Users will be able to load heigh maps in both ways. The current one will be default.

I will add a configuration item into "settings" > "landscape setting" to let user choose, how to load the map.

I will have to thing about forward compatibility too. E.g if more higth levels were available.

jk271

I have added a new configuration option "height_map_conversion_version" into "settings" > "landscape settings". It let user to choose between old way of loading heigh_maps (0) and new one (1). The default value is 0. I did the configuration option as int, not as a boolean, as more height levels might be available in future versions.
I struggled with a suitable name for this configuration option. If you have a better name, feel free to change it.

Why I coded it this way?
The change - new configuration option - requires ifs to be added. The ifs would have to be added on three places in code. It would be an ugly solution.
A new function (rgb_to_height) is suitable for that.
The rgb_to_height() function is called many times, therefore it shall be an inline function.
Header file "simworld.h" is included into almost all .cc files, therefore I moved the part of the code into a separate class with its own .cc and .h files.

jameskuyper

jk271's message of 2015-11-14 implies that the conversion from height map values to simutrans levels is level = ((6*height/4)-224)/16 for single-height paks. If that's correct, it means that for height from 129 to 139, level is -1, for height from 140 to 159, level is 0, and for height from 160 to 170, level is 1. In other words, given an evenly sloped height map, there will be nearly twice as many tiles with a level of 0 as there are of adjacent levels. This is because integer division rounds toward 0, so that (-5)/3 == 0, just as 5/3 == 0. Prior to C99, it was permitted for implementations of C to have integer division round toward negative infinity, so that (-5)/3 == -1, which I preferred, since it avoids problems like this one.
This could be avoided by re-writing the formula as 6*height/64 - 14, because it never involves integer division of negative numbers. Since that formula is simpler, it occurred to me that use of the more complicated formula might be intentional: was the purpose to deliberate create a shelf at level == 0 which had half the slope of the tiles at level == -1 or level == 1?

jk271

Quote from: jameskuyper on December 13, 2015, 11:11:54 PM
This is because integer division rounds toward 0, so that (-5)/3 == 0, just as 5/3 == 0. Prior to C99, it was permitted for implementations of C to have integer division round toward negative infinity, so that (-5)/3 == -1, which I preferred, since it avoids problems like this one.

It is interesting to know this. I was not aware of it, thank you for pointing to it.

Actually, the original code uses bit mask:
(((r*2+g*3+b)/4 - 224) & 0xFFF8)/8
I had not known why the mask is used, therefore I kept the formula for loading legacy height maps untouched.




I don't know, whether there is some additional work on this patch expected from my side, or the way I solved the problem is not pleasant, or the developers are too busy.

prissi

Incorporated in r7794

I added the switch directly to the dialogue box.

kierongreen

You still need to add height_map_loader .cc and .h to svn control prissi then recommit.

prissi

Yes, I noticed that in the morning too. Well, I should not commit after midnight ...