Fix graphics crash on systems missing gpu_shader5.

dom/canvas/WebGLContextValidate.cpp

@@ -440,6 +440,13 @@ WebGLContext::InitAndValidateGL(FailureReason* const out_failReason)
 {
     MOZ_RELEASE_ASSERT(gl, "GFX: GL not initialized");
 
+    if (!gl->MakeCurrent(true)) {
+        MOZ_ASSERT(false);
+        *out_failReason = { "FEATURE_FAILURE_WEBGL_MAKECURRENT",
+                            "Failed to MakeCurrent for init." };
+        return false;
+    }
+
     // Unconditionally create a new format usage authority. This is
     // important when restoring contexts and extensions need to add
     // formats back into the authority.

dom/canvas/WebGLShaderValidator.cpp

@@ -5,6 +5,7 @@
 
 #include "WebGLShaderValidator.h"
 
+#include <algorithm>
 #include "angle/ShaderLang.h"
 #include "gfxPrefs.h"
 #include "GLContext.h"
@@ -32,14 +33,14 @@ static int
 ChooseValidatorCompileOptions(const ShBuiltInResources& resources,
                               const mozilla::gl::GLContext* gl)
 {
-    int options = SH_VARIABLES |
+    int options = SH_VARIABLES |
-                  SH_ENFORCE_PACKING_RESTRICTIONS |
+                  SH_ENFORCE_PACKING_RESTRICTIONS |
                   SH_INIT_VARYINGS_WITHOUT_STATIC_USE |
-                  SH_OBJECT_CODE |
+                  SH_OBJECT_CODE |
-                  SH_INIT_GL_POSITION;
+                  SH_INIT_GL_POSITION;
-
+
-    if (resources.MaxExpressionComplexity > 0) {
+    if (resources.MaxExpressionComplexity > 0) {
-        options |= SH_LIMIT_EXPRESSION_COMPLEXITY;
+        options |= SH_LIMIT_EXPRESSION_COMPLEXITY;
     }
     // Sampler arrays indexed with non-constant expressions are forbidden in
     // GLSL 1.30 and later.
@@ -50,17 +51,17 @@ ChooseValidatorCompileOptions(const ShBuiltInResources& resources,
     // Needed for driver bug detection
     options |= SH_EMULATE_BUILT_IN_FUNCTIONS;
 
-    if (gfxPrefs::WebGLAllANGLEOptions()) {
+    if (gfxPrefs::WebGLAllANGLEOptions()) {
-        return options |
+        return options |
-               SH_VALIDATE_LOOP_INDEXING |
+               SH_VALIDATE_LOOP_INDEXING |
-               SH_UNROLL_FOR_LOOP_WITH_INTEGER_INDEX |
+               SH_UNROLL_FOR_LOOP_WITH_INTEGER_INDEX |
-               SH_UNROLL_FOR_LOOP_WITH_SAMPLER_ARRAY_INDEX |
+               SH_UNROLL_FOR_LOOP_WITH_SAMPLER_ARRAY_INDEX |
-               SH_CLAMP_INDIRECT_ARRAY_BOUNDS |
+               SH_CLAMP_INDIRECT_ARRAY_BOUNDS |
-               SH_UNFOLD_SHORT_CIRCUIT |
+               SH_UNFOLD_SHORT_CIRCUIT |
-               SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS |
+               SH_SCALARIZE_VEC_AND_MAT_CONSTRUCTOR_ARGS |
-               SH_INIT_OUTPUT_VARIABLES |
+               SH_INIT_OUTPUT_VARIABLES |
-               SH_REGENERATE_STRUCT_NAMES;
+               SH_REGENERATE_STRUCT_NAMES;
-    }
+    }
 
 #ifndef XP_MACOSX
     // We want to do this everywhere, but to do this on Mac, we need
@@ -112,6 +113,16 @@ ShaderOutput(gl::GLContext* gl)
         return SH_ESSL_OUTPUT;
     } else {
         uint32_t version = gl->ShadingLanguageVersion();
+
+        // Version 130 starts to require integral constant expressions for loop indices,
+        // instead of "constant-index-expression".
+        // Both version 400 and gpu_shader5 remove this restrictions.
+        // gpu_shader5 went core in 400, so we can just check for the GLFeature.
+        // If we're compiling for webglsl1, even for webgl2, we need gpu_shader5, or GLSL_COMPAT.
+        if (!gl->IsSupported(gl::GLFeature::gpu_shader5)) {
+            version = std::min<uint32_t>(version, 120);
+        }
+
         switch (version) {
         case 100: return SH_GLSL_COMPATIBILITY_OUTPUT;
         case 120: return SH_GLSL_COMPATIBILITY_OUTPUT;

gfx/angle/src/compiler/translator/TranslatorGLSL.cpp

@@ -240,20 +240,14 @@ void TranslatorGLSL::writeExtensionBehavior(TIntermNode *root)
     }
 
     // Need to enable gpu_shader5 to have index constant sampler array indexing
-    if (getOutputType() != SH_ESSL_OUTPUT && getOutputType() < SH_GLSL_400_CORE_OUTPUT)
+    if (getOutputType() != SH_ESSL_OUTPUT && getOutputType() < SH_GLSL_400_CORE_OUTPUT &&
+        getShaderVersion() == 100)
     {
-        sink << "#extension GL_ARB_gpu_shader5 : ";
+        // Don't use "require" to avoid breaking WebGL 1 on drivers that silently
-
+        // support index constant sampler array indexing, but don't have the extension or
-        // Don't use "require" on WebGL 1 to avoid breaking WebGL on drivers that silently
+        // on drivers that don't have the extension at all as it would break WebGL 1 for
-        // support index constant sampler array indexing, but don't have the extension.
+        // some users.
-        if (getShaderVersion() >= 300)
+        sink << "#extension GL_ARB_gpu_shader5 : enable\n";
-        {
-            sink << "require\n";
-        }
-        else
-        {
-            sink << "enable\n";
-        }
     }
 
     TExtensionGLSL extensionGLSL(getOutputType());

gfx/angle/src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -543,16 +543,11 @@ void GenerateCaps(const FunctionsGL *functions, gl::Caps *caps, gl::TextureCapsM
         LimitVersion(maxSupportedESVersion, gl::Version(2, 0));
     }
 
-    // Check if index constant sampler array indexing is supported
+    // Non-constant sampler array indexing is required for OpenGL ES 2 and OpenGL ES after 3.2.
-    if (!functions->isAtLeastGL(gl::Version(4, 0)) &&
+    // However having it available on OpenGL ES 2 is a specification bug, and using this
-        !functions->isAtLeastGLES(gl::Version(2, 0)) &&
+    // indexing in WebGL is undefined. Requiring this feature would break WebGL 1 for some users
-        !functions->hasExtension("GL_ARB_gpu_shader5"))
+    // so we don't check for it. (it is present with ESSL 100, ESSL >= 320, GLSL >= 400 and
-    {
+    // GL_ARB_gpu_shader5)
-        // This should also be required for ES2 but there are some driver support index constant
-        // sampler array indexing without meeting the requirements above. Don't limit their ES
-        // version as it would break WebGL for some users.
-        LimitVersion(maxSupportedESVersion, gl::Version(2, 0));
-    }
 
     // Check if sampler objects are supported
     if (!functions->isAtLeastGL(gl::Version(3, 3)) &&

gfx/gl/GLContext.cpp

@@ -95,6 +95,7 @@ static const char* const sExtensionNames[] = {
     "GL_ARB_framebuffer_object",
     "GL_ARB_framebuffer_sRGB",
     "GL_ARB_geometry_shader4",
+    "GL_ARB_gpu_shader5",
     "GL_ARB_half_float_pixel",
     "GL_ARB_instanced_arrays",
     "GL_ARB_internalformat_query",
@@ -134,6 +135,7 @@ static const char* const sExtensionNames[] = {
     "GL_EXT_framebuffer_object",
     "GL_EXT_framebuffer_sRGB",
     "GL_EXT_gpu_shader4",
+    "GL_EXT_gpu_shader5",
     "GL_EXT_multisampled_render_to_texture",
     "GL_EXT_occlusion_query_boolean",
     "GL_EXT_packed_depth_stencil",
@@ -160,6 +162,7 @@ static const char* const sExtensionNames[] = {
     "GL_NV_fence",
     "GL_NV_framebuffer_blit",
     "GL_NV_geometry_program4",
+    "GL_NV_gpu_shader5",
     "GL_NV_half_float",
     "GL_NV_instanced_arrays",
     "GL_NV_primitive_restart",
@@ -575,6 +578,10 @@ GLContext::LoadFeatureSymbols(const char* prefix, bool trygl, const SymLoadStruc
 bool
 GLContext::InitWithPrefixImpl(const char* prefix, bool trygl)
 {
+    // wglGetProcAddress requires a current context.
+    if (!MakeCurrent(true))
+        return false;
+
     mWorkAroundDriverBugs = gfxPrefs::WorkAroundDriverBugs();
 
     const SymLoadStruct coreSymbols[] = {
@@ -711,7 +718,6 @@ GLContext::InitWithPrefixImpl(const char* prefix, bool trygl)
 
     ////////////////
 
-    MakeCurrent();
     MOZ_ASSERT(mProfile != ContextProfile::Unknown);
 
     uint32_t version = 0;
@@ -2250,13 +2256,11 @@ GLContext::MarkDestroyed()
     mBlitHelper = nullptr;
     mReadTexImageHelper = nullptr;
 
-    if (MakeCurrent()) {
+    mIsDestroyed = true;
-        mTexGarbageBin->GLContextTeardown();
-    } else {
-        NS_WARNING("MakeCurrent() failed during MarkDestroyed! Skipping GL object teardown.");
-    }
-
     mSymbols.Zero();
+    if (MakeCurrent(true)) {
+        mTexGarbageBin->GLContextTeardown();
+    }
 }
 
 #ifdef MOZ_GL_DEBUG

gfx/gl/GLContext.h

@@ -102,6 +102,7 @@ enum class GLFeature {
     get_query_object_iv,
     get_string_indexed,
     gpu_shader4,
+    gpu_shader5,
     instanced_arrays,
     instanced_non_arrays,
     internalformat_query,
@@ -352,6 +353,7 @@ public:
 protected:
     bool mIsOffscreen;
     bool mContextLost;
+    bool mIsDestroyed = false;
 
     /**
      * mVersion store the OpenGL's version, multiplied by 100. For example, if
@@ -421,6 +423,7 @@ public:
         ARB_framebuffer_object,
         ARB_framebuffer_sRGB,
         ARB_geometry_shader4,
+        ARB_gpu_shader5,
         ARB_half_float_pixel,
         ARB_instanced_arrays,
         ARB_internalformat_query,
@@ -460,6 +463,7 @@ public:
         EXT_framebuffer_object,
         EXT_framebuffer_sRGB,
         EXT_gpu_shader4,
+        EXT_gpu_shader5,
         EXT_multisampled_render_to_texture,
         EXT_occlusion_query_boolean,
         EXT_packed_depth_stencil,
@@ -486,6 +490,7 @@ public:
         NV_fence,
         NV_framebuffer_blit,
         NV_geometry_program4,
+        NV_gpu_shader5,
         NV_half_float,
         NV_instanced_arrays,
         NV_primitive_restart,
@@ -3261,7 +3266,7 @@ public:
 #endif
         return MakeCurrentImpl(aForce);
     }
-
+    
     virtual bool Init() = 0;
 
     virtual bool SetupLookupFunction() = 0;
@@ -3269,8 +3274,7 @@ public:
     virtual void ReleaseSurface() {}
 
     bool IsDestroyed() {
-        // MarkDestroyed will mark all these as null.
+        return mIsDestroyed;
-        return mSymbols.fUseProgram == nullptr;
     }
 
     GLContext* GetSharedContext() { return mSharedContext; }

gfx/gl/GLContextCGL.h

@@ -24,7 +24,7 @@ class GLContextCGL : public GLContext
 {
     friend class GLContextProviderCGL;
 
-    NSOpenGLContext* mContext;
+    NSOpenGLContext* const mContext;
 
 public:
     MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(GLContextCGL, override)

gfx/gl/GLContextFeatures.cpp

@@ -331,6 +331,18 @@ static const FeatureInfo sFeatureInfoArr[] = {
             GLContext::Extensions_End
         }
     },
+    {
+        "gpu_shader5",
+        GLVersion::GL4,
+        GLESVersion::NONE,
+        GLContext::Extension_None,
+        {
+            GLContext::ARB_gpu_shader5,
+            GLContext::EXT_gpu_shader5,
+            GLContext::NV_gpu_shader5,
+            GLContext::Extensions_End
+        }
+    },    
     {
         "instanced_arrays",
         GLVersion::GL3_3,

gfx/gl/GLContextGLX.h

@@ -86,6 +86,7 @@ private:
     GLXContext mContext;
     Display* mDisplay;
     GLXDrawable mDrawable;
+    Maybe<GLXDrawable> mOverrideDrawable;
     bool mDeleteDrawable;
     bool mDoubleBuffered;
 

gfx/gl/GLContextProviderCGL.mm

@@ -83,26 +83,13 @@ GLContextCGL::GLContextCGL(CreateContextFlags flags, const SurfaceCaps& caps,
 GLContextCGL::~GLContextCGL()
 {
     MarkDestroyed();
-
+    [mContext release];
-    if (mContext) {
-        if ([NSOpenGLContext currentContext] == mContext) {
-            // Clear the current context before releasing. If we don't do
-            // this, the next time we call [NSOpenGLContext currentContext],
-            // "invalid context" will be printed to the console.
-            [NSOpenGLContext clearCurrentContext];
-        }
-        [mContext release];
-    }
-
 }
 
 bool
 GLContextCGL::Init()
 {
-    if (!InitWithPrefix("gl", true))
+    return InitWithPrefix("gl", true);
-        return false;
-
-    return true;
 }
 
 CGLContextObj
@@ -114,6 +101,11 @@ GLContextCGL::GetCGLContext() const
 bool
 GLContextCGL::MakeCurrentImpl(bool aForce)
 {
+    if (IsDestroyed()) {
+        [NSOpenGLContext clearCurrentContext];
+        return false;
+    }
+
     if (!aForce && [NSOpenGLContext currentContext] == mContext) {
         return true;
     }

gfx/gl/GLContextProviderEAGL.mm

@@ -36,35 +36,24 @@ GLContextEAGL::GLContextEAGL(CreateContextFlags flags, const SurfaceCaps& caps,
 
 GLContextEAGL::~GLContextEAGL()
 {
-    MakeCurrent();
+    if (MakeCurrent()) {
-
+        if (mBackbufferFB) {
-    if (mBackbufferFB) {
+            fDeleteFramebuffers(1, &mBackbufferFB);
-        fDeleteFramebuffers(1, &mBackbufferFB);
+        }
-    }
+        if (mBackbufferRB) {
-
+            fDeleteRenderbuffers(1, &mBackbufferRB);
-    if (mBackbufferRB) {
+        }
-        fDeleteRenderbuffers(1, &mBackbufferRB);
     }
 
+    mLayer = nil;
     MarkDestroyed();
-
+    [mContext release];
-    if (mLayer) {
-      mLayer = nil;
-    }
-
-    if (mContext) {
-      [EAGLContext setCurrentContext:nil];
-      [mContext release];
-    }
 }
 
 bool
 GLContextEAGL::Init()
 {
-    if (!InitWithPrefix("gl", true))
+    return InitWithPrefix("gl", true);
-        return false;
-
-    return true;
 }
 
 bool
@@ -120,12 +109,12 @@ GLContextEAGL::MakeCurrentImpl(bool aForce)
         return true;
     }
 
-    if (mContext) {
+    if (IsDestroyed()) {
-        if(![EAGLContext setCurrentContext:mContext]) {
+        [EAGLContext setCurrentContext:nil];
-            return false;
+        return false;
-        }
     }
-    return true;
+    
+    return [EAGLContext setCurrentContext:mContext];
 }
 
 bool

gfx/gl/GLContextProviderEGL.cpp

@@ -150,13 +150,12 @@ is_power_of_two(int v)
 }
 
 static void
-DestroySurface(EGLSurface oldSurface) {
+DestroySurface(const EGLSurface surf) {
-    if (oldSurface != EGL_NO_SURFACE) {
+    if (!surf) {
-        sEGLLibrary.fMakeCurrent(EGL_DISPLAY(),
+        // Nothing to do.
-                                 EGL_NO_SURFACE, EGL_NO_SURFACE,
+        return;
-                                 EGL_NO_CONTEXT);
-        sEGLLibrary.fDestroySurface(EGL_DISPLAY(), oldSurface);
     }
+    sEGLLibrary.fDestroySurface(EGL_DISPLAY(), surf);
 }
 
 static EGLSurface
@@ -223,7 +222,7 @@ GLContextEGL::~GLContextEGL()
     sEGLLibrary.fDestroyContext(EGL_DISPLAY(), mContext);
     sEGLLibrary.UnsetCachedCurrentContext();
 
-    mozilla::gl::DestroySurface(mSurface);
+    DestroySurface(mSurface);
 }
 
 bool
@@ -247,12 +246,7 @@ GLContextEGL::Init()
     if (!InitWithPrefix("gl", true))
         return false;
 
-    bool current = MakeCurrent();
+    MOZ_ASSERT(IsCurrent());
-    if (!current) {
-        gfx::LogFailure(NS_LITERAL_CSTRING(
-            "Couldn't get device attachments for device."));
-        return false;
-    }
 
     static_assert(sizeof(GLint) >= sizeof(int32_t), "GLint is smaller than int32_t");
     mMaxTextureImageSize = INT32_MAX;
@@ -303,7 +297,10 @@ GLContextEGL::ReleaseTexImage()
 }
 
 void
-GLContextEGL::SetEGLSurfaceOverride(EGLSurface surf) {
+GLContextEGL::SetEGLSurfaceOverride(const EGLSurface surf) {
+
+    MOZ_ASSERT(!surf || surf != mSurface);
+
     if (Screen()) {
         /* Blit `draw` to `read` if we need to, before we potentially juggle
           * `read` around. If we don't, we might attach a different `read`,
@@ -314,12 +311,17 @@ GLContextEGL::SetEGLSurfaceOverride(EGLSurface surf) {
     }
 
     mSurfaceOverride = surf;
-    DebugOnly<bool> ok = MakeCurrent(true);
+    MOZ_ALWAYS_TRUE(MakeCurrent(true));
-    MOZ_ASSERT(ok);
 }
 
 bool
 GLContextEGL::MakeCurrentImpl(bool aForce) {
+    if (IsDestroyed()) {
+        //Clear and exit
+        sEGLLibrary.fMakeCurrent(EGL_DISPLAY(), nullptr, nullptr, nullptr);
+        return false;
+    }
+    
     bool succeeded = true;
 
     // Assume that EGL has the same problem as WGL does,
@@ -373,7 +375,7 @@ GLContextEGL::IsCurrent() {
 }
 
 bool
-GLContextEGL::RenewSurface(nsIWidget* aWidget) {
+GLContextEGL::RenewSurface(nsIWidget* const aWidget) {
     if (!mOwnsContext) {
         return false;
     }
@@ -389,13 +391,12 @@ GLContextEGL::RenewSurface(nsIWidget* aWidget) {
 
 void
 GLContextEGL::ReleaseSurface() {
-    if (mOwnsContext) {
+    if (!mOwnsContext) {
-        mozilla::gl::DestroySurface(mSurface);
+        return;
     }
-    if (mSurface == mSurfaceOverride) {
+    
-        mSurfaceOverride = EGL_NO_SURFACE;
+    DestroySurface(mSurface);
-    }
+    mSurface = nullptr;
-    mSurface = EGL_NO_SURFACE;
 }
 
 bool

gfx/gl/GLContextProviderGLX.cpp

@@ -894,20 +894,12 @@ GLContextGLX::~GLContextGLX()
         return;
     }
 
-    // see bug 659842 comment 76
-#ifdef DEBUG
-    bool success =
-#endif
-    mGLX->xMakeCurrent(mDisplay, X11None, nullptr);
-    MOZ_ASSERT(success,
-               "glXMakeCurrent failed to release GL context before we call "
-               "glXDestroyContext!");
-
     mGLX->xDestroyContext(mDisplay, mContext);
 
     if (mDeleteDrawable) {
         mGLX->xDestroyPixmap(mDisplay, mDrawable);
     }
+    MOZ_ASSERT(!mOverrideDrawable);
 }
 
 
@@ -944,6 +936,10 @@ GLContextGLX::MakeCurrentImpl(bool aForce)
           // DRI2InvalidateBuffers event before drawing. See bug 1280653.
           Unused << XPending(mDisplay);
         }
+        if (IsDestroyed()) {
+            MOZ_ALWAYS_TRUE(mGLX->xMakeCurrent(mDisplay, X11None, nullptr));
+            return false; // We did not MakeCurrent mContext, but that's what we wanted!
+        }
 
         succeeded = mGLX->xMakeCurrent(mDisplay, mDrawable, mContext);
         NS_ASSERTION(succeeded, "Failed to make GL context current!");
@@ -1017,16 +1013,18 @@ GLContextGLX::GetWSIInfo(nsCString* const out) const
 bool
 GLContextGLX::OverrideDrawable(GLXDrawable drawable)
 {
-    if (Screen())
+    if (Screen()) {
         Screen()->AssureBlitted();
-    Bool result = mGLX->xMakeCurrent(mDisplay, drawable, mContext);
+    }
-    return result;
+    mOverrideDrawable = Some(drawable);
+    return MakeCurrent(true);
 }
 
 bool
 GLContextGLX::RestoreDrawable()
 {
-    return mGLX->xMakeCurrent(mDisplay, mDrawable, mContext);
+    mOverrideDrawable = Nothing();
+    return MakeCurrent(true);
 }
 
 GLContextGLX::GLContextGLX(

gfx/gl/GLContextProviderWGL.cpp

@@ -314,20 +314,17 @@ GLContextWGL::Init()
     if (!mDC || !mContext)
         return false;
 
-    // see bug 929506 comment 29. wglGetProcAddress requires a current context.
-    if (!sWGLLib.fMakeCurrent(mDC, mContext))
-        return false;
-
     SetupLookupFunction();
-    if (!InitWithPrefix("gl", true))
+    return InitWithPrefix("gl", true);
-        return false;
-
-    return true;
 }
 
 bool
 GLContextWGL::MakeCurrentImpl(bool aForce)
 {
+    if (IsDestroyed()) {
+        MOZ_ALWAYS_TRUE(sWGLLib.fMakeCurrent(0, 0));
+    }
+    
     BOOL succeeded = true;
 
     // wglGetCurrentContext seems to just pull the HGLRC out