Skip to content

Prevent super tall pillars from appearing on oceans where there is no elevation data#63

Merged
david-rhodes merged 3 commits intomapbox:developfrom
Spaxe:fix-super-tall-pillars
May 15, 2017
Merged

Prevent super tall pillars from appearing on oceans where there is no elevation data#63
david-rhodes merged 3 commits intomapbox:developfrom
Spaxe:fix-super-tall-pillars

Conversation

@Spaxe
Copy link
Copy Markdown

@Spaxe Spaxe commented May 11, 2017

If @david-rhodes could review this, that would be awesome.

David Rhodes and others added 2 commits May 1, 2017 12:34
commit attempts to decipher the 'no-data' region and return 0 height
as appropriate.
@david-rhodes david-rhodes self-requested a review May 11, 2017 16:27
Assuming this is in meters? The tallest point on earth is ~8,500m.
Therefore this number should be pretty safe.
*/
if (height > 100000 * tile.RelativeScale * heightMultiplier) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will likely work. However, we have determined the missing data is because of bad rendering on our side. Textures with missing data will have width less than 256 (usually 8?). Therefore, it is likely more correct to set heightMultiplier to zero in CreateTerrainHeight():

var texture = new Texture2D(256, 256);
texture.wrapMode = TextureWrapMode.Clamp;
texture.LoadImage(pngRasterTile.Data);
if (texture.width < 256)
{
	heightMultiplier = 0f;
}
tile.HeightData = texture;
tile.HeightDataState = TilePropertyState.Loaded;
GenerateTerrainMesh(tile, heightMultiplier);

@Spaxe
Copy link
Copy Markdown
Author

Spaxe commented May 11, 2017

Thanks @david-rhodes, this method seems to work.

@Spaxe
Copy link
Copy Markdown
Author

Spaxe commented May 15, 2017

When I log into Travis CI, I can't see mapbox/mapbox-unity-sdk, so I don't know what build is failing or how.

(It works on my machine! ;)

@david-rhodes
Copy link
Copy Markdown

@Spaxe Thanks for your patience. Travis is being strange, but I don't see anything in this PR that is risky. Merging.

@david-rhodes david-rhodes merged commit 6e5ef80 into mapbox:develop May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants