Fix base level for generateMipmap in blink

Source/modules/webgl/WebGLTexture.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 196 matching lines @@
     m_immutable = true;
 }
 
 void WebGLTexture::generateMipmapLevelInfo()
 {
     if (!object() || !m_target)
         return;
     if (!canGenerateMipmaps())
         return;
     if (!m_isComplete) {
-        size_t baseLevel = std::min(m_baseLevel, m_info[0].size() - 1);
         for (size_t ii = 0; ii < m_info.size(); ++ii) {
-            const LevelInfo& info0 = m_info[ii][baseLevel];
+            const LevelInfo& info0 = m_info[ii][m_baseLevel];
             GLsizei width = info0.width;
             GLsizei height = info0.height;
             GLsizei depth = info0.depth;
             GLint levelCount = computeLevelCount(width, height, depth);
-            size_t maxLevel = m_isWebGL2OrHigher ? std::min(m_maxLevel, baseLevel + levelCount - 1) : baseLevel + levelCount - 1;
-            for (size_t level = baseLevel + 1; level <= maxLevel; ++level) {
+            size_t maxLevel = m_isWebGL2OrHigher ? std::min(m_maxLevel, m_baseLevel + levelCount - 1) : m_baseLevel + levelCount - 1;

[Ken Russell (switch to Gerrit), 2015/09/09 01:30:26]

Is this code correct? It looks to me like it's possible for m_maxLevel to be set
to an out of range value (less than m_baseLevel), since update() has determined
that the texture is incomplete -- possibly for exactly this reason. It looks to
me like the value has to be clamped, rather than set to the result of std::min.

[Zhenyao Mo, 2015/09/09 01:34:48]

in canGenerateMipmaps() the case where m_maxLevel < m_baseLevel is already
rejected.  So here we can safely assume m_baseLevel <= m_maxLevel.  Maybe adding
an ASSERT() to make it apparent.

[Ken Russell (switch to Gerrit), 2015/09/09 01:59:33]

Sorry, could you please point me to that code? Looking at
WebGLTexture::canGenerateMipmaps below I don't see any examination of
m_maxLevel. It is looked at in WebGLTexture::update() but if that condition
holds true then the texture's marked incomplete, which causes this code path to
be taken.

[qiankun, 2015/09/09 02:09:32]

As we discussed in https://github.com/KhronosGroup/WebGL/pull/1177, it doesn't
require GenerateMipmap produce an error if level_max < levle_base. The spec
doesn't define this behavior.

[Ken Russell (switch to Gerrit), 2015/09/09 02:14:19]

The question is what the loop below will do if maxLevel is out of range, and
what the state of the texture will be afterward. It looks to me like at best it
will short-circuit the loop and set m_isComplete to true, which seems like wrong
behavior. I was concerned that it would perform out-of-range accesses on
m_info[ii]. Asserts would go a long way toward helping prove the code won't
crash.

[Zhenyao Mo, 2015/09/09 02:27:19]

My bad.  The check is in update(), unrelated to generateMipmap().

The value actually might go out of range, because m_info[ii] is of size
m_maxTextureLevel or m_maxCubeMapTextureLevel, however, compueteLevelCount could
also return the same value if width/height/depth are each at its maximum value. 
So if m_baseLevel sets a value larger than 0, and m_maxLevel also is set to a
huge value, then min() is still out of range.

[qiankun, 2015/09/09 04:36:06]

WebGLRenderingContextBase::validateTexFuncDimensions() will make sure each level
has its owner max size, e.g. 16384 for level 0, 16384>>level for other levels.
So maxLevel will not exceed m_info[ii].size().

+            for (size_t level = m_baseLevel + 1; level <= maxLevel; ++level) {
                 width = std::max(1, width >> 1);
                 height = std::max(1, height >> 1);
                 depth = std::max(1, depth >> 1);
                 LevelInfo& info = m_info[ii][level];
                 info.setInfo(info0.internalFormat, width, height, depth, info0.type);
             }
         }
         m_isComplete = true;
     }
     m_needToUseBlackTexture = false;
@@ skipping 111 matching lines @@
             return 0;
     }
     return -1;
 }
 
 bool WebGLTexture::canGenerateMipmaps()
 {
     if (!m_isWebGL2OrHigher && isNPOT())
         return false;
 
-    size_t baseLevel = std::min(m_baseLevel, m_info[0].size() - 1);
-    const LevelInfo& base = m_info[0][baseLevel];
+    if (m_baseLevel >= m_info[0].size())
+        return false;
+    const LevelInfo& base = m_info[0][m_baseLevel];
     for (size_t ii = 0; ii < m_info.size(); ++ii) {
-        const LevelInfo& info = m_info[ii][baseLevel];
+        const LevelInfo& info = m_info[ii][m_baseLevel];
         if (!info.valid
             || info.width != base.width || info.height != base.height || info.depth != base.depth
             || info.internalFormat != base.internalFormat || info.type != base.type
             || (m_info.size() > 1 && !m_isCubeComplete))
             return false;
     }
     return true;
 }
 
 GLint WebGLTexture::computeLevelCount(GLsizei width, GLsizei height, GLsizei depth)
@@ skipping 21 matching lines @@
     m_isNPOT = false;
     for (size_t ii = 0; ii < m_info.size(); ++ii) {
         if (isNPOT(m_info[ii][0].width, m_info[ii][0].height)) {
             m_isNPOT = true;
             break;
         }
     }
     m_isComplete = true;
     m_isCubeComplete = true;
 
-    size_t baseLevel = std::min(m_baseLevel, m_info[0].size() - 1);
-    const LevelInfo& base = m_info[0][baseLevel];
-    size_t levelCount = computeLevelCount(base.width, base.height, base.depth);
-    size_t maxLevel = m_isWebGL2OrHigher ? std::min(m_maxLevel, baseLevel + levelCount - 1) : baseLevel + levelCount - 1;
-
-    if (baseLevel > maxLevel) {
+    if (m_baseLevel > m_maxLevel || m_baseLevel >= m_info[0].size()) {
         m_isComplete = false;
     }
     else {
+        const LevelInfo& base = m_info[0][m_baseLevel];
+        size_t levelCount = computeLevelCount(base.width, base.height, base.depth);
+        size_t maxLevel = m_isWebGL2OrHigher ? std::min(m_maxLevel, m_baseLevel + levelCount - 1) : m_baseLevel + levelCount - 1;
         for (size_t ii = 0; ii < m_info.size() && m_isComplete; ++ii) {
-            const LevelInfo& info0 = m_info[ii][baseLevel];
+            const LevelInfo& info0 = m_info[ii][m_baseLevel];
             if (!info0.valid
                 || info0.width != base.width || info0.height != base.height || info0.depth != base.depth
                 || info0.internalFormat != base.internalFormat || info0.type != base.type
                 || (m_info.size() > 1 && info0.width != info0.height)) {
                 if (m_info.size() > 1)
                     m_isCubeComplete = false;
                 m_isComplete = false;
                 break;
             }
             GLsizei width = info0.width;
             GLsizei height = info0.height;
             GLsizei depth = info0.depth;
-            for (size_t level = baseLevel + 1; level <= maxLevel; ++level) {
+            for (size_t level = m_baseLevel + 1; level <= maxLevel; ++level) {
                 width = std::max(1, width >> 1);
                 height = std::max(1, height >> 1);
                 depth = std::max(1, depth >> 1);
                 const LevelInfo& info = m_info[ii][level];
                 if (!info.valid
                     || info.width != width || info.height != height || info.depth != depth
                     || info.internalFormat != info0.internalFormat || info.type != info0.type) {
                     m_isComplete = false;
                     break;
                 }
@@ skipping 154 matching lines @@
     case GL_COMPRESSED_RGBA_S3TC_DXT1_EXT:
     case GL_COMPRESSED_RGBA_S3TC_DXT3_EXT:
     case GL_COMPRESSED_RGBA_S3TC_DXT5_EXT:
         return GL_UNSIGNED_BYTE;
     default:
         return GL_NONE;
     }
 }
 
 } // namespace blink