Skip to content

Send an IpcSharedMemory in tex_image_2d and tex_sub_image_2d #22225

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

Merged
merged 9 commits into from
Nov 21, 2018
Merged

Conversation

nox
Copy link
Contributor

@nox nox commented Nov 19, 2018

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/lib.rs, components/webdriver_server/Cargo.toml, components/script/dom/imagedata.rs, components/script/Cargo.toml, components/script/dom/paintworkletglobalscope.rs and 3 more
  • @jgraham: components/webdriver_server/Cargo.toml, components/webdriver_server/lib.rs
  • @paulrouget: components/compositing/lib.rs, components/compositing/Cargo.toml, components/compositing/compositor.rs, components/compositing/windowing.rs, components/servo/Cargo.toml
  • @KiChjang: components/net/image_cache.rs, components/script_traits/lib.rs, components/script/lib.rs, components/script/dom/imagedata.rs, components/script/Cargo.toml and 5 more

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 19, 2018
@highfive
Copy link

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!
  • These commits modify net and script code, but no tests are modified. Please consider adding a test!

@nox
Copy link
Contributor Author

nox commented Nov 19, 2018

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 6bf3753 with merge 82c853c...

bors-servo pushed a commit that referenced this pull request Nov 19, 2018
Send an IpcSharedMemory in tex_image_2d and tex_sub_image_2d
@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-css

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Nov 19, 2018
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Nov 19, 2018
@nox
Copy link
Contributor Author

nox commented Nov 19, 2018

This should be ready to be reviewed.

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 3ce1ead with merge 7e59c7a...

bors-servo pushed a commit that referenced this pull request Nov 19, 2018
Send an IpcSharedMemory in tex_image_2d and tex_sub_image_2d

<!-- 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/22225)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

💔 Test failed - status-taskcluster

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Nov 19, 2018
@highfive highfive removed the S-tests-failed The changes caused existing tests to fail. label Nov 19, 2018
@nox
Copy link
Contributor Author

nox commented Nov 19, 2018

Unhappy tidy, fixed.

@nox
Copy link
Contributor Author

nox commented Nov 20, 2018

r? @jdm

This avoids a copy in the case of textures coming from HTMLImageElement.
Copy link
Member

@emilio emilio left a comment

Choose a reason for hiding this comment

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

Looks good, with that question. Have you checked any perf numbers out of curiosity? Just wondering what the relative speed is between creating an IPC channel vs. a shared memory thing.

);
let effective_data_type = self
.extension_manager
.effective_type(data_type.as_gl_constant());
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't textSubImage2D need to go through the EM in order to get the effective internal format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was like that before, and using it makes everything panic on the GL side.

@nox
Copy link
Contributor Author

nox commented Nov 21, 2018

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

📌 Commit 9a8d03a has been approved by emilio

@highfive highfive assigned emilio and unassigned jdm Nov 21, 2018
@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 21, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit 9a8d03a with merge 2b410ac...

bors-servo pushed a commit that referenced this pull request Nov 21, 2018
Send an IpcSharedMemory in tex_image_2d and tex_sub_image_2d

<!-- 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/22225)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

@bors-servo bors-servo merged commit 9a8d03a into master Nov 21, 2018
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 21, 2018
This was referenced Nov 21, 2018
@SimonSapin SimonSapin deleted the webgl branch December 12, 2018 15:26
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.

6 participants