Bug 1535809 - Add warning on use of implicitly enabled extensions r=jgilbert

Differential Revision: https://phabricator.services.mozilla.com/D36431

--HG--
extra : moz-landing-system : lando

dom/canvas/WebGLContext.h

@@ -1694,7 +1694,8 @@ class WebGLContext : public nsICanvasRenderingContextInternal,
 
   // Enable an extension if it's supported. Return the extension on success.
   WebGLExtensionBase* EnableSupportedExtension(dom::CallerType callerType,
-                                               WebGLExtensionID ext);
+                                               WebGLExtensionID ext,
+                                               bool explict = true);
 
  public:
   // returns true if the extension has been enabled by calling getExtension.
@@ -1702,6 +1703,10 @@ class WebGLContext : public nsICanvasRenderingContextInternal,
     return mExtensions[ext];
   }
 
+  bool IsExtensionExplicit(const WebGLExtensionID ext) const;
+
+  void WarnIfImplicit(const WebGLExtensionID ext);
+
  protected:
   // returns true if the extension is supported for this caller type (this
   // decides what getSupportedExtensions exposes)

dom/canvas/WebGLContextDraw.cpp

@@ -333,13 +333,15 @@ const webgl::CachedDrawFetchLimits* ValidateDraw(WebGLContext* const webgl,
   if (!webgl->BindCurFBForDraw()) return nullptr;
 
   const auto& fb = webgl->mBoundDrawFramebuffer;
-  if (fb && !webgl->IsExtensionEnabled(WebGLExtensionID::EXT_float_blend) &&
+  if (fb && webgl->mBlendEnabled) {
-      webgl->mBlendEnabled) {
     const auto& info = *fb->GetCompletenessInfo();
     if (info.hasFloat32) {
-      webgl->ErrorInvalidOperation(
+      if (!webgl->IsExtensionEnabled(WebGLExtensionID::EXT_float_blend)) {
-          "Float32 blending requires EXT_float_blend.");
+        webgl->ErrorInvalidOperation(
-      return nullptr;
+            "Float32 blending requires EXT_float_blend.");
+        return nullptr;
+      }
+      webgl->WarnIfImplicit(WebGLExtensionID::EXT_float_blend);
     }
   }
 

dom/canvas/WebGLContextExtensions.cpp

@@ -235,20 +235,44 @@ bool WebGLContext::IsExtensionSupported(WebGLExtensionID ext) const {
   return false;
 }
 
+bool WebGLContext::IsExtensionExplicit(const WebGLExtensionID ext) const {
+  MOZ_ASSERT(ext < WebGLExtensionID::Max);
+  return mExtensions[ext] && mExtensions[ext]->IsExplicit();
+}
+
+void WebGLContext::WarnIfImplicit(const WebGLExtensionID ext) {
+  MOZ_ASSERT(ext < WebGLExtensionID::Max);
+  const auto& extension = mExtensions[ext];
+  if (!extension || extension->IsExplicit()) return;
+
+  GenerateWarning(
+      "Using format enabled by implicitly enabled extension: %s. "
+      "For maximal portability enable it explicitly.",
+      GetExtensionString(ext));
+  // Don't spam warnings
+  extension->SetExplicit();
+}
+
 static bool CompareWebGLExtensionName(const nsACString& name,
                                       const char* other) {
   return name.Equals(other, nsCaseInsensitiveCStringComparator());
 }
 
 WebGLExtensionBase* WebGLContext::EnableSupportedExtension(
-    dom::CallerType callerType, WebGLExtensionID ext) {
+    dom::CallerType callerType, WebGLExtensionID ext, bool explict) {
   if (!IsExtensionEnabled(ext)) {
     if (!IsExtensionSupported(callerType, ext)) return nullptr;
 
     EnableExtension(ext);
   }
 
-  return mExtensions[ext];
+  const auto extension = mExtensions[ext];
+  MOZ_ASSERT(extension);
+  if (explict && !extension->IsExplicit()) {
+    extension->SetExplicit();
+  }
+
+  return extension;
 }
 
 void WebGLContext::GetExtension(JSContext* cx, const nsAString& wideName,
@@ -285,22 +309,25 @@ void WebGLContext::GetExtension(JSContext* cx, const nsAString& wideName,
   // Step 4: Enable any implied extensions.
   switch (ext) {
     case WebGLExtensionID::EXT_color_buffer_float:
-      EnableSupportedExtension(callerType, WebGLExtensionID::EXT_float_blend);
+      EnableSupportedExtension(callerType, WebGLExtensionID::EXT_float_blend,
+                               false);
       break;
 
     case WebGLExtensionID::OES_texture_float:
-      EnableSupportedExtension(callerType,
+      EnableSupportedExtension(
-                               WebGLExtensionID::WEBGL_color_buffer_float);
+          callerType, WebGLExtensionID::WEBGL_color_buffer_float, false);
-      EnableSupportedExtension(callerType, WebGLExtensionID::EXT_float_blend);
+      EnableSupportedExtension(callerType, WebGLExtensionID::EXT_float_blend,
+                               false);
       break;
 
     case WebGLExtensionID::OES_texture_half_float:
-      EnableSupportedExtension(callerType,
+      EnableSupportedExtension(
-                               WebGLExtensionID::EXT_color_buffer_half_float);
+          callerType, WebGLExtensionID::EXT_color_buffer_half_float, false);
       break;
 
     case WebGLExtensionID::WEBGL_color_buffer_float:
-      EnableSupportedExtension(callerType, WebGLExtensionID::EXT_float_blend);
+      EnableSupportedExtension(callerType, WebGLExtensionID::EXT_float_blend,
+                               false);
       break;
 
     default:

dom/canvas/WebGLContextFramebufferOperations.cpp

@@ -72,11 +72,27 @@ void WebGLContext::ClearColor(GLfloat r, GLfloat g, GLfloat b, GLfloat a) {
   const FuncScope funcScope(*this, "clearColor");
   if (IsContextLost()) return;
 
-  const bool supportsFloatColorBuffers =
+  if (IsExtensionEnabled(WebGLExtensionID::EXT_color_buffer_float)) {
-      (IsExtensionEnabled(WebGLExtensionID::EXT_color_buffer_float) ||
+    MOZ_ASSERT(IsExtensionExplicit(WebGLExtensionID::EXT_color_buffer_float));
-       IsExtensionEnabled(WebGLExtensionID::EXT_color_buffer_half_float) ||
+
-       IsExtensionEnabled(WebGLExtensionID::WEBGL_color_buffer_float));
+  } else if (IsExtensionEnabled(
-  if (!supportsFloatColorBuffers) {
+                 WebGLExtensionID::EXT_color_buffer_half_float) ||
+             IsExtensionEnabled(WebGLExtensionID::WEBGL_color_buffer_float)) {
+    const bool explict =
+        (IsExtensionExplicit(WebGLExtensionID::EXT_color_buffer_half_float) ||
+         IsExtensionExplicit(WebGLExtensionID::WEBGL_color_buffer_float));
+    const bool wouldHaveClamped =
+        (r != GLClampFloat(r) || g != GLClampFloat(g) || b != GLClampFloat(b) ||
+         a != GLClampFloat(a));
+    if (!explict && wouldHaveClamped) {
+      if (IsExtensionEnabled(WebGLExtensionID::EXT_color_buffer_half_float)) {
+        WarnIfImplicit(WebGLExtensionID::EXT_color_buffer_half_float);
+      } else if (IsExtensionEnabled(
+                     WebGLExtensionID::WEBGL_color_buffer_float)) {
+        WarnIfImplicit(WebGLExtensionID::WEBGL_color_buffer_float);
+      }
+    }
+  } else {
     r = GLClampFloat(r);
     g = GLClampFloat(g);
     b = GLClampFloat(b);

dom/canvas/WebGLExtensionColorBufferFloat.cpp

@@ -15,13 +15,18 @@ WebGLExtensionColorBufferFloat::WebGLExtensionColorBufferFloat(
     WebGLContext* webgl)
     : WebGLExtensionBase(webgl) {
   MOZ_ASSERT(IsSupported(webgl), "Don't construct extension if unsupported.");
+  SetRenderable(webgl::FormatRenderableState::Implicit(
+      WebGLExtensionID::WEBGL_color_buffer_float));
+}
 
-  auto& fua = webgl->mFormatUsage;
+void WebGLExtensionColorBufferFloat::SetRenderable(
+    const webgl::FormatRenderableState state) {
+  auto& fua = mContext->mFormatUsage;
 
-  auto fnUpdateUsage = [&fua](GLenum sizedFormat,
+  auto fnUpdateUsage = [&](GLenum sizedFormat,
-                              webgl::EffectiveFormat effFormat) {
+                           webgl::EffectiveFormat effFormat) {
     auto usage = fua->EditUsage(effFormat);
-    usage->SetRenderable();
+    usage->SetRenderable(state);
     fua->AllowRBFormat(sizedFormat, usage);
   };
 
@@ -33,6 +38,10 @@ WebGLExtensionColorBufferFloat::WebGLExtensionColorBufferFloat(
 #undef FOO
 }
 
+void WebGLExtensionColorBufferFloat::OnSetExplicit() {
+  SetRenderable(webgl::FormatRenderableState::Explicit());
+}
+
 WebGLExtensionColorBufferFloat::~WebGLExtensionColorBufferFloat() {}
 
 bool WebGLExtensionColorBufferFloat::IsSupported(const WebGLContext* webgl) {

dom/canvas/WebGLExtensionColorBufferHalfFloat.cpp

@@ -15,15 +15,19 @@ WebGLExtensionColorBufferHalfFloat::WebGLExtensionColorBufferHalfFloat(
     WebGLContext* webgl)
     : WebGLExtensionBase(webgl) {
   MOZ_ASSERT(IsSupported(webgl), "Don't construct extension if unsupported.");
+  SetRenderable(webgl::FormatRenderableState::Implicit(
+      WebGLExtensionID::EXT_color_buffer_half_float));
+}
 
-  auto& fua = webgl->mFormatUsage;
+void WebGLExtensionColorBufferHalfFloat::SetRenderable(
+    const webgl::FormatRenderableState state) {
+  auto& fua = mContext->mFormatUsage;
 
-  auto fnUpdateUsage = [&fua](GLenum sizedFormat,
+  auto fnUpdateUsage = [&](GLenum sizedFormat, webgl::EffectiveFormat effFormat,
-                              webgl::EffectiveFormat effFormat,
+                           const bool renderable) {
-                              const bool renderable) {
     auto usage = fua->EditUsage(effFormat);
     if (renderable) {
-      usage->SetRenderable();
+      usage->SetRenderable(state);
     }
     fua->AllowRBFormat(sizedFormat, usage, renderable);
   };
@@ -37,6 +41,10 @@ WebGLExtensionColorBufferHalfFloat::WebGLExtensionColorBufferHalfFloat(
 #undef FOO
 }
 
+void WebGLExtensionColorBufferHalfFloat::OnSetExplicit() {
+  SetRenderable(webgl::FormatRenderableState::Explicit());
+}
+
 WebGLExtensionColorBufferHalfFloat::~WebGLExtensionColorBufferHalfFloat() {}
 
 bool WebGLExtensionColorBufferHalfFloat::IsSupported(

dom/canvas/WebGLExtensions.cpp

@@ -12,8 +12,8 @@
 
 namespace mozilla {
 
-WebGLExtensionBase::WebGLExtensionBase(WebGLContext* context)
+WebGLExtensionBase::WebGLExtensionBase(WebGLContext* webgl)
-    : WebGLContextBoundObject(context), mIsLost(false) {}
+    : WebGLContextBoundObject(webgl), mIsLost(false), mIsExplicit(false) {}
 
 WebGLExtensionBase::~WebGLExtensionBase() {}
 

dom/canvas/WebGLExtensions.h

@@ -12,6 +12,7 @@
 #include "nsWrapperCache.h"
 #include "WebGLObjectModel.h"
 #include "WebGLTypes.h"
+#include "WebGLFormats.h"
 
 namespace mozilla {
 class ErrorResult;
@@ -42,6 +43,13 @@ class WebGLExtensionBase : public nsWrapperCache,
 
   void MarkLost();
 
+  void SetExplicit() {
+    mIsExplicit = true;
+    OnSetExplicit();
+  }
+
+  bool IsExplicit() { return mIsExplicit; }
+
   NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(WebGLExtensionBase)
   NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(WebGLExtensionBase)
 
@@ -51,6 +59,10 @@ class WebGLExtensionBase : public nsWrapperCache,
   virtual void OnMarkLost() {}
 
   bool mIsLost;
+
+  virtual void OnSetExplicit() {}
+
+  bool mIsExplicit;
 };
 
 #define DECL_WEBGL_EXTENSION_GOOP             \
@@ -321,6 +333,10 @@ class WebGLExtensionColorBufferFloat : public WebGLExtensionBase {
 
   static bool IsSupported(const WebGLContext*);
 
+  void SetRenderable(const webgl::FormatRenderableState);
+
+  void OnSetExplicit() override;
+
   DECL_WEBGL_EXTENSION_GOOP
 };
 
@@ -331,6 +347,10 @@ class WebGLExtensionColorBufferHalfFloat : public WebGLExtensionBase {
 
   static bool IsSupported(const WebGLContext*);
 
+  void SetRenderable(const webgl::FormatRenderableState);
+
+  void OnSetExplicit() override;
+
   DECL_WEBGL_EXTENSION_GOOP
 };
 

dom/canvas/WebGLFormats.cpp

@@ -765,8 +765,10 @@ static bool AddUnsizedFormats(FormatUsageAuthority* fua, gl::GLContext* gl) {
   // clang-format on
 }
 
-void FormatUsageInfo::SetRenderable() {
+void FormatUsageInfo::SetRenderable(const FormatRenderableState& state) {
-  this->isRenderable = true;
+  if (!renderableState.IsExplicit()) {
+    renderableState = state;
+  }
 
 #ifdef DEBUG
   const auto format = this->format;
@@ -1125,6 +1127,11 @@ void FormatUsageAuthority::AllowRBFormat(GLenum sizedFormat,
   MOZ_ASSERT(usage->format->sizedFormat);
   MOZ_ASSERT(usage->IsRenderable() || !expectRenderable);
 
+  const auto& found = mRBFormatMap.find(sizedFormat);
+  if (found != mRBFormatMap.end()) {
+    MOZ_ASSERT(found->second == usage);
+    return;
+  }
   AlwaysInsert(mRBFormatMap, sizedFormat, usage);
 }
 

dom/canvas/WebGLFormats.h

@@ -309,11 +309,35 @@ GLenum ComponentType(const FormatInfo* format);
 */
 ////////////////////////////////////////
 
+struct FormatRenderableState final {
+ private:
+  enum class RenderableState {
+    Disabled,
+    Implicit,
+    Explicit,
+  };
+
+ public:
+  RenderableState state = RenderableState::Disabled;
+  WebGLExtensionID extid = WebGLExtensionID::Max;
+
+  static FormatRenderableState Explicit() {
+    return {RenderableState::Explicit};
+  }
+
+  static FormatRenderableState Implicit(WebGLExtensionID extid) {
+    return {RenderableState::Implicit, extid};
+  }
+
+  bool IsRenderable() const { return state != RenderableState::Disabled; }
+  bool IsExplicit() const { return state == RenderableState::Explicit; }
+};
+
 struct FormatUsageInfo {
   const FormatInfo* const format;
 
  private:
-  bool isRenderable = false;
+  FormatRenderableState renderableState;
 
  public:
   bool isFilterable = false;
@@ -338,8 +362,14 @@ struct FormatUsageInfo {
     }
   }
 
-  bool IsRenderable() const { return isRenderable; }
+  bool IsRenderable() const { return renderableState.IsRenderable(); }
-  void SetRenderable();
+  void SetRenderable(
+      const FormatRenderableState& state = FormatRenderableState::Explicit());
+  bool IsExplicitlyRenderable() const { return renderableState.IsExplicit(); }
+  WebGLExtensionID GetExtensionID() const {
+    MOZ_ASSERT(renderableState.extid != WebGLExtensionID::Max);
+    return renderableState.extid;
+  }
 
   bool IsUnpackValid(const PackingInfo& key,
                      const DriverUnpackInfo** const out_value) const;

dom/canvas/WebGLFramebuffer.cpp

@@ -143,6 +143,9 @@ bool WebGLFBAttachPoint::IsComplete(WebGLContext* webgl,
     fnWriteErrorInfo(info.BeginReading());
     return false;
   }
+  if (!formatUsage->IsExplicitlyRenderable()) {
+    webgl->WarnIfImplicit(formatUsage->GetExtensionID());
+  }
 
   const auto format = formatUsage->format;
 

dom/canvas/WebGLTexture.cpp

@@ -775,6 +775,10 @@ void WebGLTexture::GenerateMipmap() {
     return;
   }
 
+  if (usage->IsRenderable() && !usage->IsExplicitlyRenderable()) {
+    mContext->WarnIfImplicit(usage->GetExtensionID());
+  }
+
   // Done with validation. Do the operation.
 
   gl::GLContext* gl = mContext->gl;

dom/canvas/WebGLTextureUpload.cpp

@@ -1670,12 +1670,14 @@ ScopedCopyTexImageSource::ScopedCopyTexImageSource(
       if (webgl->IsExtensionEnabled(
               WebGLExtensionID::WEBGL_color_buffer_float)) {
         sizedFormat = LOCAL_GL_RGBA32F;
+        webgl->WarnIfImplicit(WebGLExtensionID::WEBGL_color_buffer_float);
         break;
       }
 
       if (webgl->IsExtensionEnabled(
               WebGLExtensionID::EXT_color_buffer_half_float)) {
         sizedFormat = LOCAL_GL_RGBA16F;
+        webgl->WarnIfImplicit(WebGLExtensionID::EXT_color_buffer_half_float);
         break;
       }
       MOZ_CRASH("GFX: Should be able to request CopyTexImage from Float.");