In OpenGL ES 2.0, the semantic is that the desktop GL capability GL_VERTEX_PROGRAM_SIZE is enabled all of the time; this enum is not exposed in the OpenGL ES 2.0 headers. In order to comply with this, in the desktop GL implementations of WebGL, this capability must be enabled at initialization time.
Created attachment 52988 [details] patch
Comment on attachment 52988 [details] patch > Index: LayoutTests/fast/canvas/webgl/point-size.html > =================================================================== > --- LayoutTests/fast/canvas/webgl/point-size.html (revision 0) > +++ LayoutTests/fast/canvas/webgl/point-size.html (revision 0) > @@ -0,0 +1,113 @@ > +<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 pos; > +attribute vec4 colorIn; > +uniform float pointSize; > +varying vec4 color; > + > +void main() > +{ > + gl_PointSize = pointSize; > + color = colorIn; > + gl_Position = vec4(pos.xyz, 1.0); > +} > +</script> > + > +<script id="fshader" type="x-shader/x-fragment"> > +varying vec4 color; > + > +void main() > +{ > + gl_FragColor = color; > +} > +</script> > + > +<script> > +var successfullyParsed = false; > + > +// These two declarations need to be global for "shouldBe" to see them > +var gl = null; > +var pixel = [0, 255, 0]; > + > +function init() > +{ > + if (window.layoutTestController) { > + layoutTestController.overridePreference("WebKitWebGLEnabled", "1"); > + layoutTestController.dumpAsText(); > + layoutTestController.waitUntilDone(); > + } This more complex framework (using layoutTestController.waitUntilDone()) isn't needed for this test. It is only needed when resources, like textures, are being downloaded in the background and the test needs to wait for them to download in order to run to completion. You can use tex-sub-image-2d.html as the basis for this test. We should use the simpler framework so it will be easier to port these to the Khronos site later. > + > + description('Verify GL_VERTEX_PROGRAM_POINT_SIZE is enabled in WebGL'); > + > + runTest(); > +} > + > +function runTest() > +{ > + shouldBeNonNull("gl = initWebGL('testbed', 'vshader', 'fshader', ['pos', 'colorIn'], [0, 0, 0, 1], 1, 0)"); > + gl.disable(gl.BLEND); > + gl.clear(gl.COLOR_BUFFER_BIT); > + > + var vertices = new WebGLFloatArray([ > + 0.5, 0.5, 0.0]); > + var colors = new WebGLUnsignedByteArray([ > + 255, 0, 0, 255]); > + > + var colorOffset = vertices.byteLength; This test should query gl.getParameter(gl.ALIASED_POINT_SIZE_RANGE) and see whether the range covers [1.0, 2.0]. If it turns out the implementation doesn't support a size greater than 1.0, the test should report that it passes. While this behavior won't work with the -expected.txt file, it will be semantically correct and will work within the Khronos conformance test harness. > + var vbo = gl.createBuffer(); > + gl.bindBuffer(gl.ARRAY_BUFFER, vbo); > + gl.bufferData(gl.ARRAY_BUFFER, colorOffset + colors.byteLength, gl.STATIC_DRAW); > + gl.bufferSubData(gl.ARRAY_BUFFER, 0, vertices); > + gl.bufferSubData(gl.ARRAY_BUFFER, colorOffset, colors); > + > + gl.vertexAttribPointer(0, 3, gl.FLOAT, false, 0, 0); > + gl.enableVertexAttribArray(0); > + gl.vertexAttribPointer(1, 4, gl.UNSIGNED_BYTE, true, 0, colorOffset); > + gl.enableVertexAttribArray(1); > + > + var locPointSize = gl.getUniformLocation(gl.program, 'pointSize'); A comment (or debug statement) would be helpful here, like "Draw a point of size 1 and verify it doesn't touch any other pixels." > + gl.uniform1f(locPointSize, 1.0); > + gl.drawArrays(gl.POINTS, 0, vertices.length / 3); > + var buf = gl.readPixels(0, 0, 1, 1, gl.RGBA, gl.UNSIGNED_BYTE); > + pixel[0] = buf[0]; > + pixel[1] = buf[1]; > + pixel[2] = buf[2]; > + shouldBeTrue("pixel[0] == 0 && pixel[1] == 0 && pixel[2] == 0"); > + > + gl.clear(gl.COLOR_BUFFER_BIT); A comment (or debug statement) would be helpful here, like "Draw a point of size 2 and verify it fills the appropriate region." > + gl.uniform1f(locPointSize, 2.0); > + gl.drawArrays(gl.POINTS, 0, vertices.length / 3); > + buf = gl.readPixels(0, 0, 1, 1, gl.RGBA, gl.UNSIGNED_BYTE); I think you should read the pixel at (0, 1) (or (1, 0)) rather than (0, 0). I believe that one should be guaranteed to be filled with exactly the intended color. > + pixel[0] = buf[0]; > + pixel[1] = buf[1]; > + pixel[2] = buf[2]; > + shouldBeTrue("pixel[0] > 0 && pixel[0] <= 255 && pixel[1] == 0 && pixel[2] == 0"); > + > + 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="testbed" width="2px" height="2px"></canvas> > +<div id="description"></div> > +<div id="console"></div> > +</body> > +</html>
Created attachment 53019 [details] revised patch : responding to Ken Russell's review
Created attachment 53021 [details] revised patch: update the expected test output
Created attachment 53022 [details] revised patch: modified a few comments in the test
Comment on attachment 53022 [details] revised patch: modified a few comments in the test Looks great. Excellent work on the test case for this.
Comment on attachment 53022 [details] revised patch: modified a few comments in the test I don't really understand what this does, or "why" we need to enable this. I believe you both, would just be nice to document "why" in the ChangeLog.
(In reply to comment #7) > (From update of attachment 53022 [details]) > I don't really understand what this does, or "why" we need to enable this. I > believe you both, would just be nice to document "why" in the ChangeLog. The description in the bug report concisely describes why it is needed. Is that sufficient?
Comment on attachment 53022 [details] revised patch: modified a few comments in the test rs=me given kbr approves.
Comment on attachment 53022 [details] revised patch: modified a few comments in the test Clearing flags on attachment: 53022 Committed r57694: <http://trac.webkit.org/changeset/57694>
All reviewed patches have been landed. Closing bug.