The place to talk about Transport Tycoon
https://www.tt-forums.net/
could you test that? it might be that you do something different than me. also, there is a parallel universe where DBSetXL 0.9 exists and uses a similar method of splitting vehicles into 3.Timberwolf wrote: 02 Jan 2021 21:45I don't think anything else is affected with the possible exception of some of the old CETS test releases.
Just tested - CETS r822 shows the same behaviour. Does it use a 1x1 transparent sprite when the front/rear sections aren't shown? I think this is only an issue when bounding boxes change in a different tick to the one in which the vehicle changes track direction.Eddi wrote: 02 Jan 2021 22:41could you test that? it might be that you do something different than me. also, there is a parallel universe where DBSetXL 0.9 exists and uses a similar method of splitting vehicles into 3.Timberwolf wrote: 02 Jan 2021 21:45I don't think anything else is affected with the possible exception of some of the old CETS test releases.
yeahTimberwolf wrote: 02 Jan 2021 23:14Does it use a 1x1 transparent sprite when the front/rear sections aren't shown?
Code: Select all
spriteset(inv_dummy_set, "src/gfx/dummy.png") {
[ 0, 0, 1, 1, 0, 0]
[ 0, 0, 1, 1, 0, 0]
[ 0, 0, 1, 1, 0, 0]
[ 0, 0, 1, 1, 0, 0]
}
It works!JGR wrote: 02 Jan 2021 20:15 Hmm, I think I'd have to spend some time resolving it myself, it is not as straightforward as I had presumed.
On the other hand, michi_cc's PR might remove the problem instead https://github.com/OpenTTD/OpenTTD/pull/8479, that will also require some resolving from me to integrate.
In the meantime, I've added a one-liner to my branch which should hopefully allow you to make progress.
Thank you very much.I think that this is a valid use case, just a bit untraditional, "Closed: Timberwolf is a bad person and should not do the unnatural things his train set attempts to inflict upon the world." would seem wrong to me.Timberwolf wrote: 02 Jan 2021 21:45 I've found a tiny bug in the optimised sprite rendering... mainly because I've been looking at similar things in regular OpenTTD and encountered the same problem myself.
The current code assumes that all sprites for a given direction have the same bounding box. Which is usually valid, but some sets get particularly abusive, for example Timberwolf's Trains which has a tendency to swap between 1x1 "invisible" sprites and larger vehicle sections depending on how it feels like rendering long vehicles at that point. This interacts unfortunately with the optimised renderer, which takes the bounding box for a sprite the first time a vehicle enters a new direction. On Timberwolf's Trains, this is still using one of the intermediate turn sprites and thus the front and rear sections of the vehicle have their 1x1 "invisible" sprites in play. This means JGRPP will then cache this size for determining whether a vehicle is in a viewport, and you get a slightly jumpy effect on straight tracks once Trains has swapped to its alternate scheme of splitting the vehicle into three sprites.
When this strikes, you get this happening at the very edge of the viewport:
I noted you're not using this optimisation for ships/aircraft, where this kind of internal mucking about with sprite geometry is more common. At any rate, it's a tiny thing you have to be looking hard to notice, and the correct response may indeed be "Closed: Timberwolf is a bad person and should not do the unnatural things his train set attempts to inflict upon the world."
(If I figure out a fix which still keeps the massive performance improvement from sprite direction caching I'll let you know. And if it's a massive, massive pain I might look at making my train set use more consistent bounding boxes as I don't think anything else is affected with the possible exception of some of the old CETS test releases. Edit: https://github.com/OpenTTD/OpenTTD/pull/8485 is my variant, which reduces the number of cases in which this happens by considering vehicles to be "viewport candidates" and reverting to checking every tick. There may be a more efficient and reliable approach than this still out there.)
The savegame seems fine here, what problem are you seeing?
I've added a fix to my branch which seems to resolve this, and the performance hit is only modest (and more importantly it's basically 0 for normal vehicles).Timberwolf wrote: 02 Jan 2021 21:45 (If I figure out a fix which still keeps the massive performance improvement from sprite direction caching I'll let you know. And if it's a massive, massive pain I might look at making my train set use more consistent bounding boxes as I don't think anything else is affected with the possible exception of some of the old CETS test releases. Edit: https://github.com/OpenTTD/OpenTTD/pull/8485 is my variant, which reduces the number of cases in which this happens by considering vehicles to be "viewport candidates" and reverting to checking every tick. There may be a more efficient and reliable approach than this still out there.)
dunno, before actually looking at your code, this sounds like a giant hack that's going to haunt us in 5 years.JGR wrote: 03 Jan 2021 16:18 If curvature variables are accessed when getting the image, cache the directions of the 4 nearest neighbour vehicles and check those along with the vehicle's own direction when deciding whether to refresh the image (and bounds) in UpdateViewport.
I'm not going to PR it upstream, so you needn't worry about it.Eddi wrote: 04 Jan 2021 13:04dunno, before actually looking at your code, this sounds like a giant hack that's going to haunt us in 5 years.JGR wrote: 03 Jan 2021 16:18 If curvature variables are accessed when getting the image, cache the directions of the 4 nearest neighbour vehicles and check those along with the vehicle's own direction when deciding whether to refresh the image (and bounds) in UpdateViewport.
that doesn't sound very reassuring
These optimisations are invisible to NewGRFs, so there's no risk of anything being broken. Flicker and poor performance are permanent issues which exist in varying amounts with or without these optimisations.
Vehicle sizing (prop 21) is just about how long vehicles are in terms of game simulation (movement, vehicle spacing, etc.), it affects the positioning of the centre-point of the sprite but does not otherwise affect drawing.michael blunck wrote: 04 Jan 2021 13:18 With regards to this optimisatuon, I still don't get the implications on vehicle bounding boxes. Are they, or should they, (more) dependent on sprite dimensions or on vehicle sizes (prop 21)?
regards
Michael
I managed to reproduce the rendering issue on a slope to flat transition (with no curve) with my my recent fix. I'll have to have a bit of a rethink.Timberwolf wrote: 05 Jan 2021 01:03 Now to be very annoying in the context of a fix... at least two of these sets have another situation in which the vehicle bounding box can change (and for long enough vehicles, the same swapping in and out of transparent and combined sprites). And it's not triggered by a curvature change either. On slopes, they also swap to different bounding boxes. Timberwolf's Trains shows this most visibly:
Note if the vehicle is long enough the CETS sprite is also a "combined" sprite at this point, to prevent the vehicle from visibly splitting into 3.
The good news is that the bounding boxes used for these "hill" sprites are similar enough to the intermediate curve sprites you don't see this issue - the vehicle is not split to three at this point. However this behaviour might affect any curvature-based bug fix if you have this situation:
Here the front and rear vehicles 1 and 3 would still be invisible even once all of the directions of the vehicle chain have updated. (I haven't been able to test this against the fixed code, it's only a theory this might present a problem even with the new curvature fix.)
For my own fix I used the idea of a "viewport candidate" - a sprite whose bounding box could potentially overlap the viewport and should receive a fresh `GetImage()` call. I don't think I trust NewGRF authors not to come up with some new and inventive way to break a scheme that is based on people using callbacks in the expected way. But this is not perfect; it can still break if a sprite has extreme differences in offsets, or if we are dealing with a set whose sprites exceed MAX_VEHICLE_PIXEL_X / MAX_VEHICLE_PIXEL_Y - assuming such a thing is possible, I haven't tried to see if OpenTTD will outright prevent you from doing this.
This looks like a crash in the NewGRF debug window, presumably you had it open when doing something to the tile it was referring to.Gadg8eer wrote: 05 Jan 2021 12:10 Had a crash while trying to convert a road vehicle depot after cheating time back to 1900 from 2200. Not sure if I just shouldn't be using time cheats too much but here's the crash report in case you think it should be fixed.
Thank you! Good work!JGR wrote: 05 Jan 2021 12:25This looks like a crash in the NewGRF debug window, presumably you had it open when doing something to the tile it was referring to.Gadg8eer wrote: 05 Jan 2021 12:10 Had a crash while trying to convert a road vehicle depot after cheating time back to 1900 from 2200. Not sure if I just shouldn't be using time cheats too much but here's the crash report in case you think it should be fixed.
I should be able to fix this later.
Edit: should be fixed now.
