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

Make GL/GLES decisions based on the API in use #23744

Merged
merged 1 commit into from Jul 12, 2019
Merged

Make GL/GLES decisions based on the API in use #23744

merged 1 commit into from Jul 12, 2019

Conversation

@jdm
Copy link
Member

jdm commented Jul 10, 2019 •
edited by SimonSapin

These changes remove a number of assumptions about whether GL/GLES is in use, improving the support for running WebGL in Servo under ANGLE.

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because no tests on Windows.

This change is Reviewable
@highfive
Copy link

highfive commented Jul 10, 2019

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/dom/webgl_extensions/ext/oesstandardderivatives.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl_extensions/ext/oeselementindexuint.rs, components/script/dom/webglshader.rs, components/script/dom/bindings/trace.rs and 3 more
  • @KiChjang: components/script/dom/webgl_extensions/ext/oesstandardderivatives.rs, components/script/dom/webglrenderingcontext.rs, components/script/dom/webgl_extensions/ext/oeselementindexuint.rs, components/script/dom/webglshader.rs, components/script/dom/bindings/trace.rs and 3 more
@highfive
Copy link

highfive commented Jul 10, 2019

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@jdm jdm force-pushed the jdm:gles-fixes branch from 752ee47 to b4a90ed Jul 10, 2019
@jdm
jdm reviewed Jul 10, 2019
View changes
components/canvas_traits/webgl.rs
@@ -74,6 +74,12 @@ pub enum WebGLMsg {
Exit,
}

#[derive(Clone, Copy, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]

This comment has been minimized.

Sign in to view
@jdm

jdm Jul 10, 2019

Author Member

This duplicate type exists because the original lacks serialization.
@jdm jdm force-pushed the jdm:gles-fixes branch from b4a90ed to 63c0b01 Jul 11, 2019
@jdm
Copy link
Member Author

jdm commented Jul 12, 2019

Review ping @asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

☔ The latest upstream changes (presumably #23516) made this pull request unmergeable. Please resolve the merge conflicts.
@asajeffrey
asajeffrey approved these changes Jul 12, 2019
View changes
Copy link
Member

asajeffrey left a comment

Much better, the fewer cfg!(os = "foo") the better!
components/script/dom/webglshader.rs
@@ -122,15 +123,15 @@ impl WebGLShader {
};
let validator = match webgl_version {
WebGLVersion::WebGL1 => {
let output_format = if cfg!(any(target_os = "android", target_os = "ios")) {
let output_format = if api_type == GlType::Gles {

This comment has been minimized.

Sign in to view
@asajeffrey

asajeffrey Jul 12, 2019

Member

This is much much nicer!
@asajeffrey
Copy link
Member

asajeffrey commented Jul 12, 2019

Rebase and r=me.
@jdm jdm force-pushed the jdm:gles-fixes branch from 63c0b01 to fd4268f Jul 12, 2019
@jdm
Copy link
Member Author

jdm commented Jul 12, 2019

@bors-servo r=asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

📌 Commit fd4268f has been approved by asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

☔ The latest upstream changes (presumably #23758) made this pull request unmergeable. Please resolve the merge conflicts.
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

🔒 Merge conflict
@jdm
Make GL/GLES decisions based on the API in use.
Loading status checks…
dbaed5e
@jdm jdm force-pushed the jdm:gles-fixes branch from fd4268f to dbaed5e Jul 12, 2019
@jdm
Copy link
Member Author

jdm commented Jul 12, 2019

@bors-servo r=asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

📌 Commit dbaed5e has been approved by asajeffrey
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

⌛ Testing commit dbaed5e with merge 2f1e822...
bors-servo added a commit that referenced this pull request Jul 12, 2019
@bors-servo
Auto merge of #23744 - jdm:gles-fixes, r=asajeffrey
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
2f1e822 
Make GL/GLES decisions based on the API in use

These changes remove a number of assumptions about whether GL/GLES is in use, improving the support for running WebGL in Servo under ANGLE.

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes do not require tests because no tests on Windows.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23744)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 12, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: asajeffrey
Pushing 2f1e822 to master...
@bors-servo bors-servo merged commit dbaed5e into servo:master Jul 12, 2019
2 of 3 checks passed
2 of 3 checks passed
continuous-integration/appveyor/pr AppVeyor build failed
Details
Taskcluster (pull_request) TaskGroup: success
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asajeffrey asajeffrey

Assignees

@asajeffrey asajeffrey

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@jdm @highfive @bors-servo @asajeffrey
You can’t perform that action at this time.