The International Simutrans Forum

Development => Patches & Projects => Incorporated Patches and Solved Bug Reports => Topic started by: Philip on September 14, 2014, 08:47:22 PM

Title: [PATCH] inconsistent diagonal images
Post by: Philip on September 14, 2014, 08:47:22 PM
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.
Title: Re: [PATCH] inconsistent diagonal images
Post by: 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.
Title: Re: [PATCH] inconsistent diagonal images
Post by: Philip on September 15, 2014, 09:46:20 PM
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.
Title: Re: [PATCH] inconsistent diagonal images
Post by: 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?
Title: Re: [PATCH] inconsistent diagonal images
Post by: Philip on September 15, 2014, 10:31:59 PM
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!
Title: Re: [PATCH] inconsistent diagonal images
Post by: prissi on September 15, 2014, 10:53:25 PM
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.