Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uplayout: Rewrite the block formatting context/float inline-size speculation code. #10085
Conversation
|
Surely there's something in this PR that's based on a spec requirement that you can point to. |
|
Rebased. |
|
|
r=mbrubeck with one typo fix -S-awaiting-review +S-needs-code-changes Reviewed 7 of 14 files at r1, 5 of 6 files at r2, 2 of 2 files at r3. components/layout/block.rs, line 844 [r3] (raw file): Comments from the review on Reviewable.io |
|
@bors-servo: r=mbrubeck |
|
@bors-servo r+ |
|
|
bors-servo
added a commit
that referenced
this pull request
Mar 24, 2016
layout: Rewrite the block formatting context/float inline-size speculation code.
The old code tried to do the speculation as a single bottom-up pass
after intrinsic inline-size calculation, which was unable to handle
cases like this:
<div>
<div style="float: left">Foo</div>
</div>
<div>
<div style="overflow: hidden">Bar</div>
</div>
No single bottom-up pass could possibly handle this case, because the
inline-size of the float flowing out of the "Foo" block could never make
it down to the "Bar" block, where it is needed for speculation.
On the pages I tried, this regresses layout performance by 1%-2%.
I first noticed this breaking some pages, like the Google SERPs, several
months ago.
r? @mbrubeck
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10085)
<!-- Reviewable:end -->
|
|
|
|
bors-servo
added a commit
that referenced
this pull request
Mar 25, 2016
layout: Rewrite the block formatting context/float inline-size speculation code.
The old code tried to do the speculation as a single bottom-up pass
after intrinsic inline-size calculation, which was unable to handle
cases like this:
<div>
<div style="float: left">Foo</div>
</div>
<div>
<div style="overflow: hidden">Bar</div>
</div>
No single bottom-up pass could possibly handle this case, because the
inline-size of the float flowing out of the "Foo" block could never make
it down to the "Bar" block, where it is needed for speculation.
On the pages I tried, this regresses layout performance by 1%-2%.
I first noticed this breaking some pages, like the Google SERPs, several
months ago.
r? @mbrubeck
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10085)
<!-- Reviewable:end -->
|
|
|
Does not appear to be an intermittent: |
pcwalton
added a commit
to pcwalton/servo
that referenced
this pull request
Mar 25, 2016
This fixes `margin-collapse-104.htm`, which is currently accidentally passing due to lack of servo#10085. When that PR lands, then that will become a representative test case.
|
Test failure fixed in #10198, which this is now blocked on. That test was accidentally passing. |
bors-servo
added a commit
that referenced
this pull request
Mar 26, 2016
layout: Allow floats to have negative ceilings due to negative margins. This fixes `margin-collapse-104.htm`, which is currently accidentally passing due to lack of #10085. When that PR lands, then that will become a representative test case. r? @mbrubeck <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10198) <!-- Reviewable:end -->
|
@bors-servo: r=mbrubeck (No changes here, just rebasing on top of #10198) |
|
|
bors-servo
added a commit
that referenced
this pull request
Mar 26, 2016
layout: Rewrite the block formatting context/float inline-size speculation code.
The old code tried to do the speculation as a single bottom-up pass
after intrinsic inline-size calculation, which was unable to handle
cases like this:
<div>
<div style="float: left">Foo</div>
</div>
<div>
<div style="overflow: hidden">Bar</div>
</div>
No single bottom-up pass could possibly handle this case, because the
inline-size of the float flowing out of the "Foo" block could never make
it down to the "Bar" block, where it is needed for speculation.
On the pages I tried, this regresses layout performance by 1%-2%.
I first noticed this breaking some pages, like the Google SERPs, several
months ago.
r? @mbrubeck
<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10085)
<!-- Reviewable:end -->
speculation code.
The old code tried to do the speculation as a single bottom-up pass
after intrinsic inline-size calculation, which was unable to handle
cases like this:
<div>
<div style="float: left">Foo</div>
</div>
<div>
<div style="overflow: hidden">Bar</div>
</div>
No single bottom-up pass could possibly handle this case, because the
inline-size of the float flowing out of the "Foo" block could never make
it down to the "Bar" block, where it is needed for speculation.
On the pages I tried, this regresses layout performance by 1%-2%.
I first noticed this breaking some pages, like the Google SERPs, several
months ago.
|
|
|
Regression: #10237 |
bors-servo
added a commit
that referenced
this pull request
Apr 30, 2016
Remove dead code left over from #10085 cc @pcwalton <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/10941) <!-- Reviewable:end -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
pcwalton commentedMar 19, 2016
The old code tried to do the speculation as a single bottom-up pass
after intrinsic inline-size calculation, which was unable to handle
cases like this:
No single bottom-up pass could possibly handle this case, because the
inline-size of the float flowing out of the "Foo" block could never make
it down to the "Bar" block, where it is needed for speculation.
On the pages I tried, this regresses layout performance by 1%-2%.
I first noticed this breaking some pages, like the Google SERPs, several
months ago.
r? @mbrubeck
This change is