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

Add support for background-repeat: space and round #13331

Merged
merged 1 commit into from
Sep 22, 2016

Conversation

mrobinson
Copy link
Member

@mrobinson mrobinson commented Sep 20, 2016

Add support for new background-repeat modes and upgrade Euclid, which now has support for basic arithmetic with Size2D.


  • ./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 _____

This adds support for more background-repeat modes using the legacy
rasterization backend.


This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/background.mako.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Sep 20, 2016
@jdm
Copy link
Member

jdm commented Sep 20, 2016

error[E0004]: non-exhaustive patterns: `space` and `round` not covered
    --> /home/travis/build/servo/servo/target/debug/build/style-b398ea74b908472c/out/gecko_properties.rs:3222:42
     |
3222 |         let (repeat_x, repeat_y) = match servo {
     |                                          ^^^^^ patterns `space` and `round` not covered
error: aborting due to previous error
error: Could not compile `style`.
To learn more, run the command again with --verbose.
The command "./mach test-geckolib" exited with 101.

@mrobinson
Copy link
Member Author

Thanks. I've updated the PR, so hopefully this build error is fixed.

@glennw
Copy link
Member

glennw commented Sep 21, 2016

@mrobinson I landed a WR update with dummy settings for the tile spacing API change here https://github.com/servo/servo/pull/13309/files today. So it should be fine to update this PR to pass the new values through to WR if you like.

@mrobinson
Copy link
Member Author

@glennw Okay. I've updated the PR. WebRender support for background-repeat space should now be included.

@mrobinson
Copy link
Member Author

I think the Windows build failure is #13340.

@glennw
Copy link
Member

glennw commented Sep 21, 2016

r=me for the display list parts and cargo.lock. Someone else should check over the parser / geckolib bits. r? @emilio or @SimonSapin

@highfive highfive assigned emilio and unassigned nox Sep 21, 2016
@emilio
Copy link
Member

emilio commented Sep 21, 2016

r=me for the gecko changes.

@bors-servo: r=glennw,emilio

Thanks for this Martin!

@bors-servo
Copy link
Contributor

📌 Commit ccd43d6 has been approved by glennw,emilio

@highfive highfive assigned glennw and unassigned emilio Sep 21, 2016
@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 Sep 21, 2016
image_size: Au) {
if *size == Au::new(0) || image_size == Au::new(0) {
*position = Au::new(0);
*size =Au::new(0);
Copy link
Member

Choose a reason for hiding this comment

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

nit: Minor formatting nit, there's a space missing here and in the previous function. Also, I think the usual way we write an empty Au value is Au(0) directly, though not sure if this has changed recently (please disregard it if that's the case).

Please fix these nits if any test fails, or otherwise in a followup, thanks!

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks. These are fixed now.

@bors-servo
Copy link
Contributor

⌛ Testing commit ccd43d6 with merge e7d2f03...

bors-servo pushed a commit that referenced this pull request Sep 22, 2016
Add support for background-repeat: space and round

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

Add support for new background-repeat modes and upgrade Euclid, which now has support for basic arithmetic with Size2D.

---
<!-- 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: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

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

This adds support for more background-repeat modes using the legacy
rasterization backend.

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

💔 Test failed - linux-rel

@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 Sep 22, 2016
@highfive
Copy link

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform3d-translatez-001.htm
  └   → /css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-ref.htm b14318ccfd8a59b7b249d41521e178d617678823
Testing 902d90a8d198625335e738587fdfe81dcc90392d == b14318ccfd8a59b7b249d41521e178d617678823
/css-transforms-1_dev/html/transform3d-translatez-001.htm 902d90a8d198625335e738587fdfe81dcc90392d
/css-transforms-1_dev/html/reference/transform3d-translatez-notref.htm 902d90a8d198625335e738587fdfe81dcc90392d
Testing 902d90a8d198625335e738587fdfe81dcc90392d != 902d90a8d198625335e738587fdfe81dcc90392d

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

📌 Commit c571689 has been approved by emilio

@highfive highfive assigned emilio and unassigned glennw Sep 22, 2016
@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 Sep 22, 2016
@bors-servo
Copy link
Contributor

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

@bors-servo
Copy link
Contributor

📌 Commit c571689 has been approved by emilio

This adds support for more background-repeat modes using the legacy
rendering backend.
@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Sep 22, 2016
@mrobinson
Copy link
Member Author

@bors-servo r=emilio

@bors-servo
Copy link
Contributor

📌 Commit 68ae97f has been approved by emilio

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

⌛ Testing commit 68ae97f with merge efdd94a...

bors-servo pushed a commit that referenced this pull request Sep 22, 2016
Add support for background-repeat: space and round

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

Add support for new background-repeat modes and upgrade Euclid, which now has support for basic arithmetic with Size2D.

---
<!-- 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: -->
- [x] There are tests for these changes OR
- [ ] These changes do not require tests because _____

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

This adds support for more background-repeat modes using the legacy
rasterization backend.

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

💔 Test failed - mac-rel-wpt

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

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

@bors-servo
Copy link
Contributor

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

@bors-servo bors-servo merged commit 68ae97f into servo:master Sep 22, 2016
@mrobinson mrobinson deleted the background-repeat branch October 17, 2016 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-tests-failed The changes caused existing tests to fail.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants