Bug 1638568 - E.g. uniform1i(null, ...) should check avail len not total len. r=lsalzman

Also:
* Propagate null-location UniformDatas to host
* Move transpose validation to host-side

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

dom/canvas/ClientWebGLContext.cpp

@@ -4262,11 +4262,6 @@ void ClientWebGLContext::UniformData(const GLenum funcElemType,
   const FuncScope funcScope(*this, "uniform setter");
   if (IsContextLost()) return;
 
-  if (!mIsWebGL2 && transpose) {
-    EnqueueError(LOCAL_GL_INVALID_VALUE, "`transpose`:true requires WebGL 2.");
-    return;
-  }
-
   const auto& activeLinkResult = GetActiveLinkResult();
   if (!activeLinkResult) {
     EnqueueError(LOCAL_GL_INVALID_OPERATION, "No active linked Program.");
@@ -4292,56 +4287,57 @@ void ClientWebGLContext::UniformData(const GLenum funcElemType,
 
   // -
 
-  if (!loc) {
+  const auto channels = ElemTypeComponents(funcElemType);
-    // We need to catch INVALID_VALUEs from bad-sized `bytes`. :S
+  if (!availCount || availCount % channels != 0) {
-    // For non-null `loc`, the Host side handles this safely.
+    EnqueueError(LOCAL_GL_INVALID_VALUE,
-    const auto lengthInType = bytes.length() / sizeof(float);
+                 "`values` length (%u) must be a positive "
-    const auto channels = ElemTypeComponents(funcElemType);
+                 "integer multiple of size of %s.",
-    if (!lengthInType || lengthInType % channels != 0) {
+                 availCount, EnumString(funcElemType).c_str());
-      EnqueueError(LOCAL_GL_INVALID_VALUE,
+    return;
-                   "`values` length (%u) must be a positive integer multiple "
+  }
-                   "of size of %s.",
+
-                   lengthInType, EnumString(funcElemType).c_str());
+  // -
+
+  uint32_t locId = -1;
+  if (MOZ_LIKELY(loc)) {
+    locId = loc->mLocation;
+    if (!loc->ValidateUsable(*this, "location")) return;
+
+    // -
+
+    const auto& reqLinkInfo = loc->mParent.lock();
+    if (reqLinkInfo.get() != activeLinkResult) {
+      EnqueueError(LOCAL_GL_INVALID_OPERATION,
+                   "UniformLocation is not from the current active Program.");
       return;
     }
-    return;  // All that validation for a no-op!
-  }
-  if (!loc->ValidateUsable(*this, "location")) return;
 
-  // -
+    // -
 
-  const auto& reqLinkInfo = loc->mParent.lock();
+    bool funcMatchesLocation = false;
-  if (reqLinkInfo.get() != activeLinkResult) {
-    EnqueueError(LOCAL_GL_INVALID_OPERATION,
-                 "UniformLocation is not from the current active Program.");
-    return;
-  }
-
-  // -
-
-  bool funcMatchesLocation = false;
-  for (const auto allowed : loc->mValidUploadElemTypes) {
-    funcMatchesLocation |= (funcElemType == allowed);
-  }
-  if (MOZ_UNLIKELY(!funcMatchesLocation)) {
-    std::string validSetters;
     for (const auto allowed : loc->mValidUploadElemTypes) {
-      validSetters += EnumString(allowed);
+      funcMatchesLocation |= (funcElemType == allowed);
-      validSetters += '/';
     }
-    validSetters.pop_back();  // Cheekily discard the extra trailing '/'.
+    if (MOZ_UNLIKELY(!funcMatchesLocation)) {
+      std::string validSetters;
+      for (const auto allowed : loc->mValidUploadElemTypes) {
+        validSetters += EnumString(allowed);
+        validSetters += '/';
+      }
+      validSetters.pop_back();  // Cheekily discard the extra trailing '/'.
 
-    EnqueueError(LOCAL_GL_INVALID_OPERATION,
+      EnqueueError(LOCAL_GL_INVALID_OPERATION,
-                 "Uniform's `type` requires uniform setter of type %s.",
+                   "Uniform's `type` requires uniform setter of type %s.",
-                 validSetters.c_str());
+                   validSetters.c_str());
-    return;
+      return;
+    }
   }
 
   // -
 
   const auto ptr = bytes.begin().get() + (elemOffset * sizeof(float));
   const auto range = Range<const uint8_t>{ptr, availCount * sizeof(float)};
-  Run<RPROC(UniformData)>(loc->mLocation, transpose, RawBuffer<>(range));
+  Run<RPROC(UniformData)>(locId, transpose, RawBuffer<>(range));
 }
 
 // -

dom/canvas/WebGLContextGL.cpp

@@ -1288,11 +1288,22 @@ void WebGLContext::StencilOpSeparate(GLenum face, GLenum sfail, GLenum dpfail,
 void WebGLContext::UniformData(const uint32_t loc, const bool transpose,
                                const Range<const uint8_t>& data) const {
   const FuncScope funcScope(*this, "uniform setter");
+
+  if (!IsWebGL2() && transpose) {
+    GenerateError(LOCAL_GL_INVALID_VALUE, "`transpose`:true requires WebGL 2.");
+    return;
+  }
+
+  // -
+
   const auto& link = mActiveProgramLinkInfo;
   if (!link) return;
 
   const auto locInfo = MaybeFind(link->locationMap, loc);
-  if (!locInfo) return;
+  if (!locInfo) {
+    // Null WebGLUniformLocations become -1, which will end up here.
+    return;
+  }
 
   const auto& validationInfo = locInfo->info;
   const auto& activeInfo = validationInfo.info;
@@ -1302,20 +1313,11 @@ void WebGLContext::UniformData(const uint32_t loc, const bool transpose,
   // -
 
   const auto lengthInType = data.length() / sizeof(float);
-  if (!lengthInType || lengthInType % channels != 0) {
-    GenerateError(LOCAL_GL_INVALID_VALUE,
-                  "(uniform %s) `values` length (%u) must be a positive "
-                  "integer multiple of "
-                  "size of %s.",
-                  activeInfo.name.c_str(), lengthInType,
-                  EnumString(activeInfo.elemType).c_str());
-    return;
-  }
   const auto elemCount = lengthInType / channels;
   if (elemCount > 1 && !validationInfo.isArray) {
     GenerateError(
         LOCAL_GL_INVALID_OPERATION,
-        "(uniform %s) `values` length (%u) must exactle match size of %s.",
+        "(uniform %s) `values` length (%u) must exactly match size of %s.",
         activeInfo.name.c_str(), lengthInType,
         EnumString(activeInfo.elemType).c_str());
     return;