908905 - Reduce blocking of packed_depth_stencil on mac+nv

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Kelsey Gilbert [:jgilbert]]

In bug 814839, we hit a crash at least Mac 10.8.2 machines with at least one NV card in the WebGL conformance tests for depth_texture. To resolve it, we blocked packed_depth_stencil on NV+Mac. I believe we should instead be blocking depth_texture.

Also, running the test on my 10.8.3 machine with a GeForce 9400M works fine, so we should further investigate adding a blocklisting case for NV+Mac<=10.8.2.

Comment 1

[Kelsey Gilbert [:jgilbert]]

Attachment: unblock-depth-stencil2

Change it to depth_texture, switch to disabling the feature instead of the extension, and moved it to a place where we're already doing WordAroundDrivers.

Comment 2

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

Comment on attachment 794920 [details] [diff] [review]
unblock-depth-stencil2

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

::: gfx/gl/GLContext.cpp
@@ +433,5 @@
>          if (WorkAroundDriverBugs()) {
>              if (Renderer() == RendererAdrenoTM320) {
>                  MarkExtensionUnsupported(OES_standard_derivatives);
>              }
> +            

Trailing \w .

@@ +437,5 @@
> +            
> +#ifdef XP_MACOSX
> +            // The Mac Nvidia driver, for versions up to and including 10.8, don't seem
> +            // to properly support this.  See 814839
> +            // this has been fixed in Mac OS X 10.9. See 907946

I feel that it is useful writing "bug $bug_id" rather than just "$bug_id" in comments.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 794920 [details] [diff] [review]
unblock-depth-stencil2

>From: Jeff Gilbert <[e-mail]>
>
>diff --git a/gfx/gl/GLContext.cpp b/gfx/gl/GLContext.cpp
>--- a/gfx/gl/GLContext.cpp
>+++ b/gfx/gl/GLContext.cpp
>@@ -429,16 +429,27 @@ GLContext::InitWithPrefix(const char *pr
> 
>         InitExtensions();
> 
>         // Disable extensions with partial or incorrect support.
>         if (WorkAroundDriverBugs()) {
>             if (Renderer() == RendererAdrenoTM320) {
>                 MarkExtensionUnsupported(OES_standard_derivatives);
>             }
>+
>+#ifdef XP_MACOSX
>+            // The Mac Nvidia driver, for versions up to and including 10.8, don't seem
>+            // to properly support this. (bug 814839)
>+            // This has been fixed in Mac OS X 10.9. (bug 907946)
>+            if (Vendor() == gl::GLContext::VendorNVIDIA &&
>+                !nsCocoaFeatures::OnMavericksOrLater())
>+            {
>+                MarkUnsupported(GLFeature::depth_texture);
>+            }
>+#endif
>         }
> 
>         NS_ASSERTION(!IsExtensionSupported(GLContext::ARB_pixel_buffer_object) ||
>                      (mSymbols.fMapBuffer && mSymbols.fUnmapBuffer),
>                      "ARB_pixel_buffer_object supported without glMapBuffer/UnmapBuffer being available!");
> 
>         if (SupportsRobustness()) {
>             if (IsExtensionSupported(ARB_robustness)) {
>@@ -907,28 +918,16 @@ GLContext::InitExtensions()
> 
>     if (WorkAroundDriverBugs() &&
>         Vendor() == VendorQualcomm) {
> 
>         // Some Adreno drivers do not report GL_OES_EGL_sync, but they really do support it.
>         MarkExtensionSupported(OES_EGL_sync);
>     }
> 
>-#ifdef XP_MACOSX
>-    // The Mac Nvidia driver, for versions up to and including 10.8, don't seem
>-    // to properly support this.  See 814839
>-    // this has been fixed in Mac OS X 10.9. See 907946
>-    if (WorkAroundDriverBugs() &&
>-        Vendor() == gl::GLContext::VendorNVIDIA &&
>-        !nsCocoaFeatures::OnMavericksOrLater())
>-    {
>-        MarkExtensionUnsupported(gl::GLContext::EXT_packed_depth_stencil);
>-    }
>-#endif
>-
> #ifdef DEBUG
>     firstRun = false;
> #endif
> }
> 
> 
> // Take texture data in a given buffer and copy it into a larger buffer,
> // padding out the edge pixels for filtering if necessary

Comment 4

[Kelsey Gilbert [:jgilbert]]

Cool, always wondered how 'edit as comment' worked.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/613a1dedd130

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/613a1dedd130

Comment 7

[conor]

Jeff, you mentioned investigating blacklisting for NV+Mac<=10.8.2 only, any reason that won't work? I really hate not having depth texture available to over half of our Mac users.

Comment 8

[Kelsey Gilbert [:jgilbert]]

(In reply to conor from comment #7)
> Jeff, you mentioned investigating blacklisting for NV+Mac<=10.8.2 only, any
> reason that won't work? I really hate not having depth texture available to
> over half of our Mac users.

Yep, that is now bug 918628.

Comment 9

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

(In reply to Jeff Gilbert [:jgilbert] from comment #8)
> (In reply to conor from comment #7)
> > Jeff, you mentioned investigating blacklisting for NV+Mac<=10.8.2 only, any
> > reason that won't work? I really hate not having depth texture available to
> > over half of our Mac users.
> 
> Yep, that is now bug 918628.

Hm, that's not the right number.  Is the bug still <= 10.8.2 (e.g. 10.8.5 should work if we didn't blacklist it?)