Integer-type WebGL2Context::VertexAttrib stores and accesses data in different places when the index is 0

RESOLVED FIXED in Firefox 46

Status

()

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: brad.kotsopoulos, Assigned: brad.kotsopoulos)

Tracking

45 Branch
mozilla46
x86_64
Windows 10
Points:
---

Firefox Tracking Flags

(firefox46 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

3 years ago
Posted file patch.pat (obsolete) —
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2311.135 Safari/537.36 Edge/12.10240

Steps to reproduce:

Running the test conformance2/attribs/gl-vertex-attrib.html from 

https://www.khronos.org/registry/webgl/sdk/tests/webgl-conformance-tests.html

with WebGL 2.0 using ANGLE, on Firefox 45 on Windows.

Example:

      gl.vertexAttribI4i(ii, -1, 0, 1, 2);
      shouldBeType('gl.getVertexAttrib(' + ii + ', gl.CURRENT_VERTEX_ATTRIB)', 'Int32Array');
      shouldBe('gl.getVertexAttrib(' + ii + ', gl.CURRENT_VERTEX_ATTRIB)[0]', '-1');
      shouldBe('gl.getVertexAttrib(' + ii + ', gl.CURRENT_VERTEX_ATTRIB)[1]', '0');
      shouldBe('gl.getVertexAttrib(' + ii + ', gl.CURRENT_VERTEX_ATTRIB)[2]', '1');
      shouldBe('gl.getVertexAttrib(' + ii + ', gl.CURRENT_VERTEX_ATTRIB)[3]', '2');



Actual results:

All tests with integer-type attribs with index = 0 get results similar to the following, when trying to read the attrib data:

PASS gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB) is an instance of Int32Array
FAIL gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB)[0] should be -1. Is 1987987734


Expected results:

The data should be read correctly, as set:

PASS gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB) is an instance of Int32Array
PASS gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB)[0] is -1
(Assignee)

Comment 1

3 years ago
Posted patch Proposed patch (obsolete) — Splinter Review
Attachment #8693097 - Attachment is obsolete: true
Attachment #8693100 - Flags: review?(jmuizelaar)
(Assignee)

Updated

3 years ago
OS: Unspecified → Windows 10
Hardware: Unspecified → x86_64
(Assignee)

Comment 2

3 years ago
This change allows us to conform with how we get vertex attribs in WebGLContextVertices.cpp

void
WebGLContext::GetVertexAttribInt(GLuint index, GLint* out_result)
{
    if (index) {
        gl->fGetVertexAttribIiv(index, LOCAL_GL_CURRENT_VERTEX_ATTRIB, out_result);
    } else {
        out_result[0] = BitwiseCast<GLint>(mVertexAttrib0Vector[0]);
        out_result[1] = BitwiseCast<GLint>(mVertexAttrib0Vector[1]);
        out_result[2] = BitwiseCast<GLint>(mVertexAttrib0Vector[2]);
        out_result[3] = BitwiseCast<GLint>(mVertexAttrib0Vector[3]);
    }
}

(In reply to Brad Kotsopoulos from comment #0)
> Created attachment 8693097 [details]
> patch.pat
> 
> User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36
> (KHTML, like Gecko) Chrome/42.0.2311.135 Safari/537.36 Edge/12.10240
> 
> Steps to reproduce:
> 
> Running the test conformance2/attribs/gl-vertex-attrib.html from 
> 
> https://www.khronos.org/registry/webgl/sdk/tests/webgl-conformance-tests.html
> 
> with WebGL 2.0 using ANGLE, on Firefox 45 on Windows.
> 
> Example:
> 
>       gl.vertexAttribI4i(ii, -1, 0, 1, 2);
>       shouldBeType('gl.getVertexAttrib(' + ii + ',
> gl.CURRENT_VERTEX_ATTRIB)', 'Int32Array');
>       shouldBe('gl.getVertexAttrib(' + ii + ',
> gl.CURRENT_VERTEX_ATTRIB)[0]', '-1');
>       shouldBe('gl.getVertexAttrib(' + ii + ',
> gl.CURRENT_VERTEX_ATTRIB)[1]', '0');
>       shouldBe('gl.getVertexAttrib(' + ii + ',
> gl.CURRENT_VERTEX_ATTRIB)[2]', '1');
>       shouldBe('gl.getVertexAttrib(' + ii + ',
> gl.CURRENT_VERTEX_ATTRIB)[3]', '2');
> 
> 
> 
> Actual results:
> 
> All tests with integer-type attribs with index = 0 get results similar to
> the following, when trying to read the attrib data:
> 
> PASS gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB) is an instance of
> Int32Array
> FAIL gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB)[0] should be -1. Is
> 1987987734
> 
> 
> Expected results:
> 
> The data should be read correctly, as set:
> 
> PASS gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB) is an instance of
> Int32Array
> PASS gl.getVertexAttrib(0, gl.CURRENT_VERTEX_ATTRIB)[0] is -1
Comment on attachment 8693100 [details] [diff] [review]
Proposed patch

Review of attachment 8693100 [details] [diff] [review]:
-----------------------------------------------------------------

The white space in the additions to this file is off. It looks like tabs vs spaces.
Attachment #8693100 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 4

3 years ago
Posted patch Proposed patch (obsolete) — Splinter Review
Attachment #8693100 - Attachment is obsolete: true
Attachment #8693103 - Flags: review?(jmuizelaar)
Comment on attachment 8693103 [details] [diff] [review]
Proposed patch

Review of attachment 8693103 [details] [diff] [review]:
-----------------------------------------------------------------

For what ever reason this seems to have two copies of the patch.
Attachment #8693103 - Flags: review?(jmuizelaar) → review?(jgilbert)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: gfx-noted
(Assignee)

Comment 6

3 years ago
Attachment #8693103 - Attachment is obsolete: true
Attachment #8693103 - Flags: review?(jgilbert)
Attachment #8693230 - Flags: review?(jmuizelaar)
(Assignee)

Updated

3 years ago
Attachment #8693230 - Flags: review?(jmuizelaar) → review?(jgilbert)
Review ping.
Flags: needinfo?(jgilbert)
Comment on attachment 8693230 [details] [diff] [review]
Proposed patch

Review of attachment 8693230 [details] [diff] [review]:
-----------------------------------------------------------------

Yep, sorry for delay.
Attachment #8693230 - Flags: review?(jgilbert) → review+
Assignee: nobody → brad18
Flags: needinfo?(jgilbert)

Comment 10

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/d3f624354fb3
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox46: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.