Implement the basic WebGL2 buffer data operations #24473

mmatyas · 2019-10-17T10:06:11Z

Adds support for the WebGL2 calls bufferData, bufferSubData, copyBufferSubData and getBufferSubData.

Reference: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.3

This patch depends on servo/sparkle#8. Some tests cause a crash for me at the moment, as they depend on other, not yet implemented buffer calls and transform feedback objects.

As for the code, there are a few parts I'm not sure about:

To get the element byte size of a TypedArray I've wrote a simple match, as the relevant field is not published in rust-mozjs. Is that okay or there's some other way to get this already?
The WebGL1 BufferData implementations were copied into the WebGL2 code as a workaround, due to the difference in the available buffer slots (ie. self.bound_buffer). An alternative could be is to pass this function and self as parameters to an internal buffer data implementation function, but personally I found that code to be quite ugly.

cc @jdm @zakorgy

./mach build -d does not report any errors (with the sparkle patch)
./mach test-tidy does not report any errors
These changes fix #___ (GitHub issue number if applicable)

There are tests for these changes OR
These changes do not require tests because ___

highfive · 2019-10-17T10:06:17Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/webidls/WebGL2RenderingContext.webidl, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglbuffer.rs
@jgraham: tests/wpt/webgl/meta/conformance2/buffers/buffer-copying-contents.html.ini, tests/wpt/webgl/meta/conformance2/buffers/get-buffer-sub-data-validity.html.ini, tests/wpt/webgl/meta/conformance2/buffers/get-buffer-sub-data.html.ini, tests/wpt/webgl/meta/conformance2/buffers/buffer-data-and-buffer-sub-data-sub-source.html.ini, tests/wpt/webgl/meta/conformance2/buffers/buffer-copying-restrictions.html.ini
@KiChjang: components/script/dom/webidls/WebGL2RenderingContext.webidl, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl2renderingcontext.rs, components/script/dom/webglbuffer.rs

highfive · 2019-10-17T10:06:19Z

Warning

These commits modify unsafe code. Please review it carefully!

jdm

Looks pretty good! I'm curious about the new failing test results, though.

...wpt/webgl/meta/conformance2/buffers/buffer-copying-restrictions.html.ini

...conformance2/buffers/buffer-data-and-buffer-sub-data-sub-source.html.ini

...pt/webgl/meta/conformance2/buffers/get-buffer-sub-data-validity.html.ini

components/script/dom/webgl2renderingcontext.rs

mmatyas · 2019-10-18T11:53:21Z

Took a fresh look on the tests, and I've managed to fix the crashes. pass the rest and also reduce the code code duplication between the WebGL1 and 2 code.

jdm · 2019-10-24T06:21:16Z

r? @nox
Can you take a look at these changes?

nox

Seems ok to me, just a couple of questions.

nox · 2019-10-24T11:51:28Z

Cargo.toml

@@ -33,3 +33,4 @@ rand_core = { git = "https://github.com/servo/rand", branch = "servo-rand_os-0.1
 autocfg = { git = "https://github.com/servo/autocfg", branch = "rustflags" }
 # https://github.com/retep998/winapi-rs/pull/816
 winapi = { git = "https://github.com/servo/winapi-rs", branch = "patch-1" }
+sparkle = { git = "https://github.com/servo/sparkle", branch = "master" }


There have been a few new commits since the last published version of sparkle (ie. servo/sparkle#8 has landed). I can remove it when @jdm increases the version.

Published 0.1.7.

components/script/dom/webgl2renderingcontext.rs

mmatyas · 2019-10-29T12:15:11Z

Updated with the sparkle version bump.

jdm · 2019-10-29T12:45:46Z

@bors-servo r=nox,jdm

bors-servo · 2019-10-29T12:45:47Z

📌 Commit 1e11e3e has been approved by nox,jdm

bors-servo · 2019-11-04T21:02:31Z

💔 Test failed - linux-rel-wpt


        Implement the basic WebGL2 buffer data operations

Adds support for `bufferData`, `bufferSubData`, `copyBufferSubData` and `getBufferSubData`. Reference: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.3

community-tc-integration · 2019-11-05T10:35:21Z

Submitting the task to Taskcluster failed. Details

Taskcluster-GitHub attempted to create a task for this event with the following scopes:

[
  "assume:repo:github.com/servo/servo:pull-request",
  "queue:route:statuses",
  "queue:scheduler-id:taskcluster-github"
]

The expansion of these scopes is not sufficient to create the task, leading to the following:

Client ID static/taskcluster/github does not have sufficient scopes and is missing the following scopes:

{
  "AnyOf": [
    {
      "AnyOf": [
        "queue:create-task:highest:aws-provisioner-v1/servo-docker-untrusted",
        "queue:create-task:very-high:aws-provisioner-v1/servo-docker-untrusted",
        "queue:create-task:high:aws-provisioner-v1/servo-docker-untrusted",
        "queue:create-task:medium:aws-provisioner-v1/servo-docker-untrusted",
        "queue:create-task:low:aws-provisioner-v1/servo-docker-untrusted",
        "queue:create-task:very-low:aws-provisioner-v1/servo-docker-untrusted",
        "queue:create-task:lowest:aws-provisioner-v1/servo-docker-untrusted"
      ]
    },
    {
      "AnyOf": [
        "queue:create-task:aws-provisioner-v1/servo-docker-untrusted",
        {
          "AllOf": [
            "queue:define-task:aws-provisioner-v1/servo-docker-untrusted",
            "queue:task-group-id:taskcluster-github/CybUaT64RIe8YSwjvjtSFQ",
            "queue:schedule-task:taskcluster-github/CybUaT64RIe8YSwjvjtSFQ/CybUaT64RIe8YSwjvjtSFQ"
          ]
        }
      ]
    }
  ]
}

This request requires the client to satisfy the following scope expression:

{
  "AllOf": [
    "assume:repo:github.com/servo/servo:pull-request",
    "queue:route:tc-treeherder.v2._/servo-prs.4050b7f9eca4c581d100fed778fa09f21d7e09dd",
    "queue:route:tc-treeherder-staging.v2._/servo-prs.4050b7f9eca4c581d100fed778fa09f21d7e09dd",
    "queue:route:statuses",
    {
      "AnyOf": [
        {
          "AllOf": [
            "queue:scheduler-id:taskcluster-github",
            {
              "AnyOf": [
                "queue:create-task:highest:aws-provisioner-v1/servo-docker-untrusted",
                "queue:create-task:very-high:aws-provisioner-v1/servo-docker-untrusted",
                "queue:create-task:high:aws-provisioner-v1/servo-docker-untrusted",
                "queue:create-task:medium:aws-provisioner-v1/servo-docker-untrusted",
                "queue:create-task:low:aws-provisioner-v1/servo-docker-untrusted",
                "queue:create-task:very-low:aws-provisioner-v1/servo-docker-untrusted",
                "queue:create-task:lowest:aws-provisioner-v1/servo-docker-untrusted"
              ]
            }
          ]
        },
        {
          "AnyOf": [
            "queue:create-task:aws-provisioner-v1/servo-docker-untrusted",
            {
              "AllOf": [
                "queue:define-task:aws-provisioner-v1/servo-docker-untrusted",
                "queue:task-group-id:taskcluster-github/CybUaT64RIe8YSwjvjtSFQ",
                "queue:schedule-task:taskcluster-github/CybUaT64RIe8YSwjvjtSFQ/CybUaT64RIe8YSwjvjtSFQ"
              ]
            }
          ]
        }
      ]
    }
  ]
}

method: createTask
errorCode: InsufficientScopes
statusCode: 403
time: 2019-11-05T10:35:21.647Z

mmatyas · 2019-11-05T10:50:16Z

Well I've looked into the issue but I'm not sure whether it's related to the buffers or not. Like the other one, this test also goes further with copyBufferSubData available, then checks the result of drawElements, but the canvas doesn't have the expected colors in headless mode. The buffer operations seem to be working fine though and the test does pass in non-headless mode. Because of that I'm not sure where the error comes from; I've updated the test expectations for now.

jdm · 2019-11-05T13:48:30Z

@bors-servo r+

bors-servo · 2019-11-05T13:48:32Z

📌 Commit 4050b7f has been approved by jdm

bors-servo · 2019-11-05T19:13:29Z

⌛ Testing commit 4050b7f with merge 42aecf5...


        Auto merge of #24473 - mmatyas:webgl_fns_buffer, r=jdm

@jdm

Implement the basic WebGL2 buffer data operations Adds support for the WebGL2 calls `bufferData`, `bufferSubData`, `copyBufferSubData` and `getBufferSubData`. Reference: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.3 ---  This patch depends on servo/sparkle#8. Some tests cause a crash for me at the moment, as they depend on other, not yet implemented buffer calls and transform feedback objects. As for the code, there are a few parts I'm not sure about: - To get the element byte size of a TypedArray I've wrote a simple `match`, as the relevant field is not published in `rust-mozjs`. Is that okay or there's some other way to get this already? - The WebGL1 BufferData implementations were copied into the WebGL2 code as a workaround, due to the difference in the available buffer slots (ie. `self.bound_buffer`). An alternative could be is to pass this function and self as parameters to an internal buffer data implementation function, but personally I found that code to be quite ugly. cc @jdm @zakorgy ---  - [x] `./mach build -d` does not report any errors (with the sparkle patch) - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2019-11-05T20:47:25Z

💔 Test failed - status-taskcluster

jdm · 2019-11-05T20:56:59Z

@bors-servo retry

#23290

bors-servo · 2019-11-05T21:52:48Z

💣 Failed to start rebuilding: Unknown error

bors-servo · 2019-11-05T21:52:53Z

⌛ Testing commit 4050b7f with merge afbcbf7...


        Auto merge of #24473 - mmatyas:webgl_fns_buffer, r=jdm

@jdm

Implement the basic WebGL2 buffer data operations Adds support for the WebGL2 calls `bufferData`, `bufferSubData`, `copyBufferSubData` and `getBufferSubData`. Reference: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.3 ---  This patch depends on servo/sparkle#8. Some tests cause a crash for me at the moment, as they depend on other, not yet implemented buffer calls and transform feedback objects. As for the code, there are a few parts I'm not sure about: - To get the element byte size of a TypedArray I've wrote a simple `match`, as the relevant field is not published in `rust-mozjs`. Is that okay or there's some other way to get this already? - The WebGL1 BufferData implementations were copied into the WebGL2 code as a workaround, due to the difference in the available buffer slots (ie. `self.bound_buffer`). An alternative could be is to pass this function and self as parameters to an internal buffer data implementation function, but personally I found that code to be quite ugly. cc @jdm @zakorgy ---  - [x] `./mach build -d` does not report any errors (with the sparkle patch) - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2019-11-05T22:34:41Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing afbcbf7 to master...

highfive assigned jdm Oct 17, 2019

highfive added the S-awaiting-review label Oct 17, 2019

jdm requested changes Oct 17, 2019

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Oct 17, 2019

mmatyas force-pushed the mmatyas:webgl_fns_buffer branch from 1e4bf8d to 31aff06 Oct 18, 2019

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 18, 2019

mmatyas force-pushed the mmatyas:webgl_fns_buffer branch from 31aff06 to f4f9e57 Oct 18, 2019

mmatyas force-pushed the mmatyas:webgl_fns_buffer branch 2 times, most recently from 97ff135 to 374fd88 Oct 18, 2019

highfive assigned nox and unassigned jdm Oct 24, 2019

nox reviewed Oct 24, 2019

View changes

nox added S-awaiting-answer and removed S-awaiting-review labels Oct 24, 2019

jdm assigned jdm and unassigned nox Oct 28, 2019

jdm added S-needs-code-changes and removed S-awaiting-answer labels Oct 28, 2019

mmatyas force-pushed the mmatyas:webgl_fns_buffer branch from 374fd88 to 1e11e3e Oct 29, 2019

highfive added S-awaiting-review and removed S-needs-code-changes labels Oct 29, 2019

jdm added S-needs-code-changes and removed S-awaiting-answer S-awaiting-review labels Nov 4, 2019

Implement the basic WebGL2 buffer data operations

Loading status checks…

4050b7f

Adds support for `bufferData`, `bufferSubData`, `copyBufferSubData` and `getBufferSubData`. Reference: https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.3

mmatyas force-pushed the mmatyas:webgl_fns_buffer branch from 77fdd2b to 4050b7f Nov 5, 2019

highfive removed the S-tests-failed label Nov 5, 2019

highfive added S-awaiting-review and removed S-needs-code-changes labels Nov 5, 2019

highfive added S-awaiting-merge and removed S-awaiting-review labels Nov 5, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Nov 5, 2019

highfive added S-awaiting-merge and removed S-tests-failed labels Nov 5, 2019

bors-servo merged commit 4050b7f into servo:master Nov 5, 2019
2 checks passed

2 checks passed

Taskcluster (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Nov 5, 2019

bors-servo mentioned this pull request Nov 5, 2019

Add WebGL Transformfeedback support #24514

Merged

3 of 3 tasks complete

servo / servo

Implement the basic WebGL2 buffer data operations #24473

Implement the basic WebGL2 buffer data operations #24473

mmatyas commented Oct 17, 2019

highfive commented Oct 17, 2019

highfive commented Oct 17, 2019

jdm left a comment

mmatyas commented Oct 18, 2019

jdm commented Oct 24, 2019

nox left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

mmatyas commented Oct 29, 2019

jdm commented Oct 29, 2019

bors-servo commented Oct 29, 2019

bors-servo commented Nov 4, 2019

This comment has been minimized.

community-tc-integration bot commented on `4050b7f` Nov 5, 2019

mmatyas commented Nov 5, 2019

jdm commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

jdm commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

servo / servo

Join GitHub today

Implement the basic WebGL2 buffer data operations #24473

Implement the basic WebGL2 buffer data operations #24473

Conversation

mmatyas commented Oct 17, 2019

highfive commented Oct 17, 2019

highfive commented Oct 17, 2019

jdm left a comment

mmatyas commented Oct 18, 2019

jdm commented Oct 24, 2019

nox left a comment

This comment has been minimized.

nox Oct 24, 2019

This comment has been minimized.

mmatyas Oct 25, 2019

This comment has been minimized.

jdm Oct 28, 2019

mmatyas commented Oct 29, 2019

jdm commented Oct 29, 2019

bors-servo commented Oct 29, 2019

bors-servo commented Nov 4, 2019

This comment has been minimized.

community-tc-integration bot commented on 4050b7f Nov 5, 2019

mmatyas commented Nov 5, 2019

jdm commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

jdm commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

bors-servo commented Nov 5, 2019

community-tc-integration bot commented on `4050b7f` Nov 5, 2019