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 - Fix IE test failures #3973

Merged
merged 4 commits into from
May 28, 2016
Merged

Shadows - Fix IE test failures #3973

merged 4 commits into from
May 28, 2016

Conversation

lilleyse
Copy link
Contributor

For #2594

This fixes the tests failures I was seeing in IE. This isn't completely ready because I need to update FeatureDetection to check for Edge and test it on Windows 10.

The problem was rendering any commands into the shadow map with polygon offset enabled was causing the test to completely fail, even if the test wasn't checking shadows.

@mramato
Copy link
Contributor

mramato commented May 27, 2016

According to MDN, polygon offset is supported on IE. Is it just broken? Is there a way we can check if polygonOffset works as opposed to checking for IE? It's always better to check for features rather than browsers.

@pjcozzi
Copy link
Contributor

pjcozzi commented May 28, 2016

Testing if polygon offset works would be painful; it would require doing some rendering tests up front that might not be reliable and then lead to false negatives on other browsers. I would rather leave this as is, and then expand if IE users run into a lot of problems.

@lilleyse
Copy link
Contributor Author

It doesn't pass the conformance test: https://www.khronos.org/registry/webgl/sdk/tests/conformance/rendering/polygon-offset.html?webglVersion=1&quiet=0

The tests do pass on Edge by the way, except like the OSX Chrome issue Edge supports the depth texture extension but fails to create a depth-only fbo. The fallback is already in place in the code, I just have to fix a test to handle that.

@lilleyse
Copy link
Contributor Author

Updated.

@lilleyse lilleyse mentioned this pull request May 28, 2016
18 tasks
@@ -178,8 +180,16 @@ define([
this._outOfViewPrevious = false;
this._needsUpdate = true;

// In IE11 and Edge polygon offset is not functional.
var polygonOffsetSupported = true;
// TODO : check for Edge too
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this TODO still needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I removed it.

@pjcozzi pjcozzi merged commit 511f5d1 into shadows May 28, 2016
@pjcozzi pjcozzi deleted the shadows-ie-tests branch May 28, 2016 14: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.

3 participants