Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace `object` function arguments in WebGL with typed arrays #20396

Merged
merged 9 commits into from Mar 23, 2018
+509 −577
Merged

Replace `object` function arguments in WebGL with typed arrays #20396

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
36f39ce
Change WebGL function signatures accepting typed arrays
Xanewok Mar 22, 2018
c1c74ed
Adapt function bodies for usage with typed array args
Xanewok Mar 22, 2018
20f21cb
Adapt uniform[fv] and similar to accept typed array args
Xanewok Mar 22, 2018
daceeb4
Appease test-tidy
Xanewok Mar 23, 2018
cb85a95
Fix sanity check in vertex attrib
Xanewok Mar 23, 2018
c42127b
Fix indentation
Xanewok Mar 23, 2018
2437a84
Unify argument auto rooting in codegen
Xanewok Mar 23, 2018
fbfcc76
Add codegen test for function overloads taking typed array args
Xanewok Mar 23, 2018
80c6891
Use safe to_vec() for typed arrays in WebGL bindings
Xanewok Mar 23, 2018
File filter...
Filter file types
Clear filters
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Prev Next

Adapt uniform[fv] and similar to accept typed array args

  • Loading branch information
@Xanewok
Xanewok committed Mar 23, 2018
commit 20f21cb5f85c48e7106177db6aa41fa54365d6cc
6 components/script/dom/bindings/codegen/CodegenRust.py
View file
Open in desktop
@@ -418,11 +418,15 @@ def pickFirstSignature(condition, filterLambda):
template = info.template
declType = info.declType

argName = "arg%d" % distinguishingIndex

testCode = instantiateJSToNativeConversionTemplate(
template,
{"val": distinguishingArg},
declType,
"arg%d" % distinguishingIndex)
argName)
if type_needs_auto_root(type):
testCode.append(CGGeneric("auto_root!(in(cx) let %s = %s);" % (argName, argName)))

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Mar 23, 2018

Member

What if we move this into instantiateJSToNativeConversionTemplate instead? Can we also add codegen tests for:

  • two overloads - ArrayBuffer and DOMString
  • three overloads - a union containing ArrayBuffer and DOMString

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Mar 23, 2018

Member

Specifically, if we add a field to JSToNativeConversionInfo and make getJSToNativeConversionInfo set it appropriately, instantiateJSToNativeConversionTemplate could add the extra auto_root! declaration.

This comment has been minimized.

Sign in to view
Copy link
@Xanewok

Xanewok Mar 23, 2018 •
edited

Author Contributor

Ah, now that I read your reply, it seems I misread it. Do you think 01c6bee is sufficient for now or should I change it further?

Re overloads:
Currently unions at distinguishable indices are not supported by CodegenRust (see WebGLRenderingContext.webidl:493), so this still blocks us on removing last object? workaround function here.

I can definitely add codegen tests for overloads with auto rootable types here though (e.g. overloads on ArrayBuffer and DOMString)

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Mar 23, 2018

Member

I'm a bit worried that we'll keep hitting similar issues with missing CustomAutoRoot wrappers if we keep doing spot-fixes that only change a single call-site at a time.

This comment has been minimized.

Sign in to view
Copy link
@Xanewok

Xanewok Mar 23, 2018

Author Contributor

Yeah, I agree completely.

However in this particular case I'm not sure what might be the best approach.
In general getJSToNativeConversionInfo returns template, declType and default, which is then later passed manually to instantiateJSToNativeConversionTemplate.

In the updated 01c6bee, for the instantiateJSToNativeConversionTemplate call I'm passing the information if the intantiated template needs the rooting (which is later done inside the call).

If I'm not mistaken, do you mean that we should return an additional field (e.g. needsAutoRooting) from getJSToNativeConversionInfo and pass it later manually for every following instantiateJSToNativeConversionTemplate call if needed, as well?

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Mar 23, 2018

Member

Bleah. We should really refactor the code generation to pass the JSToNativeConversionInfo class directly to instantiateJSToNativeConversionTemplate so it's easier to make changes like this. I'll file a separate issue to do that and clean up the changes from this PR.

This comment has been minimized.

Sign in to view
Copy link
@jdm

jdm Mar 23, 2018

Member

Filed #20403.

# Indent by 4, since we need to indent further than our "do" statement
caseBody.append(CGIndenter(testCode, 4))
242 components/script/dom/webgl2renderingcontext.rs
View file
Open in desktop
@@ -32,7 +32,7 @@ use euclid::Size2D;
use js::jsapi::{JSContext, JSObject};
use js::jsval::JSVal;
use js::rust::CustomAutoRooterGuard;
use js::typedarray::ArrayBufferView;
use js::typedarray::{ArrayBufferView, Float32Array, Int32Array};
use offscreen_gl_context::GLContextAttributes;
use script_layout_interface::HTMLCanvasDataSource;
use std::ptr::NonNull;
@@ -597,161 +597,197 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext {

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform1f(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
val: f32) {
self.base.Uniform1f(uniform, val)
self.base.Uniform1f(location, val)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform1i(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
val: i32) {
self.base.Uniform1i(uniform, val)
self.base.Uniform1i(location, val)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform1iv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform1iv(cx, uniform, data)
fn Uniform1iv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Int32Array>) {
self.base.Uniform1iv(location, v)
}

fn Uniform1iv_(&self, location: Option<&WebGLUniformLocation>, v: Vec<i32>) {
self.base.Uniform1iv_(location, v);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform1fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform1fv(cx, uniform, data)
fn Uniform1fv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Float32Array>) {
self.base.Uniform1fv(location, v);
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform1fv_(&self, location: Option<&WebGLUniformLocation>, v: Vec<f32>) {
self.base.Uniform1fv_(location, v);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform2f(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
x: f32, y: f32) {
self.base.Uniform2f(uniform, x, y)
self.base.Uniform2f(location, x, y)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform2fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform2fv(cx, uniform, data)
fn Uniform2fv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Float32Array>) {
self.base.Uniform2fv(location, v)
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform2fv_(&self, location: Option<&WebGLUniformLocation>, v: Vec<f32>) {
self.base.Uniform2fv_(location, v);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform2i(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
x: i32, y: i32) {
self.base.Uniform2i(uniform, x, y)
self.base.Uniform2i(location, x, y)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform2iv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform2iv(cx, uniform, data)
fn Uniform2iv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Int32Array>) {
self.base.Uniform2iv(location, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform2iv_(&self, location: Option<&WebGLUniformLocation>, v: Vec<i32>) {
self.base.Uniform2iv_(location, v);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform3f(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
x: f32, y: f32, z: f32) {
self.base.Uniform3f(uniform, x, y, z)
self.base.Uniform3f(location, x, y, z)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform3fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform3fv(cx, uniform, data)
fn Uniform3fv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Float32Array>) {
self.base.Uniform3fv(location, v)
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform3fv_(&self, location: Option<&WebGLUniformLocation>, v: Vec<f32>) {
self.base.Uniform3fv_(location, v);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform3i(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
x: i32, y: i32, z: i32) {
self.base.Uniform3i(uniform, x, y, z)
self.base.Uniform3i(location, x, y, z)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform3iv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform3iv(cx, uniform, data)
fn Uniform3iv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Int32Array>) {
self.base.Uniform3iv(location, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform3iv_(&self,
location: Option<&WebGLUniformLocation>,
v: Vec<i32>) {
self.base.Uniform3iv_(location, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform4i(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
x: i32, y: i32, z: i32, w: i32) {
self.base.Uniform4i(uniform, x, y, z, w)
self.base.Uniform4i(location, x, y, z, w)
}


/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform4iv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform4iv(cx, uniform, data)
fn Uniform4iv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Int32Array>) {
self.base.Uniform4iv(location, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform4iv_(&self,
location: Option<&WebGLUniformLocation>,
v: Vec<i32>) {
self.base.Uniform4iv_(location, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform4f(&self,
uniform: Option<&WebGLUniformLocation>,
location: Option<&WebGLUniformLocation>,
x: f32, y: f32, z: f32, w: f32) {
self.base.Uniform4f(uniform, x, y, z, w)
self.base.Uniform4f(location, x, y, z, w)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn Uniform4fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
data: *mut JSObject) -> Fallible<()> {
self.base.Uniform4fv(cx, uniform, data)
fn Uniform4fv(&self,
location: Option<&WebGLUniformLocation>,
v: CustomAutoRooterGuard<Float32Array>) {
self.base.Uniform4fv(location, v)
}

// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn Uniform4fv_(&self, location: Option<&WebGLUniformLocation>, v: Vec<f32>) {
self.base.Uniform4fv_(location, v);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn UniformMatrix2fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
fn UniformMatrix2fv(&self,
location: Option<&WebGLUniformLocation>,
transpose: bool,
data: *mut JSObject) -> Fallible<()> {
self.base.UniformMatrix2fv(cx, uniform, transpose, data)
v: CustomAutoRooterGuard<Float32Array>) {
self.base.UniformMatrix2fv(location, transpose, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn UniformMatrix3fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
fn UniformMatrix2fv_(&self, location: Option<&WebGLUniformLocation>, transpose: bool, value: Vec<f32>) {
self.base.UniformMatrix2fv_(location, transpose, value);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn UniformMatrix3fv(&self,
location: Option<&WebGLUniformLocation>,
transpose: bool,
data: *mut JSObject) -> Fallible<()> {
self.base.UniformMatrix3fv(cx, uniform, transpose, data)
v: CustomAutoRooterGuard<Float32Array>) {
self.base.UniformMatrix3fv(location, transpose, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn UniformMatrix4fv(&self,
cx: *mut JSContext,
uniform: Option<&WebGLUniformLocation>,
fn UniformMatrix3fv_(&self, location: Option<&WebGLUniformLocation>, transpose: bool, value: Vec<f32>) {
self.base.UniformMatrix3fv_(location, transpose, value);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn UniformMatrix4fv(&self,
location: Option<&WebGLUniformLocation>,
transpose: bool,
data: *mut JSObject) -> Fallible<()> {
self.base.UniformMatrix4fv(cx, uniform, transpose, data)
v: CustomAutoRooterGuard<Float32Array>) {
self.base.UniformMatrix4fv(location, transpose, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn UniformMatrix4fv_(&self, location: Option<&WebGLUniformLocation>, transpose: bool, value: Vec<f32>) {
self.base.UniformMatrix4fv_(location, transpose, value);
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.9
@@ -770,9 +806,13 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext {
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn VertexAttrib1fv(&self, cx: *mut JSContext, indx: u32, data: *mut JSObject) -> Fallible<()> {
self.base.VertexAttrib1fv(cx, indx, data)
fn VertexAttrib1fv(&self, indx: u32, v: CustomAutoRooterGuard<Float32Array>) {
self.base.VertexAttrib1fv(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn VertexAttrib1fv_(&self, indx: u32, v: Vec<f32>) {
self.base.VertexAttrib1fv_(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
@@ -781,9 +821,13 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext {
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn VertexAttrib2fv(&self, cx: *mut JSContext, indx: u32, data: *mut JSObject) -> Fallible<()> {
self.base.VertexAttrib2fv(cx, indx, data)
fn VertexAttrib2fv(&self, indx: u32, v: CustomAutoRooterGuard<Float32Array>) {
self.base.VertexAttrib2fv(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn VertexAttrib2fv_(&self, indx: u32, v: Vec<f32>) {
self.base.VertexAttrib2fv_(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
@@ -792,9 +836,13 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext {
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn VertexAttrib3fv(&self, cx: *mut JSContext, indx: u32, data: *mut JSObject) -> Fallible<()> {
self.base.VertexAttrib3fv(cx, indx, data)
fn VertexAttrib3fv(&self, indx: u32, v: CustomAutoRooterGuard<Float32Array>) {
self.base.VertexAttrib3fv(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn VertexAttrib3fv_(&self, indx: u32, v: Vec<f32>) {
self.base.VertexAttrib3fv_(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
@@ -803,9 +851,13 @@ impl WebGL2RenderingContextMethods for WebGL2RenderingContext {
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
#[allow(unsafe_code)]
unsafe fn VertexAttrib4fv(&self, cx: *mut JSContext, indx: u32, data: *mut JSObject) -> Fallible<()> {
self.base.VertexAttrib4fv(cx, indx, data)
fn VertexAttrib4fv(&self, indx: u32, v: CustomAutoRooterGuard<Float32Array>) {
self.base.VertexAttrib4fv(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
fn VertexAttrib4fv_(&self, indx: u32, v: Vec<f32>) {
self.base.VertexAttrib4fv_(indx, v)
}

/// https://www.khronos.org/registry/webgl/specs/latest/1.0/#5.14.10
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.