Skip to content

Conversation

@kdashg
Copy link
Contributor

@kdashg kdashg commented Jan 24, 2019

Copy link
Member

@kenrussell kenrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay reviewing. LGTM

@kenrussell kenrussell merged commit b28d4ee into KhronosGroup:master Feb 1, 2019
@lexaknyazev
Copy link
Member

Is it expected that Chrome 72 doesn't pass this test? Namely:

Tests that glActiveTexture and glBindTexture work as expectedSpecifically texture targets are per active texture unit.

On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

PASS getError was expected value: NO_ERROR : 
PASS getError was expected value: NO_ERROR : 
PASS getError was expected value: NO_ERROR : 
PASS getError was expected value: NO_ERROR : 
PASS getError was expected value: NO_ERROR : 
PASS should be 0,192,128,255
PASS should be 128,64,255,255
PASS should be 192,255,64,255
PASS should be 200,0,255,255

Swizzle applied to correct ActiveTexture index
PASS getError was expected value: NO_ERROR : 
FAIL window.control should be 10,20,30,40. (was 0,192,128,255)
FAIL window.after_swizzle_applied should be 10,20,30,40. (was 0,192,128,255)
PASS successfullyParsed is true

TEST COMPLETE

It seems that drawArrays here has no effect on the subsequent readPixels call.

window.control = new Uint8Array(4);
window.after_swizzle_applied = new Uint8Array(4);
gl.bindTexture(gl.TEXTURE_2D, tex0_rgba8);
gl.drawArrays(gl.POINTS, 0, 1);
gl.readPixels(0, 0, 1, 1, gl.RGBA, gl.UNSIGNED_BYTE, window.control);
shouldBeString("window.control", tex0_rgba8_data.toString());

@jdarpinian
Copy link
Contributor

We have a bug filed for this already: http://crbug.com/931006

@lexaknyazev
Copy link
Member

@jdarpinian Thanks for the link!
Further investigation shows that Chrome doesn't render points unless vertex shader sets gl_PointSize.
So, this test should be fixed here (#2819), and the spec should probably be more strict (#2818).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants