layout: Force reflow in the sequential fallback of block format context #16458

stshine · 2017-04-14T15:56:24Z

When reflowing a block format context during the inorder traversal,
propagate restyle damage manually to its children since they were
already reflowed. Also, test the border box to see if it can fit into
floats according to CSS 2.1 § 9.5.

Improves reddit and yahoo.

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

stshine · 2017-04-14T15:56:36Z

@bors-servo try

bors-servo · 2017-04-14T15:56:40Z

⌛ Trying commit 40ef73f with merge 91a7a9b...


        Auto merge of #16458 - stshine:sequential-fallback, r=<try>

layout: Force reflow in the sequential fallback of block format context When reflowing a block format context during the inorder traversal, propagate restyle damage manually to its children since they were already reflowed. Also, test the border box to see if it can fit into floats according to CSS 2.1 § 9.5. Improves reddit.  ---  - [ ] `./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 _____

bors-servo · 2017-04-14T16:10:00Z

💔 Test failed - linux-rel-css

stshine · 2017-04-14T16:15:55Z

This pull request is ready for review.

r? @notriddle or @pcwalton

notriddle · 2017-04-14T16:23:53Z

@bors-servo try again

bors-servo · 2017-04-14T16:23:56Z

⌛ Trying commit 9e32111 with merge 1df7f04...


        Auto merge of #16458 - stshine:sequential-fallback, r=<try>

layout: Force reflow in the sequential fallback of block format context When reflowing a block format context during the inorder traversal, propagate restyle damage manually to its children since they were already reflowed. Also, test the border box to see if it can fit into floats according to CSS 2.1 § 9.5. Improves reddit. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #__ (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16458)

bors-servo · 2017-04-14T17:22:40Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
State: approved= try=True

emilio · 2017-04-16T15:18:45Z

components/layout/block.rs

@@ -1491,7 +1491,7 @@ impl BlockFlow {
        debug_assert!(!self.is_inline_flex_item());

        // Compute the available space for us, based on the actual floats.
-        let rect = self.base.floats.available_rect(Au(0),


Just curious, is it this change or the force_reflow stuff what makes the new tests pass?

I'm pretty sure they are fixed by force reflow because all the blocks in these two tests have zero margins.

Oh, right. Could we add a test that exercises this? Also, I've got another question, why only reflow and not also REPOSITION? There's a long time since I looked at this code, but seems that it could also be needed?

Whoops, didn't see you comment, Sure I can add a test that utilize both :) I'm not famillar with incremental layout infrastructure, but they are the only flags AssignISizes and AssignBSizes check, so if they were any problem please comment :)

You're totally right. I have looked to the details now. we only use REPOSITION to handle absolute positioning, not all out-of-flows as I thought. Also, it's handled in a later phase, so no need to do it here.

stshine · 2017-04-21T14:26:46Z

There is a regression I may introduce in this pull request, let me see if I can fix it.

stshine · 2017-04-22T03:05:15Z

@emilio @notriddle After testing for several days I noticed that we do not advance the margin collapse info before in-order traversal, so floating does not take the margin into account both when the child is block formatting context or not. Fixing this would be quite complicated because the margin collapse info of the child is not computed when the in-order traversal happens, and since a floating rewrite is proposed I decided to revert the float ceiling change.

pcwalton · 2017-04-22T03:35:00Z

components/layout/sequential.rs

@@ -38,16 +38,22 @@ pub fn resolve_generated_content(root: &mut Flow, layout_context: &LayoutContext
 }

 pub fn traverse_flow_tree_preorder(root: &mut Flow,
-                                   layout_context: &LayoutContext) {
+                                   layout_context: &LayoutContext,
+                                   force_reflow: bool) {


Could you make this parameter an enum so as to improve clarity?

pcwalton · 2017-04-22T03:36:21Z

components/layout/sequential.rs

-            assign_block_sizes: AssignBSizes) {
+            assign_block_sizes: AssignBSizes,
+            force_reflow: bool) {
+        if force_reflow {


Add a comment saying when this can happen.

stshine · 2017-04-23T00:18:50Z

Updated according to comments.

emilio · 2017-04-23T00:27:20Z

components/layout/incremental.rs

@@ -7,6 +7,12 @@ use style::computed_values::float;
 use style::selector_parser::RestyleDamage;
 use style::servo::restyle_damage::{REFLOW, RECONSTRUCT_FLOW};

+#[derive(Clone, Copy, PartialEq)]
+pub enum RelayoutMode {


I believe a comment here saying why is this used and where would be helpful, but apart from this, it looks good.

emilio · 2017-04-23T00:27:39Z

@bors-servo delegate+

bors-servo · 2017-04-23T00:27:40Z

✌️ @stshine can now approve this pull request


        Force reflow in the sequential fallback of block format context

When reflowing a block format context during the inorder traversal, propagate restyle damage manually to its children since they were already reflowed.

bors-servo · 2017-04-23T00:34:58Z

📌 Commit 68f74d5 has been approved by pcwalton,emilio

stshine · 2017-04-23T00:35:11Z

@bors-servo r=pcwalton,emilio

bors-servo · 2017-04-23T00:35:11Z

💡 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: #16573

bors-servo · 2017-04-23T00:35:12Z

📌 Commit 68f74d5 has been approved by pcwalton,emilio

bors-servo · 2017-04-23T00:38:56Z

⌛ Testing commit 68f74d5 with merge 242ca8f...


        Auto merge of #16458 - stshine:sequential-fallback, r=pcwalton,emilio

layout: Force reflow in the sequential fallback of block format context When reflowing a block format context during the inorder traversal, propagate restyle damage manually to its children since they were already reflowed. Also, test the border box to see if it can fit into floats according to CSS 2.1 § 9.5. Improves reddit and yahoo. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #__ (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16458)

bors-servo · 2017-04-23T00:53:15Z

💔 Test failed - linux-rel-wpt

jdm · 2017-04-23T02:43:22Z

@bors-servo: retry

#16574

bors-servo · 2017-04-23T03:05:40Z

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

bors-servo · 2017-04-23T03:17:51Z

💔 Test failed - linux-rel-wpt

stshine · 2017-04-23T04:55:20Z

@bors-servo: retry

#16574

bors-servo · 2017-04-23T04:55:24Z

⌛ Testing commit 68f74d5 with merge 541db5f...


        Auto merge of #16458 - stshine:sequential-fallback, r=pcwalton,emilio

layout: Force reflow in the sequential fallback of block format context When reflowing a block format context during the inorder traversal, propagate restyle damage manually to its children since they were already reflowed. Also, test the border box to see if it can fit into floats according to CSS 2.1 § 9.5. Improves reddit and yahoo. ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #__ (github issue number if applicable).  - [X] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16458)

bors-servo · 2017-04-23T05:42:11Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-msvc-dev
Approved by: pcwalton,emilio
Pushing 541db5f to master...

highfive added the S-tests-failed label Apr 14, 2017

stshine force-pushed the stshine:sequential-fallback branch from 40ef73f to 9e32111 Apr 14, 2017

highfive added S-awaiting-review and removed S-tests-failed labels Apr 14, 2017

highfive assigned notriddle Apr 14, 2017

emilio reviewed Apr 16, 2017

View changes

stshine changed the title ~~layout: Force reflow in the sequential fallback of block format context~~ [WIP] layout: Force reflow in the sequential fallback of block format context Apr 21, 2017

stshine force-pushed the stshine:sequential-fallback branch from 9e32111 to 483b8da Apr 22, 2017

stshine changed the title ~~[WIP] layout: Force reflow in the sequential fallback of block format context~~ layout: Force reflow in the sequential fallback of block format context Apr 22, 2017

pcwalton reviewed Apr 22, 2017

View changes

components/layout/sequential.rs Outdated

assign_block_sizes: AssignBSizes) {

assign_block_sizes: AssignBSizes,

force_reflow: bool) {

if force_reflow {

This comment has been minimized.

Sign in to view

pcwalton Apr 22, 2017
Contributor

Add a comment saying when this can happen.

stshine force-pushed the stshine:sequential-fallback branch from 483b8da to 3ba74ff Apr 23, 2017

emilio approved these changes Apr 23, 2017

View changes

Force reflow in the sequential fallback of block format context

Loading status checks…

68f74d5

When reflowing a block format context during the inorder traversal, propagate restyle damage manually to its children since they were already reflowed.

stshine force-pushed the stshine:sequential-fallback branch from 3ba74ff to 68f74d5 Apr 23, 2017

highfive added S-awaiting-merge and removed S-awaiting-review labels Apr 23, 2017

highfive assigned pcwalton and unassigned notriddle Apr 23, 2017

highfive added S-tests-failed and removed S-awaiting-merge labels Apr 23, 2017

highfive added the S-tests-failed label Apr 23, 2017

highfive added S-awaiting-merge and removed S-tests-failed labels Apr 23, 2017

bors-servo merged commit 68f74d5 into servo:master Apr 23, 2017
2 of 3 checks passed

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Apr 23, 2017

stshine deleted the stshine:sequential-fallback branch Apr 23, 2017

servo / servo

layout: Force reflow in the sequential fallback of block format context #16458

layout: Force reflow in the sequential fallback of block format context #16458

stshine commented Apr 14, 2017 •

edited

stshine commented Apr 14, 2017

bors-servo commented Apr 14, 2017

bors-servo commented Apr 14, 2017

stshine commented Apr 14, 2017

notriddle commented Apr 14, 2017

bors-servo commented Apr 14, 2017

bors-servo commented Apr 14, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

stshine commented Apr 21, 2017

stshine commented Apr 22, 2017 •

edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

stshine commented Apr 23, 2017

This comment has been minimized.

emilio commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

stshine commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

jdm commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

stshine commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

servo / servo

Join GitHub today

layout: Force reflow in the sequential fallback of block format context #16458

layout: Force reflow in the sequential fallback of block format context #16458

Conversation

stshine commented Apr 14, 2017 • edited

stshine commented Apr 14, 2017

bors-servo commented Apr 14, 2017

bors-servo commented Apr 14, 2017

stshine commented Apr 14, 2017

notriddle commented Apr 14, 2017

bors-servo commented Apr 14, 2017

bors-servo commented Apr 14, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

stshine commented Apr 21, 2017

stshine commented Apr 22, 2017 • edited

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

stshine commented Apr 23, 2017

This comment has been minimized.

emilio commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

stshine commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

jdm commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

stshine commented Apr 23, 2017

bors-servo commented Apr 23, 2017

bors-servo commented Apr 23, 2017

stshine commented Apr 14, 2017 •

edited

stshine commented Apr 22, 2017 •

edited