r5830 new landscape (binary and source versions)

[Dwachs]

Split the bug report about crash with broken factories:

http://forum.simutrans.com/index.php?topic=11882.0

[Dwachs]

Here is an update. Did some tiny modifications to ribi.h and wegbesch.h.

It would be awesome if we could use the water animation also for the transition tiles. Would it be possible to implement this, Kieron? (not saying that you should be the one who implements..)

Edit: deleted patch

[kierongreen]

Water animation for transition tiles would be possible. You'd need to convert water animations to textures rather than tile images - then you'd need to generate flat images for each water depth, and slope images for water depth 0. For pak64 that means 5*8+64*8=552 images which is fine. The problem is likely to be having to redraw landscape tiles every animation step, as shore tiles require at least 2, more often 3 or more draws each (water, sometimes sand, land climate(s)).

Also your modifications to the code have broken climates only occupying 1 tile. Left shows behaviour as of 6500, right shows behaviour with 6503.

[Dwachs]

Also your modifications to the code have broken climates only occupying 1 tile. Left shows behaviour as of 6500, right shows behaviour with 6503.

Did not notice that, I think this is already broken in the first patch I posted.

Edit: This is also broken in the patch against r6448 you posted above

Edit2: Updated patch. It seems that some index into transition_water_texture were wrong. The patch only uses index 0..3 and 11.

[TurfIt]

Since I seem to be timing everything lately, might as well here too:
  sync_step sync: 2.5% slower.  sync_step display: 1% slower.  step: 2% slower.
Reasonable for the added features. This was with the normal yoshi game, so all single height, single climate per level.
A couple of anomalies - senke_t:sync_step was slowed 240%. possibly the get_climate() calls?
- karte_t:step pending season change slowed 17%.
- karte_t:step step players slowed 15%.
- karte_t:step step halts faster 25%. ??
None of these are overly significant to the overall, but standout as most affected.


What's the status with DOUBLE_GROUNDS? I note the compile fails without it defined. And with, it now loads old paks which is very good!


The water height tools can still use some work too. I frequently get greeted with 'Cannot alter water' when I would otherwise expect to be able to.
Taking flat terrain, and raising a levy around an area (slope up, slope down - no full level tile at the higher level) doesn't allow the raise water. But raise an extra ring of ground such that there's a slope up, level tile, slope down, and it allows the raise. You can then lower the outer ring ground back and things are fine.

You also can't raise the water up to the level of the surrounding flat tiles, yet you can use the set climate tile to set flat tiles to water and achieve the same thing. The results achievable using the tools should be the same IMHO.

[kierongreen]

TurfIt - try benchmarking when the view is showing some coastline - that'll give an idea of worst case performance hit.

The reason you can't have a one tile levy without a full tile at the higher level is that this would result in a transition with water at the top of the hill (which would look rather odd). As for raising water up to the level of surrounding tiles - the routine tries to cover all tiles at that height with water (and fails if any tile is occupied or it reaches the edge of the map).

[TurfIt]

The benchmark view was zoomed out at the default position the yoshi game loads to. Has about 30% water in sight - with the associated coast. Obviously no inland lakes since the game predates that ability.

I'll whip up some screenshots for the water tools to better explain...


Won't allow the water raise inside the levy. If the levy is extended to have the flat tile too as shown to the east, then the raise succeeds. The extra flat tile can then be lowed again with the raised water remaining as shown to the north.



Won't allow the raise. I'd expect the raise to occur resulting in what's shown above it. That was created using the set tile climate tool instead.

[kierongreen]

That's good if that slowdown is at maximum zoomout!

Btw snowline seems to be broken now as well...

Edit:
Right, with those levies with no central tile the issue is that it is finding no border at all as all tiles are at the same level - it's only tiles which have a greater height which are recognised.

[Dwachs]

A couple of anomalies - senke_t:sync_step was slowed 240%. possibly the get_climate() calls?

these can be easily cached - it is only computing whether a snowy image should be shown.

- karte_t:step step players slowed 15%.

That is particularly funny, as spieler_t::step actually does nothing...

What's the status with DOUBLE_GROUNDS? I note the compile fails without it defined. And with, it now loads old paks which is very good!

I may have broken this. Imho we can throw all the non-DOUBLE_GROUND code away.

[Dwachs]

Here is the fixed patch - snow transitions working again. Took me a whole afternoon to find the reason, which was just a missing 'break'.

[kierongreen]

Many thanks have to say it had me puzzled too or I'd have fixed it

[TurfIt]

Was the change in the 6516 version in get_bild_ptr() intended here:

Code Select Expand


case hang_t::suedwest*2+hang_t::nordost*2+hang_t::suedost*2+hang_t::nordwest*2:
all_rotations_slope[slope] = transition_slope_texture->get_bild_ptr(0)->copy_rotate( 0 );
all_rotations_beach[slope] = transition_water_texture->get_bild_ptr(11)->copy_rotate( 0 );
break;
}
}
else {
all_rotations_slope[slope] = NULL;
}
break;

case hang_t::suedwest+hang_t::nordost+hang_t::suedost+hang_t::nordwest:
all_rotations_slope[slope] = transition_slope_texture->get_bild_ptr(0)->copy_rotate( 0 );
all_rotations_beach[slope] = transition_water_texture->get_bild_ptr(11)->copy_rotate( 0 );

transition_water_texture->get_bild_ptr(11) breaks compatibility with old non double height paks.
Reverting from 0,11 0,11 to the previous 14,4 0,0 appears ok. But I can't see anything wrong with the snowline even with 6503 and the missing break...

[kierongreen]

The final image from transition_slope_texture and transition_water_texture are images for a one tile section of climate surrounded by different climates on all sides. transition_water_texture->get_bild_ptr(11) doesn't need to be as high as 11, it could be 4 instead as only 0-3 are used, but transition_slope_texture does need to be at 14 as transition_slope_texture has a lot more images to allow for snow transitions. I thought the patch applied to 6516 worked but I'm going to test again today (unfortunately I have to go to work now so it will be this evening before I can confirm behaviour).

[kierongreen]

the following code functions correctly for both double height paksets and older ones:

Code Select Expand


                        case hang_t::suedwest*2+hang_t::nordost*2+hang_t::suedost*2+hang_t::nordwest*2:
                            all_rotations_slope[slope] = transition_slope_texture->get_bild_ptr(14)->copy_rotate( 0 );
                            all_rotations_beach[slope] = transition_water_texture->get_bild_ptr(11)->copy_rotate( 0 );
                            break;
                    }
                }
                else {
                    all_rotations_slope[slope] = NULL;
                }
                break;

            case hang_t::suedwest+hang_t::nordost+hang_t::suedost+hang_t::nordwest:
                if(transition_slope_texture->get_bild_ptr(14)) {
                    all_rotations_slope[slope] = transition_slope_texture->get_bild_ptr(14)->copy_rotate( 0 );
                }
                else {
                    all_rotations_slope[slope] = transition_slope_texture->get_bild_ptr(0)->copy_rotate( 0 );
                }
                if(transition_water_texture->get_bild_ptr(11)) {
                    all_rotations_beach[slope] = transition_water_texture->get_bild_ptr(11)->copy_rotate( 0 );
                }
                else {
                    all_rotations_beach[slope] = transition_water_texture->get_bild_ptr(0)->copy_rotate( 0 );
                }
                break;

Although thinking about it again if(double_grounds) should be just as good a check....

Anyway, attached is patch to latest svn with above code in.

[TurfIt]

Something still doesn't seem right with the transitions for non double ground.
simscr00 - double grounds.
simscr01 - single.

And, I still can't see any difference between 6503 with 14,4   0,0  and 6516 with 0,11  0,11 and now 6526 with 14,11  14,11/0,0.  Where should I be looking?  I find this entire huge switch full of magic numbers and not a comment in sight completely undecipherable.

Also, is it intended for single tile climates to have different sizes as shown in the screen shots? i.e. Setting a climate that used to be for lower heights at a higher level give a little dot whereas higher climates on a lower level extend into adjacent tiles.

[kierongreen]

Single height paks won't have the transition images to display climates properly, or the tools to create them. So unless you have added the tools to a single height pak, or loaded a game that was created in a double height pak then you wouldn't have different climates at the same height as shown in your screenshot. The only climate transitions you'll see in a single height game are those generated at map start, between climates at different heights, from land to beach to water, and to water directly. These all work correctly using the textures present in existing single height paks. I admit I've not tested a single height pak that's been updated to have climate transitions but not double heights but is this a likely scenario?

As for magic numbers - could maybe enum those? However that's likely to give rather cryptic too unfortunately as they'd be descriptions of the different textures. Some of these might seem to be obvious - straight, corner, dot for example, but there are variants of these to consider too. Comments would have the same problem as all you could add would be the slope type - which is already given in the line "case hang_t::suedwest+hang_t::suedost+hang_t::nordost*2:" for example.

I'll happily admit that the entire texture system (as first introduced years ago and extended with this patch) does have a bit of a voodoo programming feel to it but that's because it was all thought up in my head! See the transforms in create_textured_tile_mix for example...


The easiest way of thinking about this bit of code is that it calculates the texture to use when overlaying snow onto a tile with slope slope. The only non intuitive slopes are then hang_t::suedwest+hang_t::nordost+hang_t::suedost+hang_t::nordwest and hang_t::suedwest*2+hang_t::nordost*2+hang_t::suedost*2+hang_t::nordwest*2 (actually I'm pretty sure hang_t::suedwest+hang_t::nordost+hang_t::suedost+hang_t::nordwest isn't used) - these are transitions to the little dot climates. When I first coded the system water had a similar structure, but I then added climates too which allowed me to remove the more complicated water transitions. This means that while slopes need 15 transition textures water only needs 5.

That single tile climates have different sizes is purely a reflection of how the transitions work. A higher number climate spills over into neighbouring tiles. Water spills into all neighbouring tiles also. The only way to make these transitions consistent would be to split the transition over 2 tiles which is more effort, more textures, and lower performance.

[TurfIt]

The simscr01 attachment appears to show the four right climates with working flat (non-slope) transitions, and the water at the far left, only the two small squares appear transition lacking. Hence, I thought it was simply a problem displaying them, not that they didn't exist... but then why do those 4 look right?

I did add the climate tools to a non double height pak - just added the menuconf.tab entries.

Not sure how to make this code more readable, other than perhaps extensive comments. When I see " hang_t::suedwest+hang_t::suedost+hang_t::nordost*2", all I can think is WTF!

Beyond that, I think it's time to get this into trunk. It seems stable, and now has backwards compatibility with old paks/games, and with only a minor performance hit to old games.
If no one else has done any further work, I'll remove all the DOUBLE_GROUNDS #ifs. I don't think the 2.5% performance hit is worth maintaining a separate .exe for single heights.
Also, I'd like to give some consistency to the whitespace coding style - it's rather schizophrenic right now...

[kierongreen]

hang_t::suedwest+hang_t::suedost+hang_t::nordost*2", all I can think is WTF!

Southwest corner Height 1, Southeast corner 1, Northeast corner 2, (Northwest corner 0 as not mentioned) - Simple! :p

I thought it was simply a problem displaying them, not that they didn't exist... but then why do those 4 look right?

The patch makes use of existing texture transitions where it can. This is enough to cover everything in single height paks apart from climate transitions to "dot" climates.

Also, I'd like to give some consistency to the whitespace coding style - it's rather schizophrenic right now...

Well that's a wider issue in simutrans - I've tended to adopt the style of whichever section of code I've been working on at the time...

[TurfIt]

ahh. Corners - not slopes! I was having issues with southwest slope + southeast slope + ... !

But aren't transitions to 'dot' climates just 4 overlaid transitions from 'dot's?  Anyways, not terribly important if can't be easily done; 'dots' should be rare.

Will never get to a consistent style if keep following whatever's there. I thought the policy was to always use the 'new' styles to slowly change things. Same with translations - slowly change to English.

I take it you have no further changes in progress... I'll try to get to the DOUBLE_ removal tomorrow and post up something I hope's ready for committal.

[kierongreen]

No changes in progress just this second and I'm not going to have any free time till Sunday. Transitions to dot climates are separate images not overlayed corners as there's no guarantee that overlaying would work.

[TurfIt]

Time for this to hit trunk IMHO...

There's still some work required to make the per tile climates truly functional - need to completely decouple from the old height based climates. The water level tools need to be have more intuitive rules too - as detailed above. But, that can all be worked on later; Patch is big enough as is.

[kierongreen]

Indeed once this patch is in trunk there are a few components that could be improved on easily by different people, initial climate distribution and water tools are a couple, but also bridges for example.

[prissi]

Why not, we just released. So now is the right time. Then it can ripen over summer (I fear).

[VS]

I have end of semester, two conferences, and a lot of volunteering ahead. Ripening over the summer is more or less guaranteed, as long as we talk about graphics

[Sarlock]

Agreed, this looks like a good summer for ripening.  I probably won't have much time for graphics until August.

My lawn and garden look nice though

[Markohs]

Just move it to trunk now, we'll fix whatever comes around. And we just released, so go ahead imho.

[The Hood]

(loving the fact that the patch was written by a pak128.Britain artist and therefore should be ready to go...!)

@kierongreen - let me know what/when to make changes, or alternatively you could upload them to sourceforge yourself if you have permissions?

[rainer]

Hi friends!

First of all: Many, many thanks for all your effort in this improvement!

As I would really like to see this work and include it in pak64.ho-scale, I downloaded Simutrans 112.4-6527 from the nightly page and the latest pak128.Britain for testing. But to my real shame I can't see any difference. What am I doing wrong?

For sure, it is obvious for me that I have to produce a bunch of graphics. But I am not afraid of that.
In case of pak64.ho-scale, I would like to reduce the overall height step to some 8 to 10 pixels, and the half height to 4 to 5. I am really, really curios about the result and all the new possibilities to build more realistic landscape and track/street systems. : - )

Go ahead with your great work! I love it!

[kierongreen]

Don't think I have sourceforge permissions. Just copy the latest pak128.Britain sources from this thread onto sourceforge. The only missing graphics are for maglevs.

[rainer]

(...) Just copy the latest pak128.Britain sources from this thread onto sourceforge. (...)

Hm. Tried to search the 9 pages of this thread for a link to that sources. Without success.
Any hint or URL? : - )

[kierongreen]

http://forum.simutrans.com/index.php?topic=10242.msg115142#msg115142

That post has sources and compiled pak files.

[TurfIt]

And.... incorporated r6530.

[Markohs]

A waring shows up in MSVC I guess in gcc too:

1>c:\code\simutrans\trunk\boden\wege\../../simworld.h(1654): warning C4244: '=' : conversion from 'sint16' to 'sint8', possible loss of data


What's the best option, to change the parameter of the function or making a explicit cast?

Code Select Expand


   /**
    * Sets water height.
    * @author Kieron Green
    */
   void set_water_hgt(koord k, sint16 hgt) { water_hgts[k.x + k.y * (uint32)(cached_grid_size.x)] = (hgt); }


water_hgths is a sint8 array.

Compiled and executed without problems, works with single height pak128 and with double height pak128.britain . Thank you for your work!

EDIT: I see the commit might be late for the nightly and not generate until tomorrow. I compiled it, you can download here for the curious. It's compiled with MSVC so it might not work on all computers since binaries seem to be pickier to distribute than mingw ones. Try installing http://www.microsoft.com/en-us/download/details.aspx?id=30679 if it doesn't work.

Or you can wait for next nightly.

[TurfIt]

No need for the sint16 there, I changed to sint8 - everybody calling this is passing a sint8 already.
GCC doesn't warn here.

[Ters]

I can't remember GCC ever putting up a warning about type size mismatches. A few weeks ago, I had mistakenly used the wrong type, and not a word from the compiler about it.