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

Shadows Tests #3958

Merged
merged 5 commits into from May 26, 2016
Merged

Shadows Tests #3958

merged 5 commits into from May 26, 2016

Conversation

lilleyse
Copy link
Contributor

For #2594

These are most of the shadows tests unless I or anyone else can think of more. I have the globe test commented out for now but plan on figuring that out soon. After that, I'm not sure of the best way of testing terrain casting on terrain.

I also made some small code changes, a lot of which were related to first-frame type problems. Also Scene is a little smarter about not rendering with shadows if no shadow maps are in view.

@bagnell
Copy link
Contributor

bagnell commented May 25, 2016

There are some jsHint failures:

Source/Scene/Scene.js
line 1753 col 13 'center' is defined but never used.
line 1755 col 13 'radiusSquared' is defined but never used.

⚠ 2 warnings

Specs/Scene/ShadowMapSpec.js
line 238 col 14 'loadGlobe' is defined but never used.

⚠ 1 warning

@bagnell
Copy link
Contributor

bagnell commented May 25, 2016

This looks good to me. I'm also not sure how to test terrain. I have some failing tests on Linux with the Intel HD 4000, but there are a bunch of others that are failing with that graphics card too.

@pjcozzi Can you review and merge?

@pjcozzi
Copy link
Contributor

pjcozzi commented May 26, 2016

JSHint is failing:

Source/Scene/Scene.js
  line 1753  col 13  'center' is defined but never used.
  line 1755  col 13  'radiusSquared' is defined but never used.
  ⚠  2 warnings
Specs/Scene/ShadowMapSpec.js
  line 238  col 14  'loadGlobe' is defined but never used.
  ⚠  1 warning
[22:03:45] 'jsHint' errored after 1.28 min
[22:03:45] Error in plugin 'gulp-jshint'
Message:
    JSHint failed for: Source/Scene/Scene.js, Specs/Scene/ShadowMapSpec.js

See https://travis-ci.org/AnalyticalGraphicsInc/cesium/builds/132971153

@pjcozzi
Copy link
Contributor

pjcozzi commented May 26, 2016

Wow, these tests look really good.

Code coverage is pretty good, but it doesn't look hard to get ShadowMap.js to 100% (ignoring terrain). What do you this?

@pjcozzi
Copy link
Contributor

pjcozzi commented May 26, 2016

After that, I'm not sure of the best way of testing terrain casting on terrain.

Remind me - what is the issue again? Something to do with loading?

We can include sample data, e.g., a really small quantized-mesh tileset, write a mock terrain provider that procedurally generates terrain if needed, and mock things as needed to get reasonably isolated tests.

@lilleyse
Copy link
Contributor Author

By default the loaded tiles are really big so even if the shadow is drawn it is offscreen somewhere (I think). I tried changing the sseDenominator too, but no luck. Your suggestion seems good to me, I'll see how it goes.

@lilleyse
Copy link
Contributor Author

Added the globe tests. This should be ready to go, but we can leave it open for a little while if we come up with new tests.

@lilleyse
Copy link
Contributor Author

Actually it looks like some tests are still failing, I'll fix those now.

@lilleyse
Copy link
Contributor Author

I think something about sharing the same Globe across tests was causing weird things to happen. This is ready now.

@pjcozzi
Copy link
Contributor

pjcozzi commented May 26, 2016

This is all good with me. If you want more tests later, open up another PR.

@pjcozzi pjcozzi merged commit 0e859eb into shadows May 26, 2016
@pjcozzi pjcozzi deleted the shadows-tests branch May 26, 2016 22:28
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.

None yet

3 participants