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

Create own file for background calculations in layout #19686

Merged
merged 1 commit into from Jan 7, 2018
Merged

Create own file for background calculations in layout #19686

merged 1 commit into from Jan 7, 2018

Conversation

@pyfisch
Copy link
Contributor

pyfisch commented Jan 4, 2018 •
edited by SimonSapin

See #19676

  • ./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 change is Reviewable
@highfive
Copy link

highfive commented Jan 4, 2018

Heads up! This PR modifies the following files:

  • @emilio: components/layout/display_list_builder.rs, components/layout/background.rs, components/layout/lib.rs
@highfive
Copy link

highfive commented Jan 4, 2018

warning Warning warning

  • These commits modify layout code, but no tests are modified. Please consider adding a test!
@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 4, 2018

Maybe compute_background_placement should also be moved but while it only is a method of Fragment to access self.border_padding it depends on a lot of inputs.
@emilio
emilio reviewed Jan 4, 2018
View changes
Copy link
Member

emilio left a comment

Maybe we should consider moving this to gfx? Or to a dedicated layout directory like layout/display_list/ (moving display_list_builder to layout/display_list/builder.rs)?
@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 4, 2018

Maybe we should consider moving this to gfx? Or to a dedicated layout directory like layout/display_list/ (moving display_list_builder to layout/display_list/builder.rs)?

I am not sure how layout and gfx should be separated. From what I can see gfx mainly handles fonts. It also has the display lists code but it would be no longer need if only servos display lists are used. (Some code would probably remain.)

I am fine with both suggestions.
@emilio
Copy link
Member

emilio commented Jan 4, 2018

@mbrubeck or @glennw, any opinion on the discussion above?
@emilio
Copy link
Member

emilio commented Jan 6, 2018

Given radio silence, I'd go with layout/display_list for now, does that sound fine @pyfisch? :)
@pyfisch pyfisch force-pushed the pyfisch:layout-background branch from e2bafc2 to 5589410 Jan 6, 2018
@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 6, 2018

Updated. I have also moved the webrender_helpers. See the commit message.
@pyfisch
Create own file for background calculations in layout
Loading status checks…
ea062e6 
Move display_list_builder.rs and webrender_helpers.rs
along with the new file to components/layout/display_list/

Remove apparently unused IdType enum.
Only variant used was OverflowClip.

See #19676
@pyfisch pyfisch force-pushed the pyfisch:layout-background branch from 5589410 to ea062e6 Jan 6, 2018
@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 7, 2018

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

⌛ Trying commit ea062e6 with merge 307fe60...
bors-servo added a commit that referenced this pull request Jan 7, 2018
@bors-servo
Auto merge of #19686 - pyfisch:layout-background, r=<try>
Loading status checks…
307fe60 
Create own file for background calculations in layout

See #19676

<!-- 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 _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

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

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

bors-servo commented Jan 7, 2018

💔 Test failed - mac-rel-wpt2
@pyfisch
Copy link
Contributor Author

pyfisch commented Jan 7, 2018

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

bors-servo commented Jan 7, 2018

⌛ Trying commit ea062e6 with merge c21323c...
bors-servo added a commit that referenced this pull request Jan 7, 2018
@bors-servo
Auto merge of #19686 - pyfisch:layout-background, r=<try>
Loading status checks…
c21323c 
Create own file for background calculations in layout

See #19676

<!-- 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 _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

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

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

bors-servo commented Jan 7, 2018

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
State: approved= try=True
@emilio
emilio approved these changes Jan 7, 2018
View changes
Copy link
Member

emilio left a comment

LGTM, thanks a lot!
@emilio
Copy link
Member

emilio commented Jan 7, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

📌 Commit ea062e6 has been approved by emilio
@highfive highfive assigned emilio and unassigned Manishearth Jan 7, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Jan 7, 2018

⌛ Testing commit ea062e6 with merge 8e226fe...
bors-servo added a commit that referenced this pull request Jan 7, 2018
@bors-servo
Auto merge of #19686 - pyfisch:layout-background, r=emilio
Loading status checks…
8e226fe 
Create own file for background calculations in layout

See #19676

<!-- 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 _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

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

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

bors-servo commented Jan 7, 2018

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: emilio
Pushing 8e226fe to master...
@bors-servo bors-servo merged commit ea062e6 into servo:master Jan 7, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
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
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@pyfisch @highfive @emilio @bors-servo @Manishearth
You can’t perform that action at this time.