Patch: fix the clipping errors

[Dwachs]

This patch tries to fix the graphical glitches with longer vehicles.

The main idea is to use a clipping along tile edges with ways to avoid that sprites are drawn in a different order.

No profiling yet.

Please test! I did not want to post this in the public part to avoid to raise expectations.

Known issues:
-- ships (there is no way yet to determine the water ribi's)
-- sheep and airplane shadows when driving over tiles that are marked as draw_as_ding (e.g. if a vertical wall is behind)
-- roads with long tram and road vehicles on it may cause the (old) glitches  (I did not see this though)

I did debug it with the pak128-Britain start game and did some small tests with other paksets.

Edit: removed patch, updated version below.

[prissi]

For really bad clipping, pak128.japan is the test ... I will patch today and test.

EDIT:
what does display dinge_xyz? (The code could do with a little more comments).

[Dwachs]

display_dinge_all .. is the main routine in grund_t

_bg / _vh / _fg .. draw background images of non moving things / vehicles / forground images. I had to separate the one routine display_dinge.

[prissi]

Tested a little: Very minor error only in curves directly before covered stations on elevated rails. But neglible.

Even the monorail of clipping errors in pak128.japan works flawlessly. And the performance hit is not large it seems. Most likely because there are never so many vehicles on the

[Dwachs]

Tested a little: Very minor error only in curves directly before covered stations on elevated rails. But neglible.

Do you have a screenshot or savegame for this?

[prissi]

I have. I think this is due to the vehicle in question to much forward. I will save it at home in the evening.

[Dwachs]

Small update: fix clipping errors for airplanes by introducing some special checks. The airplane drawing routine are a dark chapter in simutrans code

[prissi]

Here the two attachments. It is a station on a bridge directly behing a curve.

[Dwachs]

I have to test this again. Does this happen only with specific paks or is this a generic error?

Edit: Can you upload the game / pak? Which pak-set is this?

I can check for the train. The station and the bridge are standard pak128 or download from japanese if not. And elevated way and/or this bridge should produce it, as the the vehicles are statndard of size 8. But this is about the only glitch I found, the rest is working great!

[prissi]

This is a game with 8000+ busses and some train of a special pak128.101. It has tons of addons and cannot be run in debug mode, since it then runs with 0.1 simloops/s and 3fps. (Aktually a real map of Hongkong.)

It is even too large for Franks server.

[Dwachs]

Which train causes the error? Can you upload the pak of the train?

[prissi]

I think any train on that combination (starting tile of briges with station.) I will try to build something wiht pak 128 vanilla. Ok, bridges do not work. Before tile seems to be a elevated way. I will try to make something like this.

The elevated way in front confuses simutrans ... here is a pak64 savegame

[Dwachs]

Ok this reproduces the bug, will investigate.

Edit: the bridge and monorail is not necessary, simply the track configuration on the ground has the same issue

Edit2: found the issue, this is fixed . Hopefully it did not break anything.

[prissi]

Without the monorail, it worked fine for me ... I'll test the new version.

[Dwachs]

Updated version: now also ships are drawn right unless they are really huge.

It introduces a new ribi member variableto wasser_t.

[VS]

Out of curiosity, what is "really huge" ?

[prissi]

Ok, I need to do more profiling with this. Any thing that makes water ribis slower is impacting ship wayfinding considerably. It might be neglible, but I would like to verify this.

[Dwachs]

Water ribis should be faster, since they are now more accurate.

@VS: basically ships with length up to 32 (two tiles) should work more or less, there are some issues with foundations though.

Patch updated again.

[prissi]

Well before on a 1024*1024 island map nearly all tiles must be checked for a route search of the two harbours are in bays behind larger islands. This could take up to 5 seconds with the optimized routines. Since in your case some ifs are added, it might become slower. I am not pessimistic, but I want to check for this.

[Dwachs]

Actually, no if's are added, only the result of the 'then' branch is different.

[Dwachs]

Updated again. The large ships in pak128.Britain (slightly more than 2 tiles long) cause now only very marginal errors.

[prissi]

How about patching trunk and wait for error reports?

[Dwachs]

why not 

One question though: Should there be an option to toggle between old and new? In case the performance with the new version is much worse.

Did you some profiling? (I did not profile the patch)

[Dwachs]

some quick profiling: karte_ansicht_t::display needs 0.016 s (clipping pathc) and 0.013 (trunk), which means a small performance loss.

trunk:

Code Select Expand


Each sample counts as 0.01 seconds.
 %   cumulative   self              self     total          
time   seconds   seconds    calls   s/call   s/call  name    
50.61      7.82     7.82   726481     0.00     0.00  display_img_nc(short, short, short, unsigned short const*)
 4.53      8.52     0.70  3295327     0.00     0.00  pixcopy(unsigned short*, unsigned short const*, unsigned int)
 2.01      8.83     0.31    49461     0.00     0.00  display_img_wc(short, short, short, unsigned short const*)
 1.78      9.11     0.28    14008     0.00     0.00  karte_t::sync_step(long, bool, bool)
 1.75      9.38     0.27 47806470     0.00     0.00  decode_uint16(char*&)
 1.68      9.63     0.26 47880223     0.00     0.00  endian_uint16(unsigned short const*)
 1.10      9.80     0.17    20632     0.00     0.00  image_reader_t::read_node(_IO_FILE*, obj_node_info_t&)
 1.10      9.97     0.17    14307     0.00     0.00  register_image(bild_t*)
 1.04     10.13     0.16  6095874     0.00     0.00  vehikel_basis_t::fahre_basis(unsigned int)
 1.04     10.29     0.16      730     0.00     0.01  karte_ansicht_t::display(bool)

clipping patch:

Code Select Expand


Each sample counts as 0.01 seconds.
 %   cumulative   self              self     total          
time   seconds   seconds    calls   s/call   s/call  name    
40.44      7.04     7.04   730804     0.00     0.00  display_img_nc(short, short, short, unsigned short const*)
11.75      9.09     2.04 12947024     0.00     0.00  pixcopy(unsigned short*, unsigned short const*, unsigned int)
 4.65      9.89     0.81   167260     0.00     0.00  display_img_pc(short, short, short, unsigned short const*)
 2.07     10.26     0.36  8792448     0.00     0.00  get_xrange_and_step_y(int&, int&)
 1.95     10.60     0.34  9291401     0.00     0.00  clip_line_t::inc_y(xrange&) const
 1.72     10.89     0.30    50420     0.00     0.00  display_img_wc(short, short, short, unsigned short const*)
 1.55     11.16     0.27      803     0.00     0.02  karte_ansicht_t::display(bool)
 1.38     11.40     0.24 47806472     0.00     0.00  decode_uint16(char*&)
 1.32     11.63     0.23    14081     0.00     0.00  karte_t::sync_step(long, bool, bool)
 1.09     11.82     0.19  6081706     0.00     0.00  vehikel_basis_t::fahre_basis(unsigned int)
 1.09     12.02     0.19  1971601     0.00     0.00  display_img_aux(unsigned int, short, short, signed char, int, int)

profiling results are here:  http://simutrans-germany.com/files/upload/prof-results.zip

profiled with: -objects pak128.Britain/ -screensize 1920x1200 -fullscreen -load test -until 23309

savegame here: http://simutrans-germany.com/files/upload/test.sve

[prissi]

I did also some worst case pathfinding stuff: Ships need need 15% long for pathfinding. Considering the gain, I am all for incoporation. Maybe you should wrote a little longer explanation on how this works as a real paragraph for anyone to find out about this later. Because in grund.cc really not much comments are added. Or are the elsewhere?

[Dwachs]

Update: tried some low-level optimization, added comments in grund.*

Compiling for the 8-bit version needs to be fixed.

Edit: the optimization introduced some bugs. The clipping stuff in simgraph16.cc needs to be commented properly.

[prissi]

I found no obvious bug. Maybe it is time for comitting ...

[Dwachs]

ok I will do that tonight.

But before I want to do commenting and optimizing in simgraph16.cc. The introduced routines are high up in the profiling table. As they are implemented now they are way too general, allowing arbitrary clipping lines, while only very specific clipping lines are used actually (along flat tile edges, so dx=+-2 dy=+-1).

[prissi]

It is your great work ... In my test the slow down was not really noticeable, even though the screen were crowded with traffic jams. I could live with a submit first and optimizing later. (But this may be due to my relatively small screen for the test game.)

[Dwachs]

The great moment has come: incorporated in revision 3151.

Awaiting screenshots & bug reports & praises  .... in this order

[Amelek]

There is a minor problem with power lines - it seem that they are drawn first (before overhead and trains)

[Dwachs]

should be fixed in rev 2153.

[prissi]

Would it not make more sense to add front and back images for ways too? This would able ways with automatic front walls (would not be too difficult and would enable new possibilities like "Lärmschutzwände" or side catenaries within city limits) This would keep dingliste without exceptions, which I find much more appealing.

[Dwachs]

Would it not make more sense to add front and back images for ways too? This would able ways with automatic front walls (would not be too difficult and would enable new possibilities like "Lärmschutzwände" or side catenaries within city limits) This would keep dingliste without exceptions, which I find much more appealing.

Front images of ways would make sense, yes.

But I do not see where this would simplify anything.

The exceptional cases in dingliste are (a) airplanes and (b) powerlines, nothing related to ways.

In grund_t::display_boden and grund_t::display_dinge_all there are checks related to ways. These checks are for ways whose images go over the tile borders.

[prissi]

The powerlines could as well automatically return a valid front image:

Code Select Expand


Index: leitung2.cc
===================================================================
--- leitung2.cc (revision 3153)
+++ leitung2.cc (working copy)
@@ -299,6 +296,7 @@
void leitung_t::recalc_bild()
{
const koord pos = get_pos().get_2d();
+ is_crossing = false;

grund_t *gr = welt->lookup(get_pos());
if(gr==NULL) {
@@ -325,6 +323,7 @@
else {
set_bild( wegbauer_t::leitung_besch->get_diagonal_bild_nr(ribi_t::sued|ribi_t::ost,0));
}
+ is_crossing = true;
}
else {
if(ribi_t::ist_gerade(ribi)  &&  !ribi_t::ist_einfach(ribi)  &&  (pos.x+pos.y)&1) {
Index: leitung2.h
===================================================================
--- leitung2.h (revision 3153)
+++ leitung2.h (working copy)
@@ -23,6 +23,8 @@
protected:
image_id bild;

+ bool is_crossing:1;
+
// direction of the next pylon
ribi_t::ribi ribi;

@@ -80,7 +82,9 @@
ribi_t::ribi get_ribi(void) const { return ribi; }

inline void set_bild( image_id b ) { bild = b; }
- image_id get_bild() const {return bild;}
+ image_id get_bild() const { return is_crossing ? IMG_LEER : bild; }
+
+ image_id get_after_bild() const { return is_crossing ? bild : IMG_LEER; }

/**
* Recalculates the images of all neighbouring


The additional way checks in grund_t dates back from the time when ways were not part of dingliste. That is the main reason, whey they have no foreground, since they were not derived from ding_t back then.

The old system of two different types of ground is obsolete with your system, imho. Thus the two different ways of ground drawing could go I think, and also the drawing ways as part of ground tiles.