Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(978)

Unified Diff: third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp

Issue 2407753002: Fix crash from state management cleanup (Closed)
Patch Set: Fix state restore missed Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
diff --git a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
index 2844f45682b8ab3a4195f816f74bd025998e89e4..0a53aa1b67f1f18adcb8f1dcecffad57654b781e 100644
--- a/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
+++ b/third_party/WebKit/Source/modules/webgl/WebGLRenderingContextBase.cpp
@@ -714,7 +714,7 @@ PassRefPtr<Image> WebGLRenderingContextBase::getImage(
if (!drawingBuffer())
return nullptr;
- drawingBuffer()->commit();
+ drawingBuffer()->resolveAndBindForReadAndDraw();
IntSize size = clampedCanvasSize();
OpacityMode opacityMode =
creationAttributes().hasAlpha() ? NonOpaque : Opaque;
@@ -1053,7 +1053,7 @@ PassRefPtr<DrawingBuffer> WebGLRenderingContextBase::createDrawingBuffer(
NOTREACHED();
}
return DrawingBuffer::create(
- std::move(contextProvider), clampedCanvasSize(), premultipliedAlpha,
+ std::move(contextProvider), this, clampedCanvasSize(), premultipliedAlpha,
wantAlphaChannel, wantDepthBuffer, wantStencilBuffer, wantAntialiasing,
preserve, webGLVersion, chromiumImageUsage);
}
@@ -1084,12 +1084,10 @@ void WebGLRenderingContextBase::initializeNewContext() {
m_numGLErrorsToConsoleAllowed = maxGLErrorsAllowedToConsole;
m_clearColor[0] = m_clearColor[1] = m_clearColor[2] = m_clearColor[3] = 0;
- drawingBuffer()->setClearColor(m_clearColor);
m_scissorEnabled = false;
m_clearDepth = 1;
m_clearStencil = 0;
m_colorMask[0] = m_colorMask[1] = m_colorMask[2] = m_colorMask[3] = true;
- drawingBuffer()->setColorMask(m_colorMask);
GLint numCombinedTextureImageUnits = 0;
contextGL()->GetIntegerv(GL_MAX_COMBINED_TEXTURE_IMAGE_UNITS,
@@ -1376,8 +1374,11 @@ WebGLRenderingContextBase::clearIfComposited(GLbitfield mask) {
!drawingBuffer()->defaultBufferRequiresAlphaChannelToBePreserved());
drawingBuffer()->clearFramebuffers(clearMask);
- restoreStateAfterClear();
- drawingBuffer()->restoreFramebufferBindings();
+ // Call the DrawingBufferClient method to restore scissor test, mask, and
+ // clear values, because we dirtied them above.
+ DrawingBufferClientRestoreScissorTest();
+ DrawingBufferClientRestoreMaskAndClearValues();
+
drawingBuffer()->setBufferClearNeeded(false);
return combinedClear ? CombinedClear : JustClear;
@@ -1410,27 +1411,6 @@ void WebGLRenderingContextBase::restoreClearColor() {
m_clearColor[3]);
}
-void WebGLRenderingContextBase::restoreClearDepthf() {
- if (isContextLost())
- return;
-
- contextGL()->ClearDepthf(m_clearDepth);
-}
-
-void WebGLRenderingContextBase::restoreClearStencil() {
- if (isContextLost())
- return;
-
- contextGL()->ClearStencil(m_clearStencil);
-}
-
-void WebGLRenderingContextBase::restoreStencilMaskSeparate() {
- if (isContextLost())
- return;
-
- contextGL()->StencilMaskSeparate(GL_FRONT, m_stencilMask);
-}
-
void WebGLRenderingContextBase::restoreColorMask() {
if (isContextLost())
return;
@@ -1439,24 +1419,6 @@ void WebGLRenderingContextBase::restoreColorMask() {
m_colorMask[3]);
}
-void WebGLRenderingContextBase::restoreDepthMask() {
- if (isContextLost())
- return;
-
- contextGL()->DepthMask(m_depthMask);
-}
-
-void WebGLRenderingContextBase::restoreStateAfterClear() {
- // Restore clear-related state items back to what the context had set.
- restoreScissorEnabled();
- restoreClearColor();
- restoreColorMask();
- restoreClearDepthf();
- restoreClearStencil();
- restoreStencilMaskSeparate();
- restoreDepthMask();
-}
-
void WebGLRenderingContextBase::markLayerComposited() {
if (!isContextLost())
drawingBuffer()->setBufferClearNeeded(true);
@@ -1492,7 +1454,7 @@ bool WebGLRenderingContextBase::paintRenderingResultsToCanvas(
ScopedTexture2DRestorer restorer(this);
ScopedFramebufferRestorer fboRestorer(this);
- drawingBuffer()->commit();
+ drawingBuffer()->resolveAndBindForReadAndDraw();
if (!canvas()->buffer()->copyRenderingResultsFromDrawingBuffer(
drawingBuffer(), sourceBuffer)) {
// Currently, copyRenderingResultsFromDrawingBuffer is expected to always
@@ -1514,7 +1476,7 @@ ImageData* WebGLRenderingContextBase::paintRenderingResultsToImageData(
return nullptr;
clearIfComposited();
- drawingBuffer()->commit();
+ drawingBuffer()->resolveAndBindForReadAndDraw();
ScopedFramebufferRestorer restorer(this);
int width, height;
WTF::ArrayBufferContents contents;
@@ -1562,17 +1524,7 @@ void WebGLRenderingContextBase::reshape(int width, int height) {
// We don't have to mark the canvas as dirty, since the newly created image
// buffer will also start off clear (and this matches what reshape will do).
- drawingBuffer()->reset(IntSize(width, height));
- restoreStateAfterClear();
-
- contextGL()->BindTexture(
- GL_TEXTURE_2D,
- objectOrZero(
- m_textureUnits[m_activeTextureUnit].m_texture2DBinding.get()));
- contextGL()->BindRenderbuffer(GL_RENDERBUFFER,
- objectOrZero(m_renderbufferBinding.get()));
- drawingBuffer()->restoreFramebufferBindings();
- drawingBuffer()->restorePixelUnpackBufferBindings();
+ drawingBuffer()->resize(IntSize(width, height));
}
int WebGLRenderingContextBase::drawingBufferWidth() const {
@@ -1593,8 +1545,6 @@ void WebGLRenderingContextBase::activeTexture(GLenum texture) {
}
m_activeTextureUnit = texture - GL_TEXTURE0;
contextGL()->ActiveTexture(texture);
-
- drawingBuffer()->setActiveTextureUnit(texture);
}
void WebGLRenderingContextBase::attachShader(WebGLProgram* program,
@@ -1686,10 +1636,6 @@ void WebGLRenderingContextBase::bindBuffer(GLenum target, WebGLBuffer* buffer) {
}
if (!validateAndUpdateBufferBindTarget("bindBuffer", target, buffer))
return;
-
- if (target == GL_PIXEL_UNPACK_BUFFER) {
- drawingBuffer()->setPixelUnpackBufferBinding(objectOrZero(buffer));
- }
contextGL()->BindBuffer(target, objectOrZero(buffer));
}
@@ -1729,9 +1675,6 @@ void WebGLRenderingContextBase::bindRenderbuffer(
}
m_renderbufferBinding = renderBuffer;
contextGL()->BindRenderbuffer(target, objectOrZero(renderBuffer));
-
- drawingBuffer()->setRenderbufferBinding(objectOrZero(renderBuffer));
-
if (renderBuffer)
renderBuffer->setHasEverBeenBound();
}
@@ -1754,9 +1697,6 @@ void WebGLRenderingContextBase::bindTexture(GLenum target,
if (target == GL_TEXTURE_2D) {
m_textureUnits[m_activeTextureUnit].m_texture2DBinding = texture;
-
- if (!m_activeTextureUnit)
- drawingBuffer()->setTexture2DBinding(objectOrZero(texture));
} else if (target == GL_TEXTURE_CUBE_MAP) {
m_textureUnits[m_activeTextureUnit].m_textureCubeMapBinding = texture;
} else if (isWebGL2OrHigher() && target == GL_TEXTURE_2D_ARRAY) {
@@ -2018,7 +1958,6 @@ void WebGLRenderingContextBase::clearColor(GLfloat r,
m_clearColor[1] = g;
m_clearColor[2] = b;
m_clearColor[3] = a;
- drawingBuffer()->setClearColor(m_clearColor);
contextGL()->ClearColor(r, g, b, a);
}
@@ -2046,7 +1985,6 @@ void WebGLRenderingContextBase::colorMask(GLboolean red,
m_colorMask[1] = green;
m_colorMask[2] = blue;
m_colorMask[3] = alpha;
- drawingBuffer()->setColorMask(m_colorMask);
contextGL()->ColorMask(red, green, blue, alpha);
}
@@ -2257,10 +2195,8 @@ bool WebGLRenderingContextBase::deleteObject(WebGLObject* object) {
}
void WebGLRenderingContextBase::deleteBuffer(WebGLBuffer* buffer) {
- GLuint bufferName = objectOrZero(buffer);
if (!deleteObject(buffer))
return;
- drawingBuffer()->notifyBufferDeleted(bufferName);
removeBoundBuffer(buffer);
}
@@ -2270,7 +2206,6 @@ void WebGLRenderingContextBase::deleteFramebuffer(
return;
if (framebuffer == m_framebufferBinding) {
m_framebufferBinding = nullptr;
- drawingBuffer()->setFramebufferBinding(GL_FRAMEBUFFER, 0);
// Have to call drawingBuffer()->bind() here to bind back to internal fbo.
drawingBuffer()->bind(GL_FRAMEBUFFER);
}
@@ -2288,7 +2223,6 @@ void WebGLRenderingContextBase::deleteRenderbuffer(
return;
if (renderbuffer == m_renderbufferBinding) {
m_renderbufferBinding = nullptr;
- drawingBuffer()->setRenderbufferBinding(0);
}
if (m_framebufferBinding)
m_framebufferBinding->removeAttachmentFromBoundFramebuffer(GL_FRAMEBUFFER,
@@ -2312,8 +2246,6 @@ void WebGLRenderingContextBase::deleteTexture(WebGLTexture* texture) {
if (texture == m_textureUnits[i].m_texture2DBinding) {
m_textureUnits[i].m_texture2DBinding = nullptr;
maxBoundTextureIndex = i;
- if (!i)
- drawingBuffer()->setTexture2DBinding(0);
}
if (texture == m_textureUnits[i].m_textureCubeMapBinding) {
m_textureUnits[i].m_textureCubeMapBinding = nullptr;
@@ -2391,10 +2323,8 @@ void WebGLRenderingContextBase::disable(GLenum cap) {
applyStencilTest();
return;
}
- if (cap == GL_SCISSOR_TEST) {
+ if (cap == GL_SCISSOR_TEST)
m_scissorEnabled = false;
- drawingBuffer()->setScissorEnabled(m_scissorEnabled);
- }
contextGL()->Disable(cap);
}
@@ -2530,10 +2460,8 @@ void WebGLRenderingContextBase::enable(GLenum cap) {
applyStencilTest();
return;
}
- if (cap == GL_SCISSOR_TEST) {
+ if (cap == GL_SCISSOR_TEST)
m_scissorEnabled = true;
- drawingBuffer()->setScissorEnabled(m_scissorEnabled);
- }
contextGL()->Enable(cap);
}
@@ -3934,7 +3862,6 @@ void WebGLRenderingContextBase::pixelStorei(GLenum pname, GLint param) {
if (param == 1 || param == 2 || param == 4 || param == 8) {
if (pname == GL_PACK_ALIGNMENT) {
m_packAlignment = param;
- drawingBuffer()->setPackAlignment(param);
} else { // GL_UNPACK_ALIGNMENT:
m_unpackAlignment = param;
}
@@ -5953,12 +5880,6 @@ void WebGLRenderingContextBase::loseContextImpl(
ASSERT(m_contextLostMode != NotLostContext);
m_autoRecoveryMethod = autoRecoveryMethod;
- // Make absolutely sure we do not refer to an already-deleted texture or
- // framebuffer.
- drawingBuffer()->setTexture2DBinding(0);
- drawingBuffer()->setFramebufferBinding(GL_FRAMEBUFFER, 0);
- drawingBuffer()->setRenderbufferBinding(0);
-
detachAndRemoveAllObjects();
// Lose all the extensions.
@@ -6115,6 +6036,62 @@ void WebGLRenderingContextBase::stop() {
}
}
+bool WebGLRenderingContextBase::DrawingBufferClientIsBoundForDraw() {
+ return !m_framebufferBinding;
+}
+
+void WebGLRenderingContextBase::DrawingBufferClientRestoreScissorTest() {
+ if (!contextGL())
+ return;
+ if (m_scissorEnabled)
+ contextGL()->Enable(GL_SCISSOR_TEST);
+ else
+ contextGL()->Disable(GL_SCISSOR_TEST);
+}
+
+void WebGLRenderingContextBase::DrawingBufferClientRestoreMaskAndClearValues() {
+ if (!contextGL())
+ return;
+ contextGL()->ColorMask(m_colorMask[0], m_colorMask[1], m_colorMask[2],
+ m_colorMask[3]);
+ contextGL()->DepthMask(m_depthMask);
+ contextGL()->StencilMaskSeparate(GL_FRONT, m_stencilMask);
+
+ contextGL()->ClearColor(m_clearColor[0], m_clearColor[1], m_clearColor[2],
+ m_clearColor[3]);
+ contextGL()->ClearDepthf(m_clearDepth);
+ contextGL()->ClearStencil(m_clearStencil);
+}
+
+void WebGLRenderingContextBase::DrawingBufferClientRestorePixelPackAlignment() {
+ if (!contextGL())
+ return;
+ contextGL()->PixelStorei(GL_PACK_ALIGNMENT, m_packAlignment);
+}
+
+void WebGLRenderingContextBase::DrawingBufferClientRestoreTexture2DBinding() {
+ if (!contextGL())
+ return;
+ restoreCurrentTexture2D();
+}
+
+void WebGLRenderingContextBase::
+ DrawingBufferClientRestoreRenderbufferBinding() {
+ if (!contextGL())
+ return;
+ contextGL()->BindRenderbuffer(GL_RENDERBUFFER,
+ objectOrZero(m_renderbufferBinding.get()));
+}
+
+void WebGLRenderingContextBase::DrawingBufferClientRestoreFramebufferBinding() {
+ if (!contextGL())
+ return;
+ restoreCurrentFramebuffer();
+}
+
+void WebGLRenderingContextBase::
+ DrawingBufferClientRestorePixelUnpackBufferBinding() {}
+
ScriptValue WebGLRenderingContextBase::getBooleanParameter(
ScriptState* scriptState,
GLenum pname) {
@@ -7408,9 +7385,6 @@ void WebGLRenderingContextBase::setFramebuffer(GLenum target,
m_framebufferBinding = buffer;
applyStencilTest();
}
- drawingBuffer()->setFramebufferBinding(
- target, objectOrZero(getFramebufferBinding(target)));
-
if (!buffer) {
// Instead of binding fb 0, bind the drawing buffer.
drawingBuffer()->bind(target);
@@ -7424,8 +7398,10 @@ void WebGLRenderingContextBase::restoreCurrentFramebuffer() {
}
void WebGLRenderingContextBase::restoreCurrentTexture2D() {
- bindTexture(GL_TEXTURE_2D,
- m_textureUnits[m_activeTextureUnit].m_texture2DBinding.get());
+ if (m_activeTextureUnit < m_textureUnits.size()) {
+ bindTexture(GL_TEXTURE_2D,
+ m_textureUnits[m_activeTextureUnit].m_texture2DBinding.get());
+ }
}
void WebGLRenderingContextBase::findNewMaxNonDefaultTextureUnit() {

Powered by Google App Engine
This is Rietveld 408576698