News:

SimuTranslator
Make Simutrans speak your language.

[PATCH] inconsistent diagonal images

Started by Philip, September 14, 2014, 08:47:22 PM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

Philip

I think this is purely a cosmetic issue, but I'm not totally sure.

There appears to be an inconsistency in the code that decides whether to display a curve or a diagonal image for roads. The attached screenshot shows two equivalent pieces of road; one of them, built from one end, has two diagonal tiles, the other one has just one. After a load-save cycle, both road pieces use just one diagonal tile.

Looking at the code, it seems to me we can fix things by removing some logic from weg_t::calc_bild(); there will be a slight performance hit, but I think it's just a few extra cycles per road tile. The problem is that extending a dead-end road might change a neighbouring road's diagonal status, even though the dead-end road itself is not a diagonal.

I think this only affects 135-degree turns as in the screenshot, which it doesn't really make sense to build under ordinary circumstances.

prissi

A static variable is not a good idea, because this function can be called threaded, for instance when the map is rotated or a season is changed. Now slightly different visials will happen only at seams between two threads map slices.

Philip

Quote from: prissi on September 15, 2014, 09:30:13 PM
A static variable is not a good idea, because this function can be called threaded, for instance when the map is rotated or a season is changed. Now slightly different visials will happen only at seams between two threads map slices.

I didn't change that part of the code, it just moved down an indentation level. We do hold a mutex (weg_calc_bild_mutex) to avoid the threading problem, though.

I agree it would be better to change this code, and other places in the code that use the "static int recursion" trick, but that's really independent of this issue.

prissi

Ok, sorry for being thick. This poses next question: Why call this on ways which cannot have diagonal images (besch->has_diagonal_bild()==false) This would be only wasting time. Maybe I overlooked again something?

Philip

Quote from: prissi on September 15, 2014, 10:20:42 PM
Ok, sorry for being thick. This poses next question: Why call this on ways which cannot have diagonal images (besch->has_diagonal_bild()==false) This would be only wasting time. Maybe I overlooked again something?

I think it's possible for a way that doesn't have a diagonal image to be connected to another way (same waytype, different besch) that does have a diagonal image, so we need to recalculate the other way's image as well. However, I think most way beschs for the standard types do have diagonal images always, so even if it were correct to check for has_diagonal_bild() first, the performance improvement would be absolutely minimal.

Thank you for taking the time to comment!

prissi

That really depends on pak set. pak64 has no diagonal city roads. Same for channels.

I think not calling calc_bild if there is no diagonal should take care of that. Submitted in r7312. Thank you.