-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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 OIT on machines where it was broken. #2441
Conversation
As pointed out by one of the Chromium developers, we were using an undefined variable in `czm_alphaWeight` due to a double declaration of the `z` variable resuling in scoping issues. This change then lead to another issue that where textured were black, which was fixed by flipping `q.w` We also discovered that #2327 is related to `czm_alphaWeight` and may be an issue with the weighting function. @bagnell is going to look into it separately.
Here's the Chrome issue: https://code.google.com/p/chromium/issues/detail?id=414280 Fixes #2129 |
I will test this on Mac / Intel tomorrow. |
Nice! This fixes the issue in Ubuntu 14.04 on a Dell XPS 14 for me. The "Geometry and Appearances" Sandcastle example shows pixel flickering in master but not in the fix-oit branch. |
It also fixes the issue on my machine. Thank you guys. |
Code looks good. Works on my my MacBook Pro forced to Intel. @bagnell can you please verify this in scenes with high depth complexity like we did before http://cesiumjs.org/2014/03/14/Weighted-Blended-Order-Independent-Transparency/ @bagnell can you also followup in the Chrome issue to confirm that this is not a GLSL compiler issue that they need to test for (assuming that is the case)? When we merge this, please post this to the forum and also followup on the few threads where this was discussed. CC @kenrussell |
Conflicts: CHANGES.md
I also tested this on OS X with an Radeon HD6630M and Linux Mint with an Intel HD 4000. Both showed the problem before and both are now fixed. |
Please do take a little time and see if you can reduce this into a small test case. It looks like there may be bugs in many GLSL compilers related to shadowing of variables, and it's critically important that we expose these issues so browsers can provide uniform behavior, and so that the underlying bugs may eventually be fixed. Thanks. |
Conflicts: CHANGES.md
The weighting function still needs to be investigated, but this fixes the issue and still matches master so I'm merging. |
Fix OIT on machines where it was broken.
@kenrussell we'll see if we can put together a trimmed down test to see if this was 100% our issue or a potential GLSL shadowing issue. |
As pointed out by one of the Chromium developers, we were using an undefined variable in
czm_alphaWeight
due to a double declaration of thez
variable resulting in scoping issues. This change then lead to another issue where textures were too dark, which was fixed by flippingq.w
We also discovered that #2327 is related to
czm_alphaWeight
and may be an issue with the weighting function. @bagnell is going to look into it separately.This makes OIT work everywhere and disabling it should no longer be necessary.