Skip to content
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

Webrender no nativewindow #10842

Merged
merged 1 commit into from Apr 26, 2016

Conversation

fabricedesre
Copy link
Contributor

@glennw this fixed the panic when using webrender and no native display is available.


This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

@highfive
Copy link

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Apr 25, 2016
@@ -572,7 +577,9 @@ impl<Window: WindowMethods> IOCompositor<Window> {

(Msg::ReturnUnusedNativeSurfaces(native_surfaces),
ShutdownState::NotShuttingDown) => {
self.surface_map.insert_surfaces(&self.native_display, native_surfaces);
if self.native_display.is_some() {
Copy link
Contributor

@frewsxcv frewsxcv Apr 25, 2016

Choose a reason for hiding this comment

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

if let Some(ref native_display) = self.native_display {
    self.surface_map.insert_surfaces(native_display, native_surfaces);
}

@Manishearth
Copy link
Member

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit cc3c80a has been approved by Manishearth

@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 Apr 25, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit cc3c80a with merge 67540e9...

bors-servo pushed a commit that referenced this pull request Apr 25, 2016
…shearth

Webrender no nativewindow

@glennw this fixed the panic when using webrender and no native display is available.

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

💔 Test failed - linux-dev

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 26, 2016
@@ -2220,7 +2227,8 @@ impl<Window: WindowMethods> IOCompositor<Window> {
fn initialize_compositing(&mut self) {
if self.webrender.is_none() {
let show_debug_borders = opts::get().show_debug_borders;
self.context = Some(rendergl::RenderContext::new(self.native_display.clone(),
// We can unwrap() native_display because it's only None when using webrender.
self.context = Some(rendergl::RenderContext::new(self.native_display.unwrap().clone(),
Copy link
Member

Choose a reason for hiding this comment

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

Please use .expect("native_display should be Some when not using Webrender") instead of .unwrap()

@Manishearth
Copy link
Member

@bors-servo delegate+

feel free to r=me after you fix the unwrap

@bors-servo
Copy link
Contributor

✌️ @fabricedesre can now approve this pull request

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 26, 2016
@Manishearth
Copy link
Member

@bors r+

@Manishearth
Copy link
Member

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 92526a4 has been approved by Manishearth

@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 Apr 26, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit 92526a4 with merge 2c7d1d1...

bors-servo pushed a commit that referenced this pull request Apr 26, 2016
…shearth

Webrender no nativewindow

@glennw this fixed the panic when using webrender and no native display is available.

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

💔 Test failed - linux-dev

@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 26, 2016
@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Apr 26, 2016
@@ -2220,7 +2227,9 @@ impl<Window: WindowMethods> IOCompositor<Window> {
fn initialize_compositing(&mut self) {
if self.webrender.is_none() {
let show_debug_borders = opts::get().show_debug_borders;
self.context = Some(rendergl::RenderContext::new(self.native_display.clone(),
// We can unwrap() native_display because it's only None when using webrender.
Copy link
Member

Choose a reason for hiding this comment

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

heh, CI doesn't like this comment, just make it unwrap instead of unwrap()

Run etc/ci/check_no_unwrap.sh to verify it works, and squash the commits. Feel free to @bors-servo r=Manishearth after that.

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Apr 26, 2016
@fabricedesre
Copy link
Contributor Author

@bors-servo r=Manishearth

@bors-servo
Copy link
Contributor

📌 Commit aac7101 has been approved by Manishearth

@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 Apr 26, 2016
@bors-servo
Copy link
Contributor

⌛ Testing commit aac7101 with merge 5ef355c...

bors-servo pushed a commit that referenced this pull request Apr 26, 2016
…shearth

Webrender no nativewindow

@glennw this fixed the panic when using webrender and no native display is available.

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

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt

@bors-servo bors-servo merged commit aac7101 into servo:master Apr 26, 2016
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 26, 2016
@fabricedesre fabricedesre deleted the webrender-no-nativewindow branch April 26, 2016 03:39
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.

None yet

5 participants