Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

#define out water effect code for tiles that are entirely land. #2261

Merged
merged 2 commits into from
Nov 7, 2014

Conversation

shunter
Copy link
Contributor

@shunter shunter commented Nov 6, 2014

SHOW_REFLECTIVE_OCEAN and SHOW_OCEAN_WAVES have moved into the GlobeSurfaceShaderSet so they can vary on a per-tile basis, rather than being set identically for all tiles. Tiles that are entirely land now use undefined for their water mask texture instead of a shared "land" texture.

This is a roadmap item in #526.

`SHOW_REFLECTIVE_OCEAN` and `SHOW_OCEAN_WAVES` have moved into the `GlobeSurfaceShaderSet` so they can vary on a per-tile basis, rather than being set identically for all tiles.  Tiles that are entirely land now use `undefined` for their water mask texture instead of a shared "land" texture.
@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

Very nice. Any before/after performance numbers?

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

Assuming the performance win is non-trivial, we should mention this in CHANGES.md.

@shunter
Copy link
Contributor Author

shunter commented Nov 6, 2014

On my 9800GT, GPU usage, measured with Process Explorer, for a horizon view facing North at Seneca Rocks drops from around 60% to 50%.

@shunter
Copy link
Contributor Author

shunter commented Nov 6, 2014

I should add, that's when using the heightmap terrain with water masks, of course.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

What about top-down view all land vs. all water before and after?

var sampler = context.createSampler({
wrapS : TextureWrap.CLAMP_TO_EDGE,
wrapT : TextureWrap.CLAMP_TO_EDGE,
minificationFilter : TextureMinificationFilter.LINEAR,
Copy link
Contributor

Choose a reason for hiding this comment

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

I hope the driver would optimize for this 1x1 case, but I would still go with NEAREST here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this sampler is only used for the non-1x1 tiles. The allWaterTexture doesn't specify a sampler.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, OK.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

@kring do you want to look at this?

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

Tests and terrain/water rendering are good on Mac in Chrome and Firefox.

@shunter
Copy link
Contributor Author

shunter commented Nov 6, 2014

top-down view, all land, ~47% before, ~36% after. top-down view, all water, North Pole, same ~60% before and after.

@pjcozzi
Copy link
Contributor

pjcozzi commented Nov 6, 2014

Very nice.

@kring
Copy link
Member

kring commented Nov 7, 2014

All looks good to me, thanks Scott!

I don't see any comments that need addressing, so I'm merging.

kring added a commit that referenced this pull request Nov 7, 2014
`#define` out water effect code for tiles that are entirely land.
@kring kring merged commit aa68e37 into master Nov 7, 2014
@kring kring deleted the optimizeLandTiles branch November 7, 2014 01:18
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.

3 participants