Skip to content
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

[css-shapes] Allow new formatting contexts to be sized based on *bounding* float-area. #1970

Open
bfgeek opened this issue Nov 10, 2017 · 8 comments
Labels
css-shapes-1 Current Work

Comments

@bfgeek
Copy link

bfgeek commented Nov 10, 2017

With css-shapes, floats are positioned based on their margin-box. See example-5 in:
https://drafts.csswg.org/css-shapes-1/#relation-to-box-model-and-float-behavior

They will not try and place themselves based on the "float-area", instead the bounding margin-box.

New formatting contexts however, (overflow: hidden, etc) are a different story. It's not exactly clear what the spec intended here.

Currently today in Blinks implementation we will try and size new formatting contexts based on the "float-area". Our implementation uses a previous frame's data, which is completely broken.

In order to do this "correctly" you would have to iterate through every available inline-size, (stepping probably at each pixel?), perform a layout, and check if the block-size of the new FC will fit in the given area.

We (blink) have added a UseCounter for the broken behaviour, are probably going to move to our default sizing/positioning algorithm for new-FCs in the near future.

Given the "correct" behaviour is expensive (requires potentially N layouts - where N is a large number), it might be worth changing the spec to size & position new-FCs similar to how floats currently behave.

This would leave only line-boxes being sized and positioned based on the "float-area".

@bfgeek bfgeek added the css-shapes-1 Current Work label Nov 10, 2017
@emilio
Copy link
Collaborator

emilio commented Mar 13, 2018

cc @bradwerth

@FremyCompany
Copy link
Contributor

Is this related to #2452?

@bfgeek
Copy link
Author

bfgeek commented Mar 28, 2018

@FremyCompany Related. It's an extension of that issue. Basicly, once you decide on behaviour on #2452 what do you do with shapes?

We've just implemented (and will ship next release) sizing a BFC as if the float had no shape-outside on it. We had use-counters, and they weren't getting hit often enough.

The tentative test I added is here:
https://github.com/w3c/web-platform-tests/blob/master/css/css-shapes/shape-outside/formatting-context/shape-outside-formatting-context.tentative.html

@astearns
Copy link
Member

@bfgeek we decided in #2452 to continue to require the more-difficult-but-better result from CSS 2.1. So the question now is whether to relax this for floats with a shape-outside, or not special-case BFCs for shape-outside.

Looking at the current results for your test above, Blink and Webkit are sizing to the float-area without the shape-outside, and Gecko is using the shape-outside contours. I very much prefer the Gecko behavior.

The main reason we relaxed the behavior for float stacking is that it gets even harder if the floats you're stacking both have shapes that could nestle together like puzzle pieces. I don't think there's a need to add more complexity to float stacking. I do think it would be useful (and consistent) to get BFC sizing to respect the shape-outside contours.

@astearns
Copy link
Member

Looking through the minutes I see it was asserted that there should be no difference between regular floats and floats with shape-outside for BFC avoidance. But asserted is different than resolved, so I'll agenda+ this to get a resolution.

@bfgeek
Copy link
Author

bfgeek commented Oct 30, 2019

The issue here is that the solution to #2452 is far more computationally complex here. Typically there aren't that many floats on the page which limits the computational complexity of the previous resolution.

Implementations today current "cheat" in different ways, leading to different implementation bugs and iterop issues:

Firefox fails this relatively simple testcase:
https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=7328
Here I believe Firefox assumes that an element will always shrink in block-size as its inline-size grows, which isn't always the case as show by the testcase. To do this properly you can't assume that a height of a given element will larger or smaller given a different inline-size.

I haven't tested Safari recently, but if it had the same issues with the previous Blink implementation, it probably uses hysteresis (using data from a previous layout, for the next layout) which got things mostly correct, but was unstable.

Essentially to get this perfect you need to perform layout at almost every available inline-size.
We (blink) prioritize users and want to avoid a computational explosion like this :).

Further - we haven't seen demand from web developers for this type of usecase. We have usecounters in place that which show that css-shapes are very popular! But nobody was depending on sizing a BFC in this way. Further since we removed this feature, we haven't received any bug reports etc.

@aethanyc
Copy link

aethanyc commented Nov 2, 2019

Firefox fails this relatively simple testcase:
https://software.hixie.ch/utilities/js/live-dom-viewer/?saved=7328
Here I believe Firefox assumes that an element will always shrink in block-size as its inline-size grows, which isn't always the case as show by the testcase. To do this properly you can't assume that a height of a given element will larger or smaller given a different inline-size.

This is an interesting test. The flex container's block-size depends on its inline-size because the <img> uses percentage width. So if flex's inline-size shrink, both <img>'s inline-size and block-size become smaller, resulting in a smaller block-size for flex.

What Firefox does to layout this test case is: before the flex container's block-size is known, it uses the full 200px inline space provided by test to calculate flex's dimension. Once flex's block-size is known, it realizes it will overlap the float, so it will find the first available inline coordinate which has large enough block space to fit flex, and calculate flex's dimension again. But this time, as the inline-space shrink, both flex's inline-size and block-size shrink as well. The block space at the given inline coordinate is now too big, but Firefox is happy about it because flex already fits.

I think if either of the flex container or <img> has a fixed dimension like width: 100px, Firefox can layout this test without any issue.

@bfgeek
Copy link
Author

bfgeek commented Nov 2, 2019

@aethanyc Yes - this is the fundamental difference between performing layout upon line-boxes, vs. new formatting contexts.

With line-boxes you can safely assume that given a smaller available inline-size, the block-size of the resulting line-box will stay the same, or decrease.

With a new formatting context you can't assume the final block-size given some available inline-size - you need to perform a full layout. This is only going to increase in complexity as engines get things like aspect-ratio, min() / max() functions, (and potentially some variants of container queries). (E.g. this relationship between inline-size and block-size is some unknown piecewise function).

But back to my original point :) - performance cliffs in the platform like this are bad for users. We've been shipping this behaviour for quite a while now, and determined that there wasn't any compat issues or demand for this feature.

This decision made with #2452 was subtly different, we had compat issues to deal with which is why we went down that path.

@astearns astearns removed the Agenda+ label Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-shapes-1 Current Work
Projects
None yet
Development

No branches or pull requests

5 participants