907946 - Re-enable EXT_packed_depth_stencil on OS X 10.9

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Guillaume Abadie]

This extension has been blacklisted on OS X 10.9 running on a Nvidia driver. But the crash described in 814839 seams to be fixed, and the WebGL conformance test is passed.

Comment 1

[Guillaume Abadie]

Attachment: patch revision 1

Comment 2

[Guillaume Abadie]

Attachment: patch revision 2

I have forgotten to qrefresh before uploading... =D

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 793723 [details] [diff] [review]
patch revision 2

Review of attachment 793723 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/gl/GLContext.cpp
@@ +830,3 @@
>      if (WorkAroundDriverBugs() &&
> +        Vendor() == gl::GLContext::VendorNVIDIA &&
> +        !nsCocoaFeatures::OnMavericksOrLater())

We should be doing this with our blacklisting system, not hard-coded. We're not trying to use a 10.9 feature, but rather stop ourselves from using apparently broken drivers from 10.8 and earlier.

Comment 4

[Guillaume Abadie]

Oh I see... So, why is that been done this way?

Comment 5

[Benoit Jacob [:bjacob] (mostly away)]

Comment on attachment 793723 [details] [diff] [review]
patch revision 2

Review of attachment 793723 [details] [diff] [review]:
-----------------------------------------------------------------

In theory, I agree with Jeff: it would be nice to have all these things handled by the blacklisting system.

The problem is that we do not currently have in our blocklisting system a Feature for "EXT_packed_depth_stencil", so we would have to add one, that that is very dangerous with our stupid blocklisting system: if a new downloadable blacklist entry were added with that feature, it would get mis-interpreted as "all features" by older Gecko's. That has severely bitten us before.

So for now --- pending the blocklisting system rewrite that has been discussed and even designed but never prioritized --- we will have to do with hardcoded checks as in the present patch.

Jeff: please consider lifting the r-
Guillaume: please do not land as long as there is a r-

Comment 6

[Guillaume Abadie]

For sure! Thanks!

Comment 7

[Guillaume Abadie]

As green as an apple : https://tbpl.mozilla.org/?tree=Try&rev=7753376c1239

Comment 8

[Kelsey Gilbert [:jgilbert]]

(In reply to Benoit Jacob [:bjacob] from comment #5)
> Comment on attachment 793723 [details] [diff] [review]
> patch revision 2
> 
> Review of attachment 793723 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> In theory, I agree with Jeff: it would be nice to have all these things
> handled by the blacklisting system.
> 
> The problem is that we do not currently have in our blocklisting system a
> Feature for "EXT_packed_depth_stencil", so we would have to add one, that
> that is very dangerous with our stupid blocklisting system: if a new
> downloadable blacklist entry were added with that feature, it would get
> mis-interpreted as "all features" by older Gecko's. That has severely bitten
> us before.
> 
> So for now --- pending the blocklisting system rewrite that has been
> discussed and even designed but never prioritized --- we will have to do
> with hardcoded checks as in the present patch.
> 
> Jeff: please consider lifting the r-
> Guillaume: please do not land as long as there is a r-
Alright, that was my fear. Let's just hardcode this then.

Comment 9

[Guillaume Abadie]

https://hg.mozilla.org/integration/mozilla-inbound/rev/13bbb02033f8

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/13bbb02033f8