Make world limits not forced to water level

[An_dz]

2 issues I found:
1. This is probably easy to correct, slope tools don't work on map borders. It's a little weird be able to create a mountain but not a simple slope.

2. 96comic have a different slope image. Rather than using images 8/19 bottom texture as images 4/15 it uses the same top texture. And it looks little weird in the world borders, I changed the code to only use images 4/15 on border and images 5/16 to all other heights. Here's the result:

It looks smoother, but since only 96comic have textures like described it's the only affected pakset.

Extreme easy and simple change:

Code Select Expand


Index: grund.cc
===================================================================
--- grund.cc (revision 6516)
+++ grund.cc (working copy)
@@ -971,7 +971,12 @@
diff ++;
// ok, now we have the height; since the slopes may end with a fence they are drawn in reverse order
for(  sint16 zz = pos.z-welt->get_grundwasser();  diff <= zz;  diff ++  ) {
- display_normal( grund_besch_t::slopes->get_bild(15), x, y, 0, true, false );
+ if (zz == diff) {
+ display_normal( grund_besch_t::slopes->get_bild(15), x, y, 0, true, false );
+ }
+ else {
+ display_normal( grund_besch_t::slopes->get_bild(16), x, y, 0, true, false );
+ }
y -= hgt_step;
}
#else
@@ -1010,7 +1015,12 @@
diff ++;
// ok, now we have the height; since the slopes may end with a fence they are drawn in reverse order
for(  sint16 zz = pos.z-welt->get_grundwasser();  diff <= zz;  diff ++  ) {
- display_normal( grund_besch_t::slopes->get_bild(4), x, y, 0, true, false );
+ if (zz == diff) {
+ display_normal( grund_besch_t::slopes->get_bild(4), x, y, 0, true, false );
+ }
+ else {
+ display_normal( grund_besch_t::slopes->get_bild(5), x, y, 0, true, false );
+ }
y -= hgt_step;
}
#else


[kierongreen]

This change will be moot once double heights are introduced anyway.

[An_dz]

I see, double heights don't have this double size images, it uses a single image per height.
Can't new image slots be used to add a top and bottom image? These would increase in only 4 images.
pak128 and pak96.comic would lose their top or bottom textures with this.

[Markohs]

5 bit per color won't allow for much gradient unfortunately...



Without textures this won't look very good.

EDIT: choosing levels a bit better, well, doesn't look so bad, but without textures it doesn't feel similar to the rest of the map, this needs texturing.

I'll implement seabed first and then texturing.

[sdog]

wouldn't a bit noise be enough to make the cut's face look good?

[Markohs]

mmm... yes, it can work, something like a lightmap... I'll make some tests.

[Markohs]

I made a lightmap fast implementation and it doesn't look bad, but I'm having some problems. Two questions:

On the climate code, calc_water_level() generates all required images, and stores them in ground_bild_list, a slist. Ok, it's needed to remove them on next re-generation, but I can't see at wich point in the code they are added to "static const grund_besch_t *border;", that member variable is inited to NUL, and I'm unable to see in wich part of the code it's assigned to some kind of complex structure, that's able to contain various bild_besch_t, that are later used in grund.cc . I guess it's obvious but can't  see where is that mechanism, anyone can point me what's behind this?

Another thing, it's I thought, related to PIXVAL values, that if the bit 15 in that value was set to 0, I caould treat just each component as 5 bits and be safe from special colors. Looks like not, because I'm getting dark blue pixels on the generated images. I'm wrong or the bug is anywere else?



It might be a bug somewere in the way I generate the image because when I zoom too much I get a crash, so I'm clearly doing something wrong. But what are those dark blue pixels doing there?

I'll show my code if you want, but I'd prefer to not do it so because I'm still ashamed of its quality, it's the first time I explore this part of the code and it's quite complex. It's full of tweaks and tests.

Thx.

[Ters]

I assume that when you are referring to bit 15, you are counting from 0. What do you do if this bit is actually set? Special colors have a tendency to sneak into images from time to time.

[Markohs]

Well, yes, the highest weight bit in a 16-bit number, bit 15.

Well, I think I remember that all special colors had that bit set to 1, and that the ones with the bit to 0 are the actual game colors, that will get dark if you adjust night level. I just designed the routines that generate images to output pixels like 0RRRRRGGGGGBBBBB  and assume the color will be correctly treated.

I just basically generate a world slope that will cover the highest allowable slope in-map, with a color gradient, and then I apply a lightmap pattern to give it some texture. I mix both values with this routine:

Code Select Expand


static PIXVAL map_color(PIXVAL map, PIXVAL src){
   if(map==0) {
      return 0;
   }

   uint16 rc = red_comp(map);
   uint16 gc = green_comp(map);
   uint16 bc = blue_comp(map);

   uint16 rs = red_comp(src);
   uint16 gs = green_comp(src);
   uint16 bs = blue_comp(src);

   // overflow safe method ...
   uint16 rcf = (rc*rs)/31 & 0x1F;
   uint16 gcf = (gc*gs)/31 & 0x1F;
   uint16 bcf = (bc*bs)/31 & 0x1F;

   return (bcf)|(gcf<<5)|(rcf<<10);


}


The 1f masking it's not necessary I think, but put it the re to be sure...

The three missing routines are actually the same macro kierongreeen uses in the climate code.

Code Select Expand


#define red_comp(pix)         (((pix)>>10)&0x001f)
#define green_comp(pix)      (((pix)>>5)&0x001f)
#define blue_comp(pix)         ((pix)&0x001f)


[Ters]

So no color values come from loaded images? Because if they do, I would suspect that that image contains special colors. It does not appear that special colors in the source image files have the same value as they do when found in PIXVAL variables in the game, so that is something to look out for in that case.

[Markohs]

oh, that makes sense. the lightmap comes from a file, i'll have it a look thx.

[Dwachs]

I made a lightmap fast implementation and it doesn't look bad, but I'm having some problems. Two questions:

On the climate code, calc_water_level() generates all required images, and stores them in ground_bild_list, a slist. Ok, it's needed to remove them on next re-generation, but I can't see at wich point in the code they are added to "static const grund_besch_t *border;", that member variable is inited to NUL, and I'm unable to see in wich part of the code it's assigned to some kind of complex structure, that's able to contain various bild_besch_t, that are later used in grund.cc . I guess it's obvious but can't  see where is that mechanism, anyone can point me what's behind this?

There is this magic behind:

Code Select Expand


static spezial_obj_tpl<grund_besch_t> grounds[] = { ...
    { &grund_besch_t::borders, "Borders" }, ...

Afaik, these entries are assigned in grund_besch_t::register_besch(), which calles ::register_besch(), which is the method in spezial_obj_tpl.h

[Ters]

That is a common way for Simutrans to find mandatory besch-es by the way.

[Markohs]

Hummm.. you are right, the problem is the lightmap contained special colors, now looks better:



The crash on zoom in is as usual on last problems I use to face, a limit in simutrans code data types. Since the default slope I'm creating it's 416 pixels high and on big zoom levels I'm overflowing some limit, a 16-bit number I think.

[prissi]

I think there is no need to change the texture below 5 levels below the surface or so. (Most people will no look this deep, or?)

[Markohs]

yep, prissi, I was thinking 4 or 5 levels under water too. there are just 3 depth levels in water,  so more than 5 are too much.

[Markohs]

I'm having problems as usual with this, have been hours trying to debug this with no success.

The image has no special colors, the generated slope has no special colors, I think the encoding is correct, but this still crashes on zoom-in, and I can't understand why. If anyone wants to have a look at this, I'd appreciate it, I'm quite sure it must be a stupid error but it's driving me crazy.

The slope is about 64x420 pixels, this should not be of any problem for simutrans to manage, but rezoom_img it's giving segfaults. I'm a bit desesperate and cluesless of where can the problem be. I know the implementation it's dirty and hackish, but I can't see any reason of why this shoudn't work.

I tested this on pak128.

https://dl.dropboxusercontent.com/u/30024783/slopes/segfault.patch
https://dl.dropboxusercontent.com/u/30024783/slopes/ground.LightSlope.pak
https://dl.dropboxusercontent.com/u/30024783/slopes/world-slope.png
https://dl.dropboxusercontent.com/u/30024783/slopes/world-slope.dat

[Ters]

The thing that goes kaboom when I try this (with pak64) is attempting to free the memory pointed at by zoom_data. I haven't had time to find out more than that yet.

[Markohs]

Yea, same on pak128, somehow the heap gets corrupted I guess because it's writing out of bounds. But why?

[Ters]

Sometimes my breakpoints in rezoom_img won't trigger at zoom_factor 0. If they do, freeing crashes then, if not, it crashes when I zoom back to zoom_factor 1. Most of the time I get a crash with a "complete" stack trace, but sometimes I get a trunkated one.



UPDATE:
I found something. rezoom_img() allocates two buffers of equal size (size), baseimage and baseimage2. If first writes to baseimage, then reads from baseimage and writes to baseimage2, then reads from baseimage2 and writes to baseimage, before copying from baseimage to a perfectly sized newly allocated final block of memory. However, at the third step (baseimage2 -> baseimage), baseimage is too small. When doing ((uint8*)dest) - baseimage when y=592, I get 60486, but size is only 60480. zoom_factor is 1 at this point, and y is supposed to continue up to 624.

[Markohs]

mmm.... Thx, that is certainly the cause of the heap corruption. Now I just need to know why it's writing more bytes that's supposed to, but it's indeed a very nice find. Thanks a lot Ters!

I checked and the re-compressing of the image doesn't render any transparent void zone that causes an exess size in bytes because of the RLE, neither could identify any special color being generated... But somehow the rezoom it's generating more bytes that's supposed to.

[Ters]

The question is whether it writes more bytes than it should, or if it supposes that there are fewer bytes needed to represent the image than can be safely assumed.

[Markohs]

It was indeed understimating the potential size of the image once recoded, I guess the algorithm never failed so far because simitrans had no scalable full solid images in any pak but my wild guess is this whould happen on all fully non-transparent images (just on those the height is more than 2 times taller than wide). Or even in a fully patterned image (one pixel transp, one solid pixel, one transparent, one solid...)

The "bug" come from simgraph16.cc, line about 1364:

Code Select Expand


// thus the unpack buffer must at least fit the window => find out maximum size
size_t new_size = newzoomwidth*(newzoomheight+6)*sizeof(PIXVAL);
size_t unpack_size = (xl_margin+orgzoomwidth+xr_margin)*(yl_margin+orgzoomheight+yr_margin)*4;


In my code, the image was about 32 x 620 pixels. The formula is not enough for the potential size of that image:

size_t new_size = newzoomwidth*(newzoomheight+6)*sizeof(PIXVAL);

I don't understand where that +6 comes from, but in a fully solid vertical stripe the correct size is:

size_t new_size = (newzoomwidth+3)*newzoomheight*sizeof(PIXVAL);

At zoom level 1, 3/2 zoom base_w 64 base_h 416  => w = 96, h = 624

size_t new_size = newzoomwidth*(newzoomheight+6)*sizeof(PIXVAL) => 120960 bytes buffer

at the end of the function zoom_len: 123552 -> We wrote out of bounds

with

size_t new_size = (newzoomwidth+3)*(newzoomheight)*sizeof(PIXVAL) => 123552 bytes buffer , no segfault


+3 because it will come as 0x0000 clear pixels, 0x0040 color run and 0x0000 to mark the end of the line.

A image could potentially be even larger if it used a pattern on transp-notransp pixels, that whould be, (I think)

size_t new_size = ((newzoomwidth*3/2) +1)*newzoomheight*sizeof(PIXVAL);

But I don't really think such a image exists anywere in simutrans, but who knows.

What do you think, am I right or have mislooked something?

Thanks for your help Ters!

Anyone knows where does that +6 come from? Because if it's for the extra bytes a rle-encoded image might add in respect a non-rle encoded one, maybe the safest definitive formula might be:

size_t new_size = (newzoomwidth+3)*(newzoomheight+6)*sizeof(PIXVAL);

But is this really necessary? I tested with just the +3 and all seems to work good...

[prissi]

Since it is only for rezooming, using the maximum possible size with transparency is probably even better to fix this once and for all. The pixl/transparent/pixel is used very frequntly in shadows of trees.

[Markohs]

Commiting this to trunk if nobody says the opposite soon then:

Code Select Expand


         // thus the unpack buffer must at least fit the window => find out maximum size
         // Note: This value is certainly way bigger than the average size we'll get,
         // but it's the worst scenario possible, a sucession of solid - transparent - solid - transparent
         // pattern.
         // This whould encode EACH LINE as:
         // 0x0000 (0 transparent) 0x0001 PIXWORD 0x0001 (every 2 pixels, 3 words) 0x0000 (EOL)
         // The extra +1 is to make sure we cover divisions with module != 0
         // We end with a oversized buffer for the normal usage, but since it's re-used for all rezooms,
         // it's not performance critical and we are safe from all possible inputs.

         size_t new_size = ( ( (newzoomwidth*3) / 2 ) + 1 + 2)*newzoomheight*sizeof(PIXVAL);


[Max-Max]

I'll implement seabed first and then texturing.

Is this with texture or auto generated? In any case I like this version most
Maybe this can be the fallback if there are no special images defined in the pak for "map walls"?
...or configurabel like textured map walls on/off...

[Ters]

It's the troublesome texture I had to help debug above. Looks a bit different since that screen shot, as can be seen in later screen shots.

[Markohs]

that's the slope without a lightmap applied.  You can achieve the same effect with a 100% white lightmap. lightmap is important because south slope it's suposed to receive more sun than east one, simutrans sun comes from south.

on my last screenshots my lightmap was too dark, I made a much lighter version I'll show here soon

[Max-Max]

Looking forward to this implementation, looks really nice 

[Markohs]

 While developing this, I noticed there are not much image creation and management routines in the game, just like the climate code does, this patch needs to create new images each time the world is created. kierongreen aproach to this was duplicating the image and rotating it if necessary. It's all implemented around bild_besch_t::copy_rotate.

I need to create *new* images, with arbitrary size and recode them using them the RLE, do you think it's a good idea to place this routines in bild_besch_t , routines that accept a uncompressed buffer and compress it or it's not the right place to code this?

Just asking, I'm doing it that way, but wanted to hear your oppinions for the case you point me to a better solution. We could maybe implement that in simgraph16.cc (rezoom routines are there), or in a new collection of routines called bild_tools::* or image_tools:: . Dunno.

[prissi]

The world limit should be also part of the ground images, imho. I would reuse the code from there. (And it generates new images, by combining three images or four.

[sdog]

112.3 is already in an official package for arch.
i've just started it up and had a look at it for a moment, it really looks good. Thanks a lot Markohs et al!

[Markohs]

Thx sdog.

@prissi: Okay, I'll try to mimic ground images code then. My initial approach is just one image per slope, no need to add even more images to simutrans if I can avoid to, we have more than enough.

[Markohs]

It's been some days since I coded something, back to war. Not much to show, I just added a few routines to bild_besch.cc to create images on the fly from a buffer. At calc_water_level I get wasserlevel = -82 now, I think a change in trunk changed this, I was getting -2 before, this makes the tripe too long.

The display has clipping and display errors, but it's a first step. For the curious developers, here is the code, comments welcome ofc.

https://dl.dropboxusercontent.com/u/30024783/worldslope-merged-double4.patch

[Markohs]

Still full of errors, but some progress:



I thought it was as easy as single height paks had 1 tile height step difference in the y axis, I was wrong. I have to fix this. Also have to find a place to do this so wasserlevel is not -81. Still WIP ofc. https://dl.dropboxusercontent.com/u/30024783/worldslope-merged-double6.patch

Offsets and east border is obviously very wrong now atm.