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

Deal with changes to the WebRender API #14200

Merged
merged 2 commits into from Nov 16, 2016
Merged

Conversation

mrobinson
Copy link
Member

@mrobinson mrobinson commented Nov 14, 2016


  • ./mach build -d does not report any errors
  • ./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 they should not change behavior.

The WebRender display list is now similar to the Servo display list,
which simplifies the conversion.


This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @emilio: components/layout/webrender_helpers.rs

@highfive
Copy link

warning Warning warning

  • These commits modify layout 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 Nov 14, 2016
@mrobinson
Copy link
Member Author

@bors-servo try

bors-servo pushed a commit that referenced this pull request Nov 14, 2016
Deal with changes to the WebRender API

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they should not change behavior.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The WebRender display list is now similar to the Servo display list,
which simplifies the conversion.
@bors-servo
Copy link
Contributor

⌛ Trying commit 27a6111 with merge b1603ca...

Ms2ger
Ms2ger previously requested changes Nov 14, 2016
@@ -100,3 +100,7 @@ android_glue = "0.2"

[target.'cfg(not(target_os = "windows"))'.dependencies]
gaol = {git = "https://github.com/servo/gaol"}

[replace]
"webrender:0.8.0" = { path = '/home/martin/work/mozilla/webrender/webrender/' }
Copy link
Contributor

Choose a reason for hiding this comment

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

Uh...

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that's not supposed to be there!

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay. I've cleaned this up. Sorry, this wasn't ready for review yet.

@mrobinson
Copy link
Member Author

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 978f534 with merge 236f156...

bors-servo pushed a commit that referenced this pull request Nov 14, 2016
Deal with changes to the WebRender API

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they should not change behavior.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The WebRender display list is now similar to the Servo display list,
which simplifies the conversion.

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

@mrobinson
Copy link
Member Author

Okay, this should be ready to review now. r? @glennw, @pcwalton, or anyone else who feels confident.

@highfive highfive assigned glennw and unassigned Ms2ger Nov 14, 2016
@Ms2ger Ms2ger dismissed their stale review November 14, 2016 11:59

Comment addressed

@nox
Copy link
Contributor

nox commented Nov 14, 2016

@mrobinson There was no shader change in that bump?

@mrobinson
Copy link
Member Author

@nox No, no shaders were updated as far as I can tell.

@glennw
Copy link
Member

glennw commented Nov 14, 2016

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 978f534 has been approved by glennw

@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 14, 2016
@glennw
Copy link
Member

glennw commented Nov 14, 2016

@bors-servo try- r+ clean

@bors-servo
Copy link
Contributor

💡 This pull request was already approved, no need to approve it again.

@bors-servo
Copy link
Contributor

📌 Commit 978f534 has been approved by glennw

@glennw
Copy link
Member

glennw commented Nov 14, 2016

@bors-servo p=10

(Until this lands it's complicated working with a local WR override due to the version / API mismatch).

@glennw
Copy link
Member

glennw commented Nov 14, 2016

@bors-servo retry

@bors-servo
Copy link
Contributor

⌛ Testing commit 978f534 with merge 85a773a...

bors-servo pushed a commit that referenced this pull request Nov 14, 2016
Deal with changes to the WebRender API

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they should not change behavior.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The WebRender display list is now similar to the Servo display list,
which simplifies the conversion.

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

⚡ Previous build results for arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-css, linux-rel-wpt...

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive
Copy link

  ▶ FAIL [expected PASS] /_mozilla/css/inline_block_opacity_change.html
  └   → /_mozilla/css/inline_block_opacity_change.html aed6845267bba6c52d3aa69c4889449b454d8117
/_mozilla/css/inline_block_opacity_change_ref.html febec1b4730afab195f280694dad47200b09ea3c
Testing aed6845267bba6c52d3aa69c4889449b454d8117 == febec1b4730afab195f280694dad47200b09ea3c

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Nov 15, 2016
@mrobinson
Copy link
Member Author

@bors-servo
Copy link
Contributor

⚡ Previous build results for arm32, arm64, linux-dev, linux-rel-css, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-wpt...

@bors-servo
Copy link
Contributor

💔 Test failed - linux-rel-wpt

@highfive highfive added the S-tests-failed The changes caused existing tests to fail. label Nov 16, 2016
@highfive
Copy link

  ▶ FAIL [expected PASS] /_mozilla/css/iframe/positioning_margin.html
  └   → /_mozilla/css/iframe/positioning_margin.html d9d9e489bc5b4508c01b06c7ff92f19dcc2ece3c
/_mozilla/css/iframe/positioning_margin_ref.html 3a4684450cbc08c6e80ab78eb761e71881c74ab1
Testing d9d9e489bc5b4508c01b06c7ff92f19dcc2ece3c == 3a4684450cbc08c6e80ab78eb761e71881c74ab1

@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 Nov 16, 2016
@mrobinson
Copy link
Member Author

I extended the reftest-wait workaround to the other failing iframe test, perhaps it would be okay to try landing this one more time?

@mrobinson
Copy link
Member Author

@bors-servo try

@bors-servo
Copy link
Contributor

⌛ Trying commit 386567e with merge 3fdce39...

bors-servo pushed a commit that referenced this pull request Nov 16, 2016
Deal with changes to the WebRender API

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [x] These changes do not require tests because they should not change behavior.

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

The WebRender display list is now similar to the Servo display list,
which simplifies the conversion.

<!-- 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/14200)
<!-- Reviewable:end -->
@emilio
Copy link
Member

emilio commented Nov 16, 2016

@highfive highfive assigned emilio and unassigned glennw Nov 16, 2016
@bors-servo
Copy link
Contributor

📌 Commit 386567e has been approved by emilio,glennw

@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 16, 2016
@emilio
Copy link
Member

emilio commented Nov 16, 2016

I think the reftest-wait will help nothing, but it doesn't hurt either

@bors-servo
Copy link
Contributor

@bors-servo bors-servo merged commit 386567e into servo:master Nov 16, 2016
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Nov 16, 2016
@mrobinson mrobinson deleted the display-list branch November 16, 2016 12:38
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

7 participants