WebGL 2: fix the bug in invalidate-framebuffer.html

third_party/WebKit/Source/modules/webgl/WebGL2RenderingContextBase.cpp

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "modules/webgl/WebGL2RenderingContextBase.h"
 
 #include "bindings/modules/v8/WebGLAny.h"
 #include "core/html/HTMLCanvasElement.h"
 #include "core/html/HTMLImageElement.h"
@@ skipping 357 matching lines @@
                 values[ii] = 0;
             webContext()->getInternalformativ(target, internalformat, GL_SAMPLES, length, values.get());
             return WebGLAny(scriptState, DOMInt32Array::create(values.get(), length));
         }
     default:
         synthesizeGLError(GL_INVALID_ENUM, "getInternalformatParameter", "invalid parameter name");
         return ScriptValue::createNull(scriptState);
     }
 }
 
+bool WebGL2RenderingContextBase::checkAndTranslateAttachments(const char* functionName, GLenum target, Vector<GLenum>& attachments)

[Ken Russell (switch to Gerrit), 2015/11/17 23:06:37]

It's a little nasty to modify the Vector which comes in from the JavaScript
bindings. I think it would be better to mark it as a const Vector& of GLenum in
WebGL2RenderingContextBase.h and here, and allocate a new vector with the
translated values.

[yunchao, 2015/11/19 01:43:00]

Done.

+{
+    GLsizei size = attachments.size();
+    GLenum* buffer = attachments.data();

[Ken Russell (switch to Gerrit), 2015/11/17 23:06:37]

It's not necessary to fetch the data pointer out of "attachments" -- just
reference attachments[i] directly.

[yunchao, 2015/11/19 01:42:59]

Done.

[yunchao, 2015/11/19 01:42:59]

Done.

+
+    WebGLFramebuffer* framebufferBinding = getFramebufferBinding(target);
+    ASSERT(framebufferBinding || drawingBuffer());
+    if (!framebufferBinding) {
+        // For the default framebuffer
+        // Translate GL_BACK/GL_DEPTH/GL_STENCIL, because the default framebuffer of WebGL is not fb 0, it is an internal fbo
+        for (GLsizei i = 0; i < size; ++i) {
+            switch (buffer[i]) {
+            case GL_BACK:

[yunchao, 2015/11/12 12:14:50]

In GLES spec, it is GL_COLOR, but in WebGL spec, it is GL_BACK (say in
drawBuffer). Both of them means GL_COLOR_ATTACHMENT0 of the internal fbo
(default framebuffer of WebGL).

I would like it is GL_BACK, then it is aligned with other APIs in WebGL spec.

[Ken Russell (switch to Gerrit), 2015/11/17 23:06:37]

Sorry, but no, this should follow the OpenGL ES specification. WebGL does not
deviate arbitrarily from the ES specs.

[yunchao, 2015/11/19 01:42:59]

Done.

+                buffer[i] = GL_COLOR_ATTACHMENT0;
+                break;
+            case GL_DEPTH:
+                buffer[i] = GL_DEPTH_ATTACHMENT;
+                break;
+            case GL_STENCIL:
+                buffer[i] = GL_STENCIL_ATTACHMENT;
+                break;
+            default:
+                synthesizeGLError(GL_INVALID_ENUM, functionName, "invalid attachment");
+                return false;
+            }
+        }
+    } else {
+        // For the FBO
+        for (GLsizei i = 0; i < size; ++i) {
+            switch (buffer[i]) {
+            case GL_COLOR_ATTACHMENT0:
+            case GL_DEPTH_ATTACHMENT:
+            case GL_STENCIL_ATTACHMENT:
+            case GL_DEPTH_STENCIL_ATTACHMENT:
+                break;
+            default:
+                if (buffer[i] > GL_COLOR_ATTACHMENT0
+                    && buffer[i] < static_cast<GLenum>(GL_COLOR_ATTACHMENT0 + maxColorAttachments()))
+                    break;
+                synthesizeGLError(GL_INVALID_OPERATION, functionName, "invalid attachment");
+                return false;
+            }
+        }
+    }
+    return true;
+}
+
 void WebGL2RenderingContextBase::invalidateFramebuffer(GLenum target, Vector<GLenum>& attachments)
 {
     if (isContextLost())
         return;
 
+    if (!validateFramebufferTarget(target)) {
+        synthesizeGLError(GL_INVALID_ENUM, "invalidateFramebuffer", "invalid target");
+        return;
+    }
+
+    if (!checkAndTranslateAttachments("invalidateFramebuffe", target, attachments))

[Ken Russell (switch to Gerrit), 2015/11/17 23:06:37]

invalidateFramebuffe -> invalidateFramebuffer

[yunchao, 2015/11/19 01:42:59]

Done.

+        return;
+
     webContext()->invalidateFramebuffer(target, attachments.size(), attachments.data());
 }
 
 void WebGL2RenderingContextBase::invalidateSubFramebuffer(GLenum target, Vector<GLenum>& attachments, GLint x, GLint y, GLsizei width, GLsizei height)
 {
     if (isContextLost())
         return;
 
+    if (!validateFramebufferTarget(target)) {
+        synthesizeGLError(GL_INVALID_ENUM, "invalidateFramebuffer", "invalid target");
+        return;
+    }
+
+    if (width < 0 || height < 0) {

[yunchao, 2015/11/12 12:14:51]

Not sure about this code snippet, In OGL spec, it will raise a GL_INVALID_VALUE
for this case, but it seems be OK in GLES spec.

[Ken Russell (switch to Gerrit), 2015/11/17 23:06:37]

In this case it seems better to be safer by generating INVALID_VALUE as is
already tested by conformance2/renderbuffers/invalidate-framebuffer.html.
Otherwise it'll be necessary to clamp these to 0 to avoid INVALID_VALUE errors
from OpenGL drivers.

[yunchao, 2015/11/19 01:43:00]

OK, let's generate GL_INVALID_ENUM for this case.

+        synthesizeGLError(GL_INVALID_VALUE, "invalidateSubFramebuffer", "invalid width or height");
+        return;
+    }
+
+    if (!checkAndTranslateAttachments("invalidateSubFramebuffe", target, attachments))

[Ken Russell (switch to Gerrit), 2015/11/17 23:06:37]

invalidateSubFramebuffe -> invalidateSubFramebuffer

[yunchao, 2015/11/19 01:43:00]

Done.

+        return;
+
     webContext()->invalidateSubFramebuffer(target, attachments.size(), attachments.data(), x, y, width, height);
 }
 
 void WebGL2RenderingContextBase::readBuffer(GLenum mode)
 {
     if (isContextLost())
         return;
 
     switch (mode) {
     case GL_BACK:
@@ skipping 2681 matching lines @@
 GLenum WebGL2RenderingContextBase::boundFramebufferColorFormat()
 {
     if (m_readFramebufferBinding && m_readFramebufferBinding->object())
         return m_readFramebufferBinding->colorBufferFormat();
     if (m_requestedAttributes.alpha())
         return GL_RGBA;
     return GL_RGB;
 }
 
 } // namespace blink