[bugfix] [MERGED] Faulty property map boundary checks

Llarlen

22-11-2009 13:44:02

Using PagedGeometry, we had the problem that when using density maps for grass, grass would be drawn outside of the map boundary in psychedelic colours. It turns out that the problem lies in the boundary checks in DensityMap::_getDensityAt_Unfiltered, DensityMap::_getDensityAt_Bilinear, ColorMap::_getColorAt and ColorMap::_getColorAt_Bilinear. The indices are calculated correctly, but casting a negative float to an unsigned will result in 0, which is never smaller than 0 and thus passes the lower boundary check.

To solve the issue, the following patch can be used. It also fixes the default return value of _getColorAt_Bilinear to be the same as the default of _getColorAt:
Index: PagedGeometry/source/PropertyMaps.cpp
===================================================================
--- PagedGeometry/source/PropertyMaps.cpp
+++ PagedGeometry/source/PropertyMaps.cpp
@@ -126,6 +126,12 @@
{
assert(pixels);

+ // Early out if the coordinates are outside map bounds.
+ if(x < mapBounds.left || x >= mapBounds.right || z < mapBounds.top || z >= mapBounds.bottom)
+ {
+ return 0.0f;
+ }
+
uint32 mapWidth = (uint32)pixels->getWidth();
uint32 mapHeight = (uint32)pixels->getHeight();
float boundsWidth = mapBounds.width();
@@ -137,8 +143,6 @@

uint32 xindex = mapWidth * (x - mapBounds.left) / boundsWidth;
uint32 zindex = mapHeight * (z - mapBounds.top) / boundsHeight;
- if (xindex < 0 || zindex < 0 || xindex >= mapWidth || zindex >= mapHeight)
- return 0.0f;

uint8 *data = (uint8*)pixels->data;
float val = data[mapWidth * zindex + xindex] / 255.0f;
@@ -152,6 +156,12 @@
{
assert(pixels);

+ // Early out if the coordinates are outside map bounds.
+ if(x < mapBounds.left || x >= mapBounds.right || z < mapBounds.top || z >= mapBounds.bottom)
+ {
+ return 0.0f;
+ }
+
uint32 mapWidth = (uint32)pixels->getWidth();
uint32 mapHeight = (uint32)pixels->getHeight();
float boundsWidth = mapBounds.width();
@@ -314,6 +324,12 @@
{
assert(pixels);

+ // Early out if the coordinates are outside map bounds.
+ if(x < mapBounds.left || x >= mapBounds.right || z < mapBounds.top || z >= mapBounds.bottom)
+ {
+ return 0xFFFFFFFF;
+ }
+
uint32 mapWidth = (uint32)pixels->getWidth();
uint32 mapHeight = (uint32)pixels->getHeight();
float boundsWidth = mapBounds.width();
@@ -321,8 +337,6 @@

uint32 xindex = mapWidth * (x - mapBounds.left) / boundsWidth;
uint32 zindex = mapHeight * (z - mapBounds.top) / boundsHeight;
- if (xindex < 0 || zindex < 0 || xindex >= mapWidth || zindex >= mapHeight)
- return 0xFFFFFFFF;

uint32 *data = (uint32*)pixels->data;
return data[mapWidth * zindex + xindex];
@@ -356,6 +370,12 @@
{
assert(pixels);

+ // Early out if the coordinates are outside map bounds.
+ if(x < mapBounds.left || x >= mapBounds.right || z < mapBounds.top || z >= mapBounds.bottom)
+ {
+ return 0xFFFFFFFF;
+ }
+
uint32 mapWidth = (uint32)pixels->getWidth();
uint32 mapHeight = (uint32)pixels->getHeight();
float boundsWidth = mapBounds.width();
@@ -367,7 +387,7 @@
uint32 xIndex = xIndexFloat;
uint32 zIndex = zIndexFloat;
if (xIndex < 0 || zIndex < 0 || xIndex >= mapWidth-1 || zIndex >= mapHeight-1)
- return 0.0f;
+ return 0xFFFFFFFF;

float xRatio = xIndexFloat - xIndex;
float xRatioInv = 1 - xRatio;

Llarlen

22-11-2009 14:56:35

Performance would obviously be better if pages outside of map boundaries wouldn't even try to place grass, so add the following code in GrassLoader.cpp in function GrassLoader::loadPage at line 128:
//Generate meshes
std::list<GrassLayer*>::iterator it;
for (it = layerList.begin(); it != layerList.end(); ++it){
GrassLayer *layer = *it;

+ // Continue to the next layer if the current page is outside of the layers map boundaries.
+ if(layer->mapBounds.right < page.bounds.left || layer->mapBounds.left > page.bounds.right ||
+ layer->mapBounds.bottom < page.bounds.top || layer->mapBounds.top > page.bounds.bottom)
+ {
+ continue;
+ }

Fish

22-11-2009 23:43:27

I took the liberty of merging the above two patches into a single diff and added a fix for the occasional white grass at the edge of the color map:

[attachment=0]screenshot_11.jpg[/attachment]

[attachment=1]DensityMapFix.zip[/attachment]

Also submitted to the tracker.

-Fish

tdev

27-03-2010 14:21:02

thanks for the nice bugfix, just merged it with the SVN :)