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

Make render/pick tests use the scene #1259

Closed
pjcozzi opened this issue Oct 22, 2013 · 13 comments
Closed

Make render/pick tests use the scene #1259

pjcozzi opened this issue Oct 22, 2013 · 13 comments
Assignees
Labels
cleanup good first issue An opportunity for first time contributors

Comments

@pjcozzi
Copy link
Contributor

pjcozzi commented Oct 22, 2013

Most old low-level primitive rendering tests that use render directly can be updated to use the real Cesium Scene, like DebugModelMatrixPrimitiveSpec does, which makes the tests more realistic and more concise.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Feb 5, 2014

When the glTF branch is merged, they can also use renderForSpecs introduced in 4c28451.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Apr 29, 2015

Billboards/Labels were updated in #2669

@pjcozzi pjcozzi mentioned this issue Jul 29, 2015
2 tasks
@pjcozzi
Copy link
Contributor Author

pjcozzi commented Aug 26, 2015

@lilleyse after #817, we'll start chipping away at this (not all at once, we'll split it into several pull requests). To see the files that need to be updated, search in all files for Specs/render. An easy file to start with is probably ViewportQuadSpec.js. Look at DebugModelMatrixPrimitiveSpec.js for a simple example.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Aug 31, 2015

@lilleyse let's start with ViewportQuadSpec and GroundPrimitiveSpec today.

  • DebugAppearanceSpec
  • EllipsoidPrimitiveSpec
  • EllipsoidSurfaceAppearanceSpec
  • GeometryRenderingSpec
  • GlobeSpec
  • GlobeSurfaceTileProviderSpec
  • GroundPrimitiveSpec // Just remove the reference; it is not used
  • MaterialAppearanceSpec
  • MaterialSpec
  • PerInstanceColorAppearanceSpec
  • PolygonSpec
  • PolylineCollectionSpec
  • PolylineColorAppearanceSpec
  • PolylineMaterialAppearanceSpec
  • PrimitiveCollectionSpec
  • PrimitiveCullingSpec
  • PrimitiveSpec
  • RectanglePrimitiveSpec
  • SceneSpec // Not sure we'll do this one
  • ViewportQuadSpec

@lilleyse
Copy link
Contributor

Should all these pull requests go into their own branch, like renderer-refactor in #640 ?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Aug 31, 2015

Yes, one branch per pull request. The branch may have one or more refactored specs.

@lilleyse
Copy link
Contributor

I mean should the master repo have a branch called spec-refactor, like how I made my renderer pull requests go into renderer-refactor?

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Aug 31, 2015

Ah, sorry. No, it is fine to open the pull request directly into master.

@lilleyse
Copy link
Contributor

lilleyse commented Sep 3, 2015

GlobeSpec was updated in 8f20940.

@pjcozzi
Copy link
Contributor Author

pjcozzi commented Sep 17, 2015

@lilleyse can you also update the new tests in PrimitiveSpec.js in the 3d-tiles branch?

[runJsHint] Specs/Scene/PrimitiveSpec.js: line 402, col 9, 'frameState' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 403, col 9, 'frameState' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 404, col 9, 'us' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 404, col 28, 'frameState' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 406, col 9, 'ClearCommand' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 409, col 9, 'render' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 409, col 25, 'frameState' is not defined.
[runJsHint] Specs/Scene/PrimitiveSpec.js: line 439, col 39, 'frameState' is not defined.

This was referenced Mar 29, 2016
@pjcozzi
Copy link
Contributor Author

pjcozzi commented Apr 4, 2016

@lilleyse can we close this? If not, what is left? I took a quick look at SceneSpec and I'm not sure that we need to make any changes there. Do we still need to update tests to use renderForSpecs?

@lilleyse
Copy link
Contributor

lilleyse commented Apr 4, 2016

@lasalvavida is working on a final cleanup PR for any files not included on this list, so until that is ready we can leave this open.

@lilleyse
Copy link
Contributor

lilleyse commented Apr 4, 2016

Thanks for the great work on this @lasalvavida.

@lilleyse lilleyse closed this as completed Apr 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup good first issue An opportunity for first time contributors
Projects
None yet
Development

No branches or pull requests

3 participants