Skip to content

Conversation

mramato
Copy link
Contributor

@mramato mramato commented Jul 1, 2015

Merge #2858 first.

Apparently the IE 11 test failures we've been seeing for months were actually a bug in Cesium where we try to destroy scene._globeDepth even when it was undefined. Fixing that allowed IE11 to once again run tests to compeltion without thousands of errors. This then revealed several other spec-only issues that needed to be fixed as well.

This brings the overall number of failures down to 5, all of which look rendering related. I'll write up a separate issue for them.

Apparently the IE 11 test failures we've been seeing for months were
actually a bug in Cesium where we try to destroy `scene._globeDepth`
even when it was undefined. Fixing that allowed IE11 to once again
run tests to compeltion without thousands of errors. This then revealed
several other spec-only issues that needed to be fixed as well.

This brings the overall number of failures down to 5, all of which look
rendering related.  I'll write up a separate issue for them.
@mramato mramato mentioned this pull request Jul 1, 2015
@mramato
Copy link
Contributor Author

mramato commented Jul 1, 2015

Apparently IE will still start randomly failing to create WebGL contexts after repeated runs, but a clean run right after startup seems to pass all but the failures in the linked bug.

I merged in master so this is ready for review.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about this change? context.depthTexture should return a boolean and _globeDepth should be defined if context.depthTexture is true.

See the Scene constructor and implementation for context.depthTexture.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are correct, I was thrown off by the name it should probably be something like hasDepthTexture. At one point I thought the _globeDepth check was needed, but I might have been thrown off by the check on line 1839. I removed it and it looks like everything still passes.

@mramato
Copy link
Contributor Author

mramato commented Jul 1, 2015

Both comments have been addressed.

pjcozzi added a commit that referenced this pull request Jul 1, 2015
@pjcozzi pjcozzi merged commit 15c7a99 into master Jul 1, 2015
@pjcozzi pjcozzi deleted the fix-ie11-tests branch July 1, 2015 16:56
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