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

Replace Servo DL items with WR ones #21348

Merged
merged 3 commits into from Aug 24, 2018

Conversation

Projects
None yet
6 participants
@pyfisch
Contributor

pyfisch commented Aug 6, 2018

The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.

Some more internal items like line, text shadow and iframe can definitively be replaced with WebRender equivalents but I think the PR is already quite huge. If WebRender would expose a quite minimal API which allowed servo to directly push items onto the display list most of webrender_helpers boilerplate code could go away. As WebRender performs normalization of gradients this would need to be called by servo explicitly in this case.

It should be noted that gradient borders don't actually work neither with the old version nor with this PR as the measurements are all set to zero.

Part of #19676


This change is Reviewable

Replace Servo DL items with WR ones
The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.
@highfive

This comment has been minimized.

highfive commented Aug 6, 2018

Heads up! This PR modifies the following files:

  • @emilio: components/layout/display_list/items.rs, components/layout/display_list/builder.rs, components/layout/display_list/background.rs, components/layout/display_list/webrender_helpers.rs
@highfive

This comment has been minimized.

highfive commented Aug 6, 2018

warning Warning warning

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

This comment has been minimized.

Contributor

pyfisch commented Aug 7, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 7, 2018

⌛️ Trying commit 3d2957c with merge e373497...

bors-servo added a commit that referenced this pull request Aug 7, 2018

Auto merge of #21348 - pyfisch:only-webrender-items, r=<try>
Replace Servo DL items with WR ones

The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.

Some more internal items like line, text shadow and iframe can definitively be replaced with WebRender equivalents but I think the PR is already quite huge. If WebRender would expose a quite minimal API which allowed servo to directly push items onto the display list most of webrender_helpers boilerplate code could go away. As WebRender performs normalization of gradients this would need to be called by servo explicitly in this case.

It should be noted that gradient borders don't actually work neither with the old version nor with this PR as the measurements are all set to zero.

Part of #19676

<!-- 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/21348)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 7, 2018

💔 Test failed - linux-rel-wpt

Do not emit DL items for zero images
Fixes wpt test failure.
@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Aug 16, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 16, 2018

⌛️ Trying commit 1c438ed with merge b8eb421...

bors-servo added a commit that referenced this pull request Aug 16, 2018

Auto merge of #21348 - pyfisch:only-webrender-items, r=<try>
Replace Servo DL items with WR ones

The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.

Some more internal items like line, text shadow and iframe can definitively be replaced with WebRender equivalents but I think the PR is already quite huge. If WebRender would expose a quite minimal API which allowed servo to directly push items onto the display list most of webrender_helpers boilerplate code could go away. As WebRender performs normalization of gradients this would need to be called by servo explicitly in this case.

It should be noted that gradient borders don't actually work neither with the old version nor with this PR as the measurements are all set to zero.

Part of #19676

<!-- 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/21348)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 16, 2018

💔 Test failed - linux-rel-css

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Aug 16, 2018

sccache: error : No space left on device (os error 28)

@jdm

This comment has been minimized.

Member

jdm commented Aug 16, 2018

@bors-servo retry

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 16, 2018

⌛️ Trying commit 1c438ed with merge 7f8d669...

bors-servo added a commit that referenced this pull request Aug 16, 2018

Auto merge of #21348 - pyfisch:only-webrender-items, r=<try>
Replace Servo DL items with WR ones

The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.

Some more internal items like line, text shadow and iframe can definitively be replaced with WebRender equivalents but I think the PR is already quite huge. If WebRender would expose a quite minimal API which allowed servo to directly push items onto the display list most of webrender_helpers boilerplate code could go away. As WebRender performs normalization of gradients this would need to be called by servo explicitly in this case.

It should be noted that gradient borders don't actually work neither with the old version nor with this PR as the measurements are all set to zero.

Part of #19676

<!-- 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/21348)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 16, 2018

☀️ Test successful - linux-rel-css, linux-rel-wpt
State: approved= try=True

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Aug 17, 2018

I think highfive should have removed the tests-failed label?

Are there any major issues remaining with the PR?

@pyfisch

This comment has been minimized.

Contributor

pyfisch commented Aug 22, 2018

r? @emilio

Can you please review the PR?

@highfive highfive assigned emilio and unassigned asajeffrey Aug 22, 2018

@jdm

This comment has been minimized.

Member

jdm commented Aug 23, 2018

@bors-servo r+
Thanks for doing this work!

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 23, 2018

📌 Commit 1c438ed has been approved by jdm

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 23, 2018

⌛️ Testing commit 1c438ed with merge c5131a2...

bors-servo added a commit that referenced this pull request Aug 23, 2018

Auto merge of #21348 - pyfisch:only-webrender-items, r=jdm
Replace Servo DL items with WR ones

The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.

Some more internal items like line, text shadow and iframe can definitively be replaced with WebRender equivalents but I think the PR is already quite huge. If WebRender would expose a quite minimal API which allowed servo to directly push items onto the display list most of webrender_helpers boilerplate code could go away. As WebRender performs normalization of gradients this would need to be called by servo explicitly in this case.

It should be noted that gradient borders don't actually work neither with the old version nor with this PR as the measurements are all set to zero.

Part of #19676

<!-- 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/21348)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 23, 2018

💔 Test failed - mac-rel-wpt2

@jdm

This comment has been minimized.

Member

jdm commented Aug 23, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 23, 2018

⌛️ Testing commit 1c438ed with merge 54b387a...

bors-servo added a commit that referenced this pull request Aug 23, 2018

Auto merge of #21348 - pyfisch:only-webrender-items, r=jdm
Replace Servo DL items with WR ones

The Servo internal display list items are already pretty much
equivalent to the WebRender ones. Except that Servo items contain
base information and associated glyphs and gradient stops which are
stored implicitly in WebRender. Remove the display items for
rectangles, text, images, border, gradients and box shadow and
replace them with their WebRender counter parts.

Some more internal items like line, text shadow and iframe can definitively be replaced with WebRender equivalents but I think the PR is already quite huge. If WebRender would expose a quite minimal API which allowed servo to directly push items onto the display list most of webrender_helpers boilerplate code could go away. As WebRender performs normalization of gradients this would need to be called by servo explicitly in this case.

It should be noted that gradient borders don't actually work neither with the old version nor with this PR as the measurements are all set to zero.

Part of #19676

<!-- 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/21348)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Aug 24, 2018

@bors-servo bors-servo merged commit 1c438ed into servo:master Aug 24, 2018

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

@pyfisch pyfisch deleted the pyfisch:only-webrender-items branch Aug 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment