OpenTTD

Tasklist

FS#6618 - Incorrect size bounds checks in vehicle viewport hash scan in ViewportAddVehicles

Attached to Project: OpenTTD
Opened by J G Rennison (JGR) - Thursday, 31 August 2017, 20:17 GMT
Type Bug
Category Core
Status New
Assigned To No-one
Operating System All
Severity Very Low
Priority Normal
Reported Version trunk
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

In ViewportAddVehicles in src/vehicle.cpp there are bounds checks:

if (dpi->width + (70 * ZOOM_LVL_BASE) < (1 << (7 + 6 + ZOOM_LVL_SHIFT))) {
if (dpi->height + (70 * ZOOM_LVL_BASE) < (1 << (6 + 6 + ZOOM_LVL_SHIFT))) {

These are to prevent the extent of the untruncated span from xl to xu and yl to yu from being unable to fit in the hash bit allocation (6 bits each).

In the case of width, in the case where the lower 9 bits of (l - (70 * ZOOM_LVL_BASE)) are greater than the lower 9 bits of r,
dpi->width + (70 * ZOOM_LVL_BASE) can be less than (1 << (7 + 6 + ZOOM_LVL_SHIFT)) even when the the untruncated hash values differ by 0x40, such that the truncated values are the same.
This has the effect that if the viewport re-draw area is the right size and offset, the iteration over the vehicle viewport hash only finds vehicles at the two edges but not in the middle.
This can visually manifest as flickering effects.

To give a concrete example:
Noting that: ZOOM_LVL_BASE = 4, ZOOM_LVL_SHIFT = 2
If: dpi->left = 0x317, dpi->width = 0x7CE9
Then:
dpi->width + (70 * ZOOM_LVL_BASE) = 0x7E01
(1 << (7 + 6 + ZOOM_LVL_SHIFT)) = 0x8000
dpi->width + (70 * ZOOM_LVL_BASE) < (1 << (7 + 6 + ZOOM_LVL_SHIFT)) evaluates to true
l - (70 * ZOOM_LVL_BASE) = 0x1FF, r = 0x8000
xl = 0, xu = 0
Consequently: only vehicles in hash row 0 will be drawn, even though the viewport redraw covers the whole width of the viewport hash

A possible solution for width/x would be to perform the bounds check after doing the shift-right by 9, but before truncating to 6 bits.

Height/y is the same except the shift is by 8 instead of 9.
This task depends upon

Loading...