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

Specs refactor (final) #3803

Merged
merged 11 commits into from
Apr 4, 2016
Merged

Specs refactor (final) #3803

merged 11 commits into from
Apr 4, 2016

Conversation

lasalvavida
Copy link
Contributor

Final cleanup for #1259

As of this revision, none of the scene specs contain references to createContext or createFrameState.

@lasalvavida lasalvavida changed the title Specs refactor Specs refactor (final) Apr 4, 2016
sun.destroy();
});

it('does not render without a render pass', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm changing my mind here, but I think it's actually okay to include this test, but calling sun.update(scene.frameState) instead of scene.renderForSpecs()

@lilleyse
Copy link
Contributor

lilleyse commented Apr 4, 2016

While looking for occurences of [0, 0, 0, 0] I noticed one case here: https://github.com/AnalyticalGraphicsInc/cesium/blob/master/Specs/Scene/PolylineCollectionSpec.js#L799

The test fails if I change it to [0, 0, 0, 255]. Can you also look into that quickly?

@lasalvavida
Copy link
Contributor Author

@lilleyse Updated with suggested changes

viewSun(scene.camera, scene.context.uniformState);
scene.frameState.passes.render = false;
scene.sun.update(scene);
expect(scene.context.readPixels()).toEqual(backgroundColor);
Copy link
Contributor

Choose a reason for hiding this comment

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

var command = scene.sun.update(scene.frameState);
expect(command).not.toBeDefined();

Is fine here.

@lasalvavida
Copy link
Contributor Author

Updated

@lilleyse
Copy link
Contributor

lilleyse commented Apr 4, 2016

Looks good!

@lilleyse lilleyse merged commit 7aa18b1 into CesiumGS:master Apr 4, 2016
@lilleyse lilleyse deleted the specs-refactor branch April 4, 2016 20:23
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