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

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

Merged
merged 1 commit into from Sep 22, 2016
Merged

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

merged 1 commit into from Sep 22, 2016

Conversation

@mrobinson
Copy link
Member

mrobinson commented Sep 20, 2016 •
edited by jdm

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

highfive commented Sep 20, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/background.mako.rs
@jdm
Copy link
Member

jdm commented Sep 20, 2016 •
edited 

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 mrobinson force-pushed the mrobinson:background-repeat branch from 43d88e3 to 62ab2d9 Sep 21, 2016
@mrobinson
Copy link
Member Author

mrobinson commented Sep 21, 2016

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 mrobinson force-pushed the mrobinson:background-repeat branch from 62ab2d9 to ccd43d6 Sep 21, 2016
@mrobinson
Copy link
Member Author

mrobinson commented Sep 21, 2016

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

mrobinson commented Sep 21, 2016

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

bors-servo commented Sep 21, 2016

📌 Commit ccd43d6 has been approved by glennw,emilio
@highfive highfive assigned glennw and unassigned emilio Sep 21, 2016
@emilio
emilio reviewed Sep 21, 2016
View changes
components/layout/fragment.rs Outdated
image_size: Au) {
if *size == Au::new(0) || image_size == Au::new(0) {
*position = Au::new(0);
*size =Au::new(0);

This comment has been minimized.

Sign in to view
Copy link
@emilio

emilio Sep 21, 2016

Member

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!

This comment has been minimized.

Sign in to view
Copy link
@mrobinson

mrobinson Sep 22, 2016

Author Member

Thanks. These are fixed now.
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2016

⌛ Testing commit ccd43d6 with merge e7d2f03...
bors-servo added a commit that referenced this pull request Sep 22, 2016
@bors-servo
Auto merge of #13331 - mrobinson:background-repeat, r=glennw,emilio
Loading status checks…
e7d2f03 
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

bors-servo commented Sep 22, 2016

💔 Test failed - linux-rel
@highfive
Copy link

highfive commented Sep 22, 2016 

  ▶ 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
@mrobinson mrobinson force-pushed the mrobinson:background-repeat branch from ccd43d6 to c571689 Sep 22, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2016

📌 Commit c571689 has been approved by emilio
@highfive highfive assigned emilio and unassigned glennw Sep 22, 2016
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2016

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

  • There's another pull request that is currently being tested, blocking this pull request: #13351
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2016

📌 Commit c571689 has been approved by emilio
@mrobinson
Add support for background-repeat: space and round
Loading status checks…
68ae97f 
This adds support for more background-repeat modes using the legacy
rendering backend.
@mrobinson mrobinson force-pushed the mrobinson:background-repeat branch from c571689 to 68ae97f Sep 22, 2016
@mrobinson
Copy link
Member Author

mrobinson commented Sep 22, 2016

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

bors-servo commented Sep 22, 2016

📌 Commit 68ae97f has been approved by emilio
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2016

⌛ Testing commit 68ae97f with merge efdd94a...
bors-servo added a commit that referenced this pull request Sep 22, 2016
@bors-servo
Auto merge of #13331 - mrobinson:background-repeat, r=emilio
Loading status checks…
efdd94a 
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

bors-servo commented Sep 22, 2016

💔 Test failed - mac-rel-wpt
@jdm
Copy link
Member

jdm commented Sep 22, 2016

@bors-servo: retry
@bors-servo
Copy link
Contributor

bors-servo commented Sep 22, 2016

⚡ 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

bors-servo commented Sep 22, 2016

☀️ 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
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Sep 22, 2016
Merged
@glennw glennw mentioned this pull request Sep 23, 2016
Closed
@mrobinson mrobinson deleted the mrobinson:background-repeat branch Oct 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emilio emilio

Assignees

@emilio emilio

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
@mrobinson @highfive @jdm @glennw @emilio @bors-servo @nox
You can’t perform that action at this time.