37174 – Must resolve multisampled back buffer during copyTexImage2D and copyTexSubImage2D

RESOLVED FIXED Bug 37174

Must resolve multisampled back buffer during copyTexImage2D and copyTexSubImage2D

https://bugs.webkit.org/show_bug.cgi?id=37174

Summary Must resolve multisampled back buffer during copyTexImage2D and copyTexSubIma...

Kenneth Russell

Reported 2010-04-06 15:39:47 PDT

If the WebGL back buffer is multisampled (the antialias parameter is set to true during context creation), then during both copyTexImage2D and copyTexSubImage2D operations, it is necessary to resolve the framebuffer, bind the resolved version of the framebuffer, and then perform the operation. Afterward, the multisampled framebuffer should be re-bound. Currently these operations are raising an INVALID_OPERATION error because of the attempt to copy directly from the multisampled framebuffer to the texture.

Attachments
patch (14.70 KB, patch) 2010-04-06 18:12 PDT, Zhenyao Mo	no flags	Details Formatted Diff Diff
revised patch: responding to Ken Russell's review (15.16 KB, patch) 2010-04-07 14:31 PDT, Zhenyao Mo	no flags	Details Formatted Diff Diff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Zhenyao Mo

Comment 1 2010-04-06 18:12:51 PDT

Created attachment 52690 [details] patch

Kenneth Russell

Comment 2 2010-04-07 14:21:01 PDT

Comment on attachment 52690 [details] patch The code looks good. One comment on the test case. > Index: LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d.html > =================================================================== > --- LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d.html (revision 0) > +++ LayoutTests/fast/canvas/webgl/copy-tex-image-and-sub-image-2d.html (revision 0) > @@ -0,0 +1,185 @@ > +<html> > +<head> > +<link rel="stylesheet" href="../../js/resources/js-test-style.css"/> > +<script src="../../js/resources/js-test-pre.js"></script> > +<script src="resources/webgl-test.js"></script> > +<script src="resources/utils3d.js"> </script> > +<script id="vshader" type="x-shader/x-vertex"> > +attribute vec3 g_Position; > +attribute vec2 g_TexCoord0; > + > +varying vec2 texCoord; > + > +void main() > +{ > + gl_Position = vec4(g_Position.x, g_Position.y, g_Position.z, 1.0); > + texCoord = g_TexCoord0; > +} > +</script> > + > +<script id="fshader" type="x-shader/x-fragment"> > +uniform sampler2D tex; > +varying vec2 texCoord; > + > +void main() > +{ > + gl_FragColor = texture2D(tex, texCoord); > +} > +</script> > + > +<script> > +var successfullyParsed = false; > + > +function init() > +{ > + if (window.layoutTestController) { > + layoutTestController.overridePreference("WebKitWebGLEnabled", "1"); > + layoutTestController.dumpAsText(); > + layoutTestController.waitUntilDone(); > + } > + > + description('Verify copyTexImage2D and copyTexSubImage2D'); > + > + runTest(); > +} > + > +// These two declarations need to be global for "shouldBe" to see them > +var pixel = [0, 0, 0]; > +var correctColor = null; > + > +function runTestIteration(antialias) > +{ > + var gl = null; > + if (antialias) > + gl = initWebGL("antialiasOn", "vshader", "fshader", [ "g_Position", "g_TexCoord0" ], [ 0, 0, 0, 1 ], 1); > + else > + gl = initWebGL("antialiasOff", "vshader", "fshader", [ "g_Position", "g_TexCoord0" ], [ 0, 0, 0, 1 ], 1, { antialias: false }); > + > + var textureLoc = gl.getUniformLocation(gl.program, "tex"); > + > + var vertices = new WebGLFloatArray([ > + 1.0, 1.0, 0.0, > + -1.0, 1.0, 0.0, > + -1.0, -1.0, 0.0, > + 1.0, 1.0, 0.0, > + -1.0, -1.0, 0.0, > + 1.0, -1.0, 0.0]); > + var texCoords = new WebGLFloatArray([ > + 1.0, 1.0, > + 0.0, 1.0, > + 0.0, 0.0, > + 1.0, 1.0, > + 0.0, 0.0, > + 1.0, 0.0]); > + var texCoordOffset = vertices.byteLength; > + > + var vbo = gl.createBuffer(); > + gl.bindBuffer(gl.ARRAY_BUFFER, vbo); > + gl.bufferData(gl.ARRAY_BUFFER, > + texCoordOffset + texCoords.byteLength, > + gl.STATIC_DRAW); > + gl.bufferSubData(gl.ARRAY_BUFFER, 0, vertices); > + gl.bufferSubData(gl.ARRAY_BUFFER, texCoordOffset, texCoords); > + > + gl.enableVertexAttribArray(0); > + gl.vertexAttribPointer(0, 3, gl.FLOAT, false, 0, 0); > + gl.enableVertexAttribArray(1); > + gl.vertexAttribPointer(1, 2, gl.FLOAT, gl.FALSE, 0, texCoordOffset); > + > + gl.colorMask(1, 1, 1, 0); > + gl.disable(gl.BLEND); > + debug('Testing copyTexImage2D'); > + > + // Red canvas > + gl.clearColor(1, 0, 0, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); > + > + var texture = gl.createTexture(); > + // Bind the texture to texture unit 0 > + gl.bindTexture(gl.TEXTURE_2D, texture); > + // Set up texture > + gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 2, 2, 0, gl.RGBA, gl.UNSIGNED_BYTE, null); > + gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST); > + gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST); I think there should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. > + gl.copyTexImage2D(gl.TEXTURE_2D, 0, gl.RGBA, 0, 0, 2, 2, 0); There should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. Previously this was generating INVALID_OPERATION. > + > + // Green canvas > + gl.clearColor(0, 1, 0, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); > + > + // Point the uniform sampler to texture unit 0 > + gl.uniform1i(textureLoc, 0); > + // Draw the triangles > + gl.drawArrays(gl.TRIANGLES, 0, 6); > + > + // Read back the rendering results, should be red > + var buf = gl.readPixels(0, 0, 2, 2, gl.RGBA, gl.UNSIGNED_BYTE); > + var idx = 0; > + correctColor = [255, 0, 0]; > + for (var y = 0; y < 2; y++) { > + for (var x = 0; x < 2; x++) { > + idx = (y * 2 + x) * 4; > + pixel[0] = buf[idx]; > + pixel[1] = buf[idx + 1]; > + pixel[2] = buf[idx + 2]; > + shouldBe("pixel", "correctColor"); > + } > + } > + > + debug('Testing copyTexSubImage2D'); > + > + // Green canvas > + gl.clearColor(0, 1, 0, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); I think there should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. > + > + gl.copyTexSubImage2D(gl.TEXTURE_2D, 0, 0, 0, 0, 0, 2, 2); There should be a shouldBe("gl.getError()", "gl.NO_ERROR") here. Previously this was generating INVALID_OPERATION. > + > + // Blue canvas > + gl.clearColor(0, 0, 1, 1); > + gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT); > + > + // Draw the triangles > + gl.drawArrays(gl.TRIANGLES, 0, 6); > + > + // Read back the rendering results, should be green > + buf = gl.readPixels(0, 0, 2, 2, gl.RGBA, gl.UNSIGNED_BYTE); > + correctColor = [0, 255, 0]; > + for (var y = 0; y < 2; y++) { > + for (var x = 0; x < 2; x++) { > + idx = (y * 2 + x) * 4; > + pixel[0] = buf[idx]; > + pixel[1] = buf[idx + 1]; > + pixel[2] = buf[idx + 2]; > + shouldBe("pixel", "correctColor"); > + } > + } > +} > + > +function runTest(antialias) > +{ > + debug("Testing with antialias on"); > + runTestIteration(true); > + debug("Testing with antialias off"); > + runTestIteration(false); > + > + successfullyParsed = true; > + var epilogue = document.createElement("script"); > + epilogue.onload = finish; > + epilogue.src = "../../js/resources/js-test-post.js"; > + document.body.appendChild(epilogue); > +} > + > +function finish() { > + if (window.layoutTestController) { > + layoutTestController.notifyDone(); > + } > +} > +</script> > +</head> > +<body onload="init()"> > +<canvas id="antialiasOn" width="2px" height="2px"></canvas> > +<canvas id="antialiasOff" width="2px" height="2px"></canvas> > +<div id="description"></div> > +<div id="console"></div> > +</body> > +</html> > Index: LayoutTests/fast/canvas/webgl/resources/utils3d.js > =================================================================== > --- LayoutTests/fast/canvas/webgl/resources/utils3d.js (revision 57160) > +++ LayoutTests/fast/canvas/webgl/resources/utils3d.js (working copy) > @@ -17,10 +17,10 @@ > // by the caller. By default, it maps to the window.console() function on WebKit and to > // an empty function on other browsers. > // > -function initWebGL(canvasName, vshader, fshader, attribs, clearColor, clearDepth) > +function initWebGL(canvasName, vshader, fshader, attribs, clearColor, clearDepth, contextAttribs) > { > var canvas = document.getElementById(canvasName); > - var gl = canvas.getContext("experimental-webgl"); > + var gl = canvas.getContext("experimental-webgl", contextAttribs); > if (!gl) { > alert("No WebGL context found"); > return null;

Zhenyao Mo

Comment 3 2010-04-07 14:31:17 PDT

Created attachment 52782 [details] revised patch: responding to Ken Russell's review

Kenneth Russell

Comment 4 2010-04-07 14:52:46 PDT

Comment on attachment 52782 [details] revised patch: responding to Ken Russell's review Looks good to me.

Dimitri Glazkov (Google)

Comment 5 2010-04-08 16:00:36 PDT

Comment on attachment 52782 [details] revised patch: responding to Ken Russell's review kbr's word is good as gold.

WebKit Commit Bot

Comment 6 2010-04-09 03:07:03 PDT

Comment on attachment 52782 [details] revised patch: responding to Ken Russell's review Clearing flags on attachment: 52782 Committed r57322: <http://trac.webkit.org/changeset/57322>

WebKit Commit Bot

Comment 7 2010-04-09 03:07:09 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.

Status RESOLVED

Resolution FIXED

Priority P2

Severity Normal

Classification Unclassified

Version 528+ (Nightly build)

Hardware All

OS All

Product WebKit

Component WebGL

Assignee

Zhenyao Mo

Reported

2010-04-06 15:39 PDT

Modified

2010-04-09 03:07 PDT History

CC List

5 users Show

URL

Keywords

Depends on

Blocks