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

Fix depth fail material in Edge #5359

Merged
merged 3 commits into from
May 24, 2017
Merged

Fix depth fail material in Edge #5359

merged 3 commits into from
May 24, 2017

Conversation

bagnell
Copy link
Contributor

@bagnell bagnell commented May 24, 2017

Always include frag depth varying even when not supported. Fixes #5357.

@@ -994,15 +994,11 @@ define([
function depthClampVS(vertexShaderSource) {
var modifiedVS = ShaderSource.replaceMain(vertexShaderSource, 'czm_non_depth_clamp_main');
modifiedVS +=
'#ifdef GL_EXT_frag_depth\n' +
Copy link
Contributor

Choose a reason for hiding this comment

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

As an explicit optimization, we usually put the #ifdef everywhere and do not rely, for example, on the GLSL compiler to optimize out unused code.

Perhaps add a comment here that this is used for frag depth, but it is not in an #ifdef to workaround an Edge bug.

Also submit an Edge bug please.

@mramato
Copy link
Contributor

mramato commented May 24, 2017

I can confirm that this fixes the crash, I'll leave the review and merge up to @pjcozzi

Thanks @bagnell!

@bagnell
Copy link
Contributor Author

bagnell commented May 24, 2017

@bagnell
Copy link
Contributor Author

bagnell commented May 24, 2017

@pjcozzi This is ready for another look.

@pjcozzi
Copy link
Contributor

pjcozzi commented May 24, 2017

Very nice, thanks @bagnell @mramato!

@pjcozzi pjcozzi merged commit 97751cc into master May 24, 2017
@pjcozzi pjcozzi deleted the ie-workaround branch May 24, 2017 19:57
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