`content_size` for internal nodes incorrectly includes the node's left/top border.

[wlott]

It appears that for internal nodes the left/top border is included in the content_size, not just the padding. Leaf nodes, on the other hand, only include padding, not the border.

I'm pretty sure this is a bug and not the desired behavior because the original PR that added content_size (#573) explicitly calls out that the padding should be and border should not be included in content_size. It is also it is strange that there would be this left/top vs right/bottom asymmetry and difference in behavior between leaf and internal nodes.

taffy version

I tested this behavior against both taffy 0.7.7 (as used by Bevy 0.17) and 0.9.1 (latest at this moment).

Platform

Linux desktop, but nothing that I uncovered looks to depend on the platform.

What you did

I was trying to add a scrollbar to a Bevy UI and got some oddities that prompted me to dig into the implementation. Specifically, I was scrolling a grid that had both padding and border, a known number of rows, and all the rows given a fixed height. When I subtracted off nrows*rowheight, I was unable to match the result to my padding and border numbers. In digging through the implementation to try to explain the numbers I was getting, I tracked it down to the subject of this issue.

What went wrong

main.rs.txt is a simple testbed that highlights this. (Renamed to a supported extension.) It creates several fixed-sized leaf nodes and wraps them with boxes having various border and padding settings then prints out the results of layout:

TREE
└──  FLEX COL [x: 0    y: 0    w: 226  h: 328  content_w: 226  content_h: 328  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967308))
    ├──  FLEX ROW [x: 0    y: 0    w: 226  h: 50   content_w: 200  content_h: 50   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967298))
    │   └──  LEAF [x: 0    y: 0    w: 200  h: 50   content_w: 200  content_h: 50   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967297))
    ├──  FLEX ROW [x: 0    y: 50   w: 226  h: 53   content_w: 203  content_h: 53   border: l:0 r:0 t:0 b:0, padding: l:1 r:2 t:1 b:2] (NodeId(4294967300))
    │   └──  LEAF [x: 1    y: 1    w: 200  h: 50   content_w: 200  content_h: 50   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967299))
    ├──  FLEX ROW [x: 0    y: 103  w: 226  h: 73   content_w: 211  content_h: 61   border: l:11 r:12 t:11 b:12, padding: l:0 r:0 t:0 b:0] (NodeId(4294967302))
    │   └──  LEAF [x: 11   y: 11   w: 200  h: 50   content_w: 200  content_h: 50   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967301))
    ├──  FLEX ROW [x: 0    y: 176  w: 226  h: 76   content_w: 214  content_h: 64   border: l:11 r:12 t:11 b:12, padding: l:1 r:2 t:1 b:2] (NodeId(4294967304))
    │   └──  LEAF [x: 12   y: 12   w: 200  h: 50   content_w: 200  content_h: 50   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967303))
    └──  FLEX ROW [x: 0    y: 252  w: 226  h: 76   content_w: 226  content_h: 76   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967307))
        └──  FLEX ROW [x: 0    y: 0    w: 226  h: 76   content_w: 214  content_h: 64   border: l:11 r:12 t:11 b:12, padding: l:1 r:2 t:1 b:2] (NodeId(4294967306))
            └──  LEAF [x: 12   y: 12   w: 200  h: 50   content_w: 200  content_h: 50   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967305))

(I used the oddball padding/border values so that one could which values got added together.)

The first FLEX ROW (without any padding/border) has the exact same content_size as the leaf node. This is as expected.

The second FLEX ROW (with padding but no border) has the content_size inflated by the padding. Again, as expected.

The third FLEX ROW (with border but no padding) has the content_size inflated by just the left/top values. My expectation is that the content_size would stay the same.

The fourth FLEX ROW (with both) exhibits the sum of the two individual cases.

The last two FLEX ROWs show nesting an incorrectly content_sized box inside a box with no padding/border: the outer boxes content_size includes both the inner boxes padding and border--as it should. So at least the incorrect value is contained at the immediate parent and doesn't bleed further up the tree.

Additional information

I believe that this code is responsible for the above results. content_size starts out as zero then gets maxed with each item's right/bottom content extent. So it is a measure from the container's border box upper left to the lower right corner of the content. The lines referenced then expand the content_size by the container's right padding (calculated by subtracting the border and scrollbar gutter from the inset, which is the sum of the padding, border, and scrollbar gutter).

My supposition is that it should also subtract off the container's left/top border. (I'd also suggest renaming that accumulator something like content_extent to indicate that it is the max extent of the content, not the desired final content_size.)

This is just the flexbox case. I suspect that the other containers exhibit the same but I haven't explored them.

And finally, I'd be happy to put together a PR to address this (and the other containers) if this is in fact a bug and not desired behavior.

[alice-i-cecile]

And finally, I'd be happy to put together a PR to address this (and the other containers) if this is in fact a bug and not desired behavior.

I'd be happy to see this, but we should add a regression test that compares this behavior to the web standard. That will both ensure that we're actually observing a bug and prevent breaking it in the future. See the test_fixtures folder :)

[wlott]

I'd be happy to see this, but we should add a regression test that compares this behavior to the web standard. That will both ensure that we're actually observing a bug and prevent breaking it in the future. See the test_fixtures folder :)

I was just looking into that but ran into a hitch. Got chromedriver sorted, ran gentests, and one of the tests changed. Without any changes to the code. And that changed test case fails. I haven't dug into it more, but something is off. I'm going to open another issue with the details.

[wlott]

It looks like there are additional bugs that cause the test fixture stuff to produce what I suspect are false positives. I add a new test fixture with:

<!DOCTYPE html>
<html lang="en">
<head>
  <script src="../../scripts/gentest/test_helper.js"></script>
  <link rel="stylesheet" type="text/css" href="../../scripts/gentest/test_base_style.css">
  <title>
    Test description
  </title>
</head>
<body>

<div id="test-root" style="width: 100px; height: 50px; border-width: 1px 3px 5px 7px; border-style: solid; border-color: red; flex-direction: column; overflow: clip scroll; ">
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
  <div>XXXXXXXXXXXXXXXXXXXX</div>
</div>

</body>
</html>

When I load it into chrome, it appears as I would expect. The chrome dev tools report the client area of the test-root div as 90x44, which corresponds to the outer width of 100 minus the left and right borders of 3 and 7 and the outer height of 50 minus the top and bottom borders of 1 and 5. The console.log in test_helper.js reports the client width and height as 75 by 44, the scrollbar width as 15, and the scroll width and height as 200 by 200. Again, all of that lines up. In the gentest rust code, it calculates the expected scroll_width and scroll_height as the difference between the JS version of the scrollWidth/scrollHeight and clientWidth/clientHeight giving expected values of 125 and 156. The generated code compares layout.scroll_width() and layout.scroll_height() to those values. Good so far.

When the generated test runs, it reports the laid out tree as:

Computed tree:
TREE
└──  FLEX COL [x: 0    y: 0    w: 100  h: 50   content_w: 207  content_h: 201  border: l:7 r:3 t:1 b:5, padding: l:0 r:0 t:0 b:0] (NodeId(4294967317))
    ├──  LEAF [x: 7    y: 1    w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967297))
    ├──  LEAF [x: 7    y: 11   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967298))
    ├──  LEAF [x: 7    y: 21   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967299))
    ├──  LEAF [x: 7    y: 31   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967300))
    ├──  LEAF [x: 7    y: 41   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967301))
    ├──  LEAF [x: 7    y: 51   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967302))
    ├──  LEAF [x: 7    y: 61   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967303))
    ├──  LEAF [x: 7    y: 71   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967304))
    ├──  LEAF [x: 7    y: 81   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967305))
    ├──  LEAF [x: 7    y: 91   w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967306))
    ├──  LEAF [x: 7    y: 101  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967307))
    ├──  LEAF [x: 7    y: 111  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967308))
    ├──  LEAF [x: 7    y: 121  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967309))
    ├──  LEAF [x: 7    y: 131  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967310))
    ├──  LEAF [x: 7    y: 141  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967311))
    ├──  LEAF [x: 7    y: 151  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967312))
    ├──  LEAF [x: 7    y: 161  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967313))
    ├──  LEAF [x: 7    y: 171  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967314))
    ├──  LEAF [x: 7    y: 181  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967315))
    └──  LEAF [x: 7    y: 191  w: 75   h: 10   content_w: 200  content_h: 10   border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967316))

This shows the content_size as 207 by 201 which includes the left and top borders similar to my original test and not the 200 by 200 the javascript code reports. But the test passes, so layout.scroll_width() and layout.scroll_height() must be returning the expected 125 by 156. Looking at their definitions:

    /// Return the scroll width of the node.
    /// The scroll width is the difference between the width and the content width, floored at zero
    pub fn scroll_width(&self) -> f32 {
        f32_max(
            0.0,
            self.content_size.width + f32_min(self.scrollbar_size.width, self.size.width) - self.size.width
                + self.border.right,
        )
    }

    /// Return the scroll height of the node.
    /// The scroll height is the difference between the height and the content height, floored at zero
    pub fn scroll_height(&self) -> f32 {
        f32_max(
            0.0,
            self.content_size.height + f32_min(self.scrollbar_size.height, self.size.height) - self.size.height
                + self.border.bottom,
        )
    }

we see that they are adding the right and bottom borders to the content size. And that is why the math lines up: the content size has the top and left borders already added and then these add the right and bottom borders and then subtract off layout.size. layout.size includes the borders, padding, and client boxes, so these are returning content size minus (size - border).

My supposition is that content_size shouldn't include half of the borders and instead these scroll_* helper functions should handle both sides of the border box. That would make content_size match the scrollHeight and scrollWidth extracted from the element in chrome.

[wlott]

I've been working on fixing this all day, and well, it has snowballed way beyond what I first thought it would entail. Part of the problem is that I read the overflow spec. They don't describe a nice simple "content_size should be ..." Instead, they talk about the "scrollable overflow", "scrollable overflow area", "scrollable overflow rect", "scroll origin", etc. etc. I started implementing what they describe but doing so requires expanding the LayoutOutput to include the scroll origin and the extent of the overflow in all directions--not just the direction(s) you can scroll. And it looks like LayoutOutput is part of the publicly exposed interface so changing that probably wants more deliberation than some rando on the internet (i.e. me) throwing together a PR.

Even a scaled back delta of just not including the border left & top (aka the scroll origin) in content_size means changing all the layout code to account for it differently. And that is a lot of code with a lot of subtle interactions.

I'm still game if this is something the taffy project wants. But I'd want to make sure there was alignment on scope and direction before sinking that much effort into it.

[nicoburns]

@wlott Thanks for looking into this.

We'd definitely like to improve this functionality in Taffy, so if you're up for driving the changes that would be great. The below describes what I think the direction ought to be, but feel free to push back on anything if you disagree:

---

I think what we want to compute/store is the "scrollable overflow rectangle". Possibly excluding "The scroll container’s own padding box." (but also possibly not). This should be a Rect<f32> instead of the current Size<f32> so we have somewhere to store the negative overflow. It should also probably be renamed. I would recommend scrollable_overflow.

I agree that none of the borders should not be included in the content size, and should probably be accounted for in the scroll helpers. I think the confusion here has arisen from the fact that Taffy uses coordinates relative to the border box for most purposes, and in order for those to interoperate with a measurement that is relative to the padding box then the borders have to be accounted for at some point, but doing so as late as possible (i.e. in the scroll helpers) makes sense.

I don't think we need a general concept of "scroll origin". I think we can just assume that this is always the top-left of the padding box (at least for now).

I think Taffy is currently calculating the affect of a boxes's padding on the "scrollable overflow rect" wrong. I believe the method of computing the scroll overflow rectangle should be something like the union of:

1. The static positions of all in-flow children (with the container's padding, but without the child's own overflow scroll rect)
2. The relative adjusted positions of relatively positioned in-flow children (without adding padding, but with the child's own (positive) overflow scroll rect (if child is overflow: visible))
3. The absolute positions of absolute children (without adding padding, but with the child's own (positive) overflow scroll rect (if child is overflow: visible))

((i) can't cause negative overflow, so can be ignored for the top and left computations)

It's perhaps also worth bearing in mind what the use cases for this value are:

1. For scrollable nodes, it's used for calculating scroll width and scroll height. This is used to clamp scroll offset and draw scrollbars.
2. For non-scrollable nodes (specifically those with overflow: visible), it is used to calculate the area that may contain child content. This is used for optimising hit-testing (mouse hover): if the mouse position is outside the "scrollable overflow rect" then we can skip hit testing children of this node.

(perhaps you have other use cases in mind, but these are the ones that I am aware of)

We should make sure that whatever computation we end up with is fit for purpose for these use cases. Notably, the current setup is problematic for the hit testing as it means that elements positioned in the negative scroll overflow area get incorrectly excluded!

[nicoburns]

Oh, and a note that breaking changes are not considered a huge problem in Taffy. We try not to make breaking changes for no reason, but we're not going to hold back an important fix or feature for API stability reasons.

[wlott]

Okay then, I'll start working out the details. (I retired a couple weeks ago and was thinking that it would be fun to contribute to some open source projects--this can be my first!)

What you describe above is pretty close to my interpretation of the CSS overflow level 3 spec, although I think a strict reading of the spec might require that child contributions come from their possibly non-rectangular overflow area and that the overflow rects (the axis aligned bounding box of the area) be independently calculated per node. But the difference would only show up in the presence of transforms and using rects for the unioning would produce a conservative superset of the area based unioning. And both your target use cases would be better served by simple rects, so it makes sense that the client-exposed result should be rects. And then aggregating via rects or areas could be an implementation detail and we start with rects for simplicity and areas could be left as a potential future improvement.

[wlott]

Bumped into something I'm not sure how to handle. Near the end of 3.1. Managing Overflow: the overflow-x, overflow-y, and overflow properties, it says:

The visible/clip values of overflow compute to auto/hidden (respectively) if one of overflow-x or overflow-y is neither visible nor clip.

Some of the test fixtures (e.g. leaf/leaf_overflow_scrollbars_affect_available_space_y_axis.html) just set one of overflow-x or overflow-y. The other uses the initial value of visible and by that quoted rule, it computes to auto. Meanwhile, the overflow extraction code just goes ahead with the default of Overflow::Visible. If the content happens to overflow in that axis (and it does in the fixture mentioned above), chrome will use that auto setting, give it a scrollbar, and change the perpendicular axis's content width while taffy will not.

That particular test case is failing in my build (along with 1426 others...), but I have no idea if the failure is due to that divergence or the work-in-progress state of my edits. I noticed that divergence while I was looking into why.

Anyway, I'm not sure what approach to take to get chrome and taffy in alignment. Options I see:

• Change all the fixtures to explicitly set the perpendicular access to hidden and hence avoid that quoted rule.
• Change the gentest.rs code to apply that rule. But that rule requires auto support which taffy doesn't have. And it means taffy would have to invent semantics for the combinations that rule prohibits. (Okay, now that I've laid it out, this strikes me as poor option. Uh, included for completeness then.)
• Change the taffy code to apply that rule. Again, requires implementing auto to be complete.
• Change the taffy code to only apply the clip->hidden part of that rule and all of the fixtures to avoid triggering the visible->auto part. The extractor could check for places this gets missed and fail the extraction.

I'm leaning towards the last option I listed, but I wanted to hear what others think before diving in.

[wlott]

I am close. Oh so close. I even turned on the scroll_{width,height} checking in the generated tests for visible and clip nodes. But a small number of tests are failing. It looks like it might be related to this passage from CSS Overflow 3, section 2.2:

The border boxes of all boxes for which it is the containing block and whose border boxes are positioned not wholly in the unreachable scrollable overflow region, ...

I've implemented that check as:

if item_rect.right <= container_scroll_origin.x || item_rect.bottom <= container_scroll_origin.y {
    return;
}

But when I run that, this test fails:

generated::flex::width_smaller_then_content_with_flex_grow_unconstraint_size::width_smaller_then_content_with_flex_grow_unconstraint_size__border_box
generated::flex::width_smaller_then_content_with_flex_grow_unconstraint_size::width_smaller_then_content_with_flex_grow_unconstraint_size__content_box

because I compute the scroll_width of the top node as 70 but chrome computes it as 0.

And if I change those <= to <, then that one passes but these tests fail:

generated::block::block_margin_y_collapse_complex::block_margin_y_collapse_complex__border_box
generated::block::block_margin_y_collapse_complex::block_margin_y_collapse_complex__content_box
generated::block::block_margin_y_total_collapse_complex::block_margin_y_total_collapse_complex__border_box
generated::block::block_margin_y_total_collapse_complex::block_margin_y_total_collapse_complex__content_box

For both those test cases, I compute the scroll_height of the top and second child nodes as 7 and chrome computes them as 0.

These tests fail in both scenarios so aren't dependent on that comparison, but the fails also appear to be with regards to subtleties around what into include in the scrollable overflow:

generated::flex::flex_shrink_by_outer_margin_with_max_size::flex_shrink_by_outer_margin_with_max_size__border_box
generated::flex::flex_shrink_by_outer_margin_with_max_size::flex_shrink_by_outer_margin_with_max_size__content_box
generated::flex::justify_content_row_max_width_and_margin::justify_content_row_max_width_and_margin__border_box
generated::flex::justify_content_row_max_width_and_margin::justify_content_row_max_width_and_margin__content_box

I get 20 for scroll_height for the flex_shrink test and 10 for the scroll_width while again chrome gets 0.

I created a draft PR in case anyone wants to help puzzle out these last few CSS quirks or get a jump on reviewing.

[nicoburns]

Deeper review coming, but:

width_smaller_then_content_with_flex_grow_unconstraint_size because I compute the scroll_width of the top node as 70 but chrome computes it as 0.

I'm getting 70 out of both Chrome and Firefox when inspecting this manually using the devtools console.

[wlott]

I had it backwards in my write-up. When I use <= for the checks, my code produces 0 for scroll_width while chrome produces 70:

Computed tree:
TREE
└──  FLEX ROW [x: 0    y: 0    w: 0    h: 100  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..0 y: 0..100] (NodeId(4294967301))
    ├──  FLEX COL [x: 0    y: 0    w: 0    h: 100  scroll_w: 70   scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..70 y: 0..100] (NodeId(4294967298))
    │   └──  LEAF [x: 0    y: 0    w: 70   h: 100  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..70 y: 0..100] (NodeId(4294967297))
    └──  FLEX COL [x: 0    y: 0    w: 0    h: 100  scroll_w: 20   scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..20 y: 0..100] (NodeId(4294967300))
        └──  LEAF [x: 0    y: 0    w: 20   h: 100  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..20 y: 0..100] (NodeId(4294967299))

NodeId(4294967301).scroll_width() mismatch: expected 70 actual 0

The two flex columns are 0 wide at x=0 and get excluded by the <=. And the definition of "unreachable scroll overflow region":

the area beyond the scroll origin in either axis is considered the unreachable scrollable overflow region

does use the word "beyond" so that sounds to me like < is the "correct" choice.

[nicoburns]

I agree that < makes more sense to me (but also happy to match Chrome for this if we can).

For justify_content_row_max_width_and_margin:

<div id="test-root" style="width: 100px; max-width: 80px; justify-content: center; flex-direction: row;">
  <div style="height: 20px; width: 20px; margin-left: 100px;"></div>
</div>

Computed tree:
TREE
└──  FLEX ROW [x: 0    y: 0    w: 80   h: 20   scroll_w: 10   scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..90 y: 0..20] (NodeId(4294967298))
    └──  LEAF [x: 90   y: 0    w: 0    h: 20   scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..0 y: 0..20] (NodeId(4294967297))

NodeId(4294967298).scroll_width() mismatch: expected 0 actual 10

thread 'generated::flex::justify_content_row_max_width_and_margin::justify_content_row_max_width_and_margin__border_box' panicked at tests/generated/flex/justify_content_row_max_width_and_margin.rs:116:5:

The inner LEAF node getting scrollable_overflow_y: 0..20 looks wrong to me. I think that should be 0..0, and then outer node will correct itself. Possibly need to put a check for zero-area being unioning the box of the container?

[nicoburns]

flex_shrink_by_outer_margin_with_max_size looks to be the same issue but in the x axis rather than the y axis.

[wlott]

Ah, then the problem looks to be with the test extraction code. Or other assumptions. Because that inner div has no children, gentest produces a leaf node:

    let node0 = taffy
        .new_leaf(taffy::style::Style {
            overflow: taffy::geometry::Point { x: taffy::style::Overflow::Visible, y: taffy::style::Overflow::Visible },
            size: taffy::geometry::Size {
                width: taffy::style::Dimension::from_length(20f32),
                height: taffy::style::Dimension::from_length(20f32),
            },
            margin: taffy::geometry::Rect { left: length(100f32), right: zero(), top: zero(), bottom: zero() },
            ..Default::default()
        })
        .unwrap();

compute_leaf_layout returns a 0 by 20 node with a 0 by 20 scrollable overflow. That rises up to the outer flexbox, where is doesn't add that item's border box because it has zero area but it does add that item's scrollable overflow. And centering the 100px margin + 0px box within the 80px max width puts the box at x=90 expanding the outer flexbox's overflow out to 90.

But Chrome uses flexbox rules for that node. It ends up the same 0 by 20 in size, but with no children it collects no scrollable overflow from children. So the outer flexbox's scrollable overflow just contains the outer flexbox's padding box.

Except that the inner div's padding box should be part of its scrollable overflow. So the outer flexbox should get the same stuff in both cases.

Argh! I thought I was onto something.

[nicoburns]

Except that the inner div's padding box should be part of its scrollable overflow. So the outer flexbox should get the same stuff in both cases.

I think the trick here is that a padding-box that has zero-size in either dimension should be treated as 0 in both dimensions. So because the inner div is 0 by 20 that gets treated as 0 by 0, and then that has no effect when being unioned with other sizes.

[wlott]

Stranger and stranger. When given this test case:

<div id="test-root" style="display:block; ">
  <div id="inflow" style="display:block; overflow: hidden scroll; width: 100px; height: 100px; background: white; ">
    <div style="display:block; width: 0px; height: 200px; "></div>
  </div>
  <div id="out-of-flow" style="display:block; overflow: hidden scroll; width: 100px; height: 100px; background: white; ">
    <div style="display:block; width: 0px; height: 200px; position: absolute; "></div>
  </div>
  <div id="out-of-flow2" style="display:block; overflow: hidden scroll; width: 100px; height: 100px; background: white; ">
    <div style="display:block; width: 0px; height: 200px; position: absolute; ">
      <div style="display:block; width: 20px; height: 150px; background: blue;" ></div>
    </div>
  </div>
  <div id="baseline-inflow" style="display:block; overflow: hidden scroll; width: 100px; height: 100px; background: white; ">
    <div style="display:block; width: 1px; height: 200px; margin-left: 5px; background: red; "></div>
  </div>
  <div id="baseline-out-of-flow" style="display:block; overflow: hidden scroll; width: 100px; height: 100px; background: white; ">
    <div style="display:block; width: 1px; height: 200px; position: absolute; left: 10px; background: red; "></div>
  </div>
</div>

Chrome produces different scrollable overflow with zero-width children for in-flow and out-of-flow. id="inflow" gets a 200px scroll region but id="out-of-flow" does not. My code fails because it doesn't differentiate those cases and gives all of them 200px scroll regions:

Computed tree:
TREE
└──  BLOCK [x: 0    y: 0    w: 100  h: 500  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..100 y: 0..500] (NodeId(4294967308))
    ├──  BLOCK [x: 0    y: 0    w: 100  h: 100  scroll_w: 0    scroll_h: 100  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:15 h:0 scrollable_overflow: x:0..85 y: 0..200] (NodeId(4294967298))
    │   └──  LEAF [x: 0    y: 0    w: 0    h: 200  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..0 y: 0..200] (NodeId(4294967297))
    ├──  BLOCK [x: 0    y: 100  w: 100  h: 100  scroll_w: 0    scroll_h: 100  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:15 h:0 scrollable_overflow: x:0..85 y: 0..200] (NodeId(4294967300))
    │   └──  LEAF [x: 0    y: 0    w: 0    h: 200  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..0 y: 0..200] (NodeId(4294967299))
    ├──  BLOCK [x: 0    y: 200  w: 100  h: 100  scroll_w: 0    scroll_h: 100  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:15 h:0 scrollable_overflow: x:0..85 y: 0..200] (NodeId(4294967303))
    │   └──  BLOCK [x: 0    y: 0    w: 0    h: 200  scroll_w: 20   scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..20 y: 0..200] (NodeId(4294967302))
    │       └──  LEAF [x: 0    y: 0    w: 20   h: 150  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..20 y: 0..150] (NodeId(4294967301))
    ├──  BLOCK [x: 0    y: 300  w: 100  h: 100  scroll_w: 0    scroll_h: 100  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:15 h:0 scrollable_overflow: x:0..85 y: 0..200] (NodeId(4294967305))
    │   └──  LEAF [x: 5    y: 0    w: 1    h: 200  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..1 y: 0..200] (NodeId(4294967304))
    └──  BLOCK [x: 0    y: 400  w: 100  h: 100  scroll_w: 0    scroll_h: 100  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:15 h:0 scrollable_overflow: x:0..85 y: 0..200] (NodeId(4294967307))
        └──  LEAF [x: 10   y: 0    w: 1    h: 200  scroll_w: 0    scroll_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0 scrollbar: w:0 h:0 scrollable_overflow: x:0..1 y: 0..200] (NodeId(4294967306))

NodeId(4294967300).scroll_height() mismatch: expected 0 actual 100

thread 'generated::block::block_zero_width_overflow_y::block_zero_width_overflow_y__border_box' panicked at tests/generated/block/block_zero_width_overflow_y.rs:726:5:
Detected 1 mismatch(es)

But it isn't just the box size of the child item. The id="out-of-flow2" has the same 0x200 dimensions but inside it has a 20x150 box. Yet Chrome gives it a 200px scroll region even though there is only 150px of visible content.

(The two id="baseline..." cases use a 1px wide child just to establish that the difference only shows up with zero width children.)

I haven't found anything in any of the standards that would account for this difference. CSS overflow 3 just says that non-zero area border boxes of children and the scrollable overflow of children that don't otherwise clip it. It doesn't make any distinction between in-flow and out-of-flow. The child items in both my in-flow and out-of-flow cases have zero-area border boxes so the border boxes shouldn't be included. And they both have 0x200 scrollable overflow (because scrollable overflow includes ones own padding box).

So okay, I just add that same test that Chrome seems to be using: for absolutely positioned items in a block container, don't union their scrollable overflow in the container's overflow if their scrollable overflow has zero area.

And yeah! That test case I started this comment with now passes!

But then I had to go and add one more variation:

  <div id="out-of-flow3" style="display:block; overflow: hidden scroll; width: 100px; height: 100px; background: white; ">
    <div style="display:block; width: 0px; height: 150px; position: absolute; ">
      <div style="display:block; width: 0px; height: 200px; background: blue;" ></div>
    </div>
  </div>

My thinking was that the innermost div, being in-flow, would contribute its overflow to the middle div and then the middle div's overflow would be ignored by the outer div as described above.

It did not. Chrome has the middle div's scrollable area the same as its border/padding/content (they are all the same in this case) box. So that test case is back to failing: I have the scroll height as 50px while chrome has it as 0px.

Sigh.

[ickshonpe]

Really good to see someone digging into this. I spent a lot of hours trying to work out what was wrong by grinding through the same computed tree printouts but didn't make much progress and had too many other demands on my time.

[Hexlord]

I interacted with Yoga (a C++ flexbox layout engine) quite a bit, and it does not handle content_size at all, and that was not an issue in implementing scrolling: you can walk down the tree and compute scrollable node's content AABB manually (by reaching into closest scrollable ancestor) in user code, and apply scroll-related clipping on user side for rendering and input.

I also personally couldn't reason my way into scrollbar gutter making sense in practice (why would anyone create an in-flow scrollbar as soon as we start overflowing, instead of just making it be there from the start but invisible/inactive while we don't overflow). Obviously it exists in CSS, so I might be missing some use-case.

So this might sound like off-topic or be just my lack of context, but none of these features are worth the trouble / development time. Maybe the one advantage I could see would be better performance for computing content_size while the other ui values are hot in cache/registers, as opposed to doing it like I described above with manual content contribution to ancestor AABB during the separate tree walk (e.g. during rounding).

[nicoburns]

The main reason for having in Taffy is just so that the implementation can be shared, and each consumer doesn't have to implement it themselves.

It's true that you can compute it outside of Taffy, but the amount of effort is the same either way. If you want to do your own the you can turn off the feature in Taffy to avoid wasted computation.

[wlott]

I'm still planning on picking this back up. My fork is close, there are just a couple bizzaro edge cases that differ from chrome.

[nicoburns]

I'm still planning on picking this back up. My fork is close, there are just a couple bizzaro edge cases that differ from chrome.

Very glad to hear it. Getting this right is important! Feel free to share your problem cases if you want :)