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

auto-sizes concrete object size ignoring natural dimensions is not implementable #9448

Closed
zcorpan opened this issue Jun 21, 2023 · 46 comments · Fixed by #9493
Closed

auto-sizes concrete object size ignoring natural dimensions is not implementable #9448

zcorpan opened this issue Jun 21, 2023 · 46 comments · Fixed by #9493

Comments

@zcorpan
Copy link
Member

zcorpan commented Jun 21, 2023

Feedback from @progers in https://chromium-review.googlesource.com/c/chromium/src/+/4520266/comment/d6012d27_30cf4ea2/ about concrete object size ignoring natural dimensions introduced in #8008

If I understand the intended behavior, I think it may not be implementable.

The goal of the spec was to get stable, predictable, non-racy loading behavior of auto-sizes images. Therefore the natural dimensions of the loaded image shouldn't cause a different image to be selected from srcset.

Consider a page like this:

<img loading=lazy sizes=auto srcset="a 2w, b 4w, c 8w, d 16w, ..." style="min-width: 1px">

The layout width before loading anything is 1px, so maybe the browser would select a. When that has loaded, the layout width changes to 2px, which could cause the browser to instead select b. And so on until the largest image has loaded.

The spec checks for 0px concrete object size ignoring natural dimensions and translates auto to 100vw. But since min-width can be used, the image's natural dimensions can still affect the layout size as shown above.

@emilio pointed out that the natural dimensions can affect outer elements, so it's not easy to determine the concrete object size ignoring natural dimensions without doing full layout twice.

So I think we need a different approach here. Maybe checking if there's a specified 'width' (or 'height' + 'aspect-ratio'), and if not use 100vw. Any other ideas?

cc @tcaptan-cr @yoavweiss

@bfgeek
Copy link
Member

bfgeek commented Jun 21, 2023

There are cases even with the spec as-is where you could create a cyclic dependency. A path forward might be to add size containment for images with this behaviour.

[1] E.g. https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11811
Here the width of the outer box is the natural-size of the image.(Say 200px). Then the image 50% resolves against this, and becomes 100px.

We select an image based off this with a natural-size of say 400px, and wash-rinse-repeat.

Ian

@zcorpan
Copy link
Member Author

zcorpan commented Jun 22, 2023

So require contain: size to enable auto-sizes, else resolve auto to 100vw?

@yoavweiss
Copy link
Contributor

Requiring strict dimensions (width or height + aspect-ratio) makes sense to me, and seems like it'd be easier to apply to existing content compared to contain: size. Would strict dimensions not be enough to prevent cyclic dependency?

@zcorpan
Copy link
Member Author

zcorpan commented Jun 22, 2023

Percentage widths as in @bfgeek's example is still problematic. Do you mean disallow percentages with "strict dimensions"?

@zcorpan
Copy link
Member Author

zcorpan commented Jun 22, 2023

As for height + aspect-ratio we'd need to disallow auto aspect-ratio (used by default) since it again makes layout width different between no image loaded and an image loaded.

@progers
Copy link

progers commented Jun 22, 2023

When reading up on containment, I found (this link) which says size containment was used to solve a similar cycle issue for container queries, so there is some precedent with this approach. Is it possible to make auto sizes imply size containment so it doesn't need to be specified?

An alternative is to modify source selection to only go up in srcset options. The spec states that source selection is implementation-defined (selecting an image source), and Chromium currently implements this only-go-up behavior as an optimization to use larger images if they are available in the cache. This would not work for art direction, but that is probably best solved with other APIs (w3c/csswg-drafts#5889 has some info).

@zcorpan
Copy link
Member Author

zcorpan commented Jun 22, 2023

The timing for parsing sizes is not synced with layout, so making the former cause style changes (sometimes! sizes could resolve to something other than auto) also seems like a source of layout instability.

Size containment for images makes the height collapse to 0 if you only specify a width. It seems nice if we can avoid size containment as a requirement.

An alternative is to modify source selection to only go up in srcset options.

I think that still doesn't solve this, since the selection can go up to the largest image as noted in OP. Maybe you need width: 110% or zoom: 1.1 or similar (could be applied on :hover for example) in a fit-content parent.

@bfgeek
Copy link
Member

bfgeek commented Jun 22, 2023

One additional thing is that it's possible to create these cyclic dependencies with "auto" width/heights as well.

E.g. https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11816 (the final width of the images are dependent on each others height).

@eeeps
Copy link
Contributor

eeeps commented Jun 22, 2023

@zcorpan:

Size containment for images makes the height collapse to 0 if you only specify a width. It seems nice if we can avoid size containment as a requirement.

We have inline-size containment, which doesn't. I guess for this we would need width containment, which would be a different, new thing.

@eeeps
Copy link
Contributor

eeeps commented Jun 23, 2023

Basically we need to ensure that image layout size doesn't rely on intrinsic width. We can check if intrinsic width will have any effect beforehand, and if so sub 100vw for auto (same behavior that happens today), or explicitly mandate that intrinsic width will not have any effect using containment.

Checking & falling back to 100vw feels harder, spec-wise. It feels like there are many edge cases (?). I like that it wouldn't break existing layouts that are relying on intrinsic widths, but don't like that when sizes=auto isn't doing what people think it's doing, the problem will often be invisible. Shrinking intrinsically-sized images to 0-width is more noticeable, for better or for worse.

@bfgeek
Copy link
Member

bfgeek commented Jun 23, 2023

We can check if intrinsic width will have any effect beforehand

Detecting if we've used an intrinsic size in layout gets into known hard layout problems very quickly. I suspect containment will be the best path forward.

An option (perhaps not a good one) is to add a UA-style similar to img[sizes=auto] { width: 100% } to provide a better default.

@progers
Copy link

progers commented Jun 25, 2023

Could the concrete object size be adjusted so the natural dimensions don't change as the result of auto sizes selection? For example, if natural dimensions are available when selecting the auto sizes source, adjust the selected image's natural dimensions to match. For cases where layout depends on intrinsic sizes, this would only perform two cycles.

For the original issue of selecting all srcset options:

<img loading=lazy sizes=auto style="min-width: 1px" srcset="3x3.png 2w, 5x5.png 4w, 9x9.png 8w, ...">

This approach would select "3x3.png 2w", resulting in a layout size of 3x3, then select "5x5.png 4w", which would still result in a layout size of 3x3 because the natural dimensions of 5x5.png would be adjusted to 3x3.

For the width: 200%, fit-content parent issue:

<div style="width: fit-content;">
  <img loading=lazy sizes=auto style="min-width: 1px; width: 200%;" srcset="2x2.png 2w, 4x4.png 4w, ...">
</div>

This approach would select "2x2.png 2w", resulting in a layout size of 4x4, then select "4x4.png 4w", which would still result in a layout size of 4x4 because the natural dimensions of 4x4.png would be adjusted to 2x2.

@zcorpan
Copy link
Member Author

zcorpan commented Jun 26, 2023

I wrote

Maybe checking if there's a specified 'width' (or 'height' + 'aspect-ratio'), and if not use 100vw.

This would be easy to write in the spec but maybe less easy to implement, since specified style isn't generally available per @emilio. Is this feasible in Chromium and WebKit?

@eeeps

We have inline-size containment, which doesn't. I guess for this we would need width containment, which would be a different, new thing.

Good point, we could allow contain: inline-size if writing mode is horizontal.

@bfgeek

Detecting if we've used an intrinsic size in layout gets into known hard layout problems very quickly. I suspect containment will be the best path forward.

OK.

An option (perhaps not a good one) is to add a UA-style similar to img[sizes=auto] { width: 100% } to provide a better default.

This doesn't work because sizes can be omitted (equivalent to sizes=auto) and auto keyword can be used anywhere in sizes e.g. sizes="(min-width: 500px) 500px, auto" or sizes="auto, 50vw" (provide fallback for older browsers).

@progers two requests per image which hasn't been acceptable in the past.

@progers
Copy link

progers commented Jun 26, 2023

Don't we still have the cycle issues with size containment approaches that don't constrain all axes?

@emilio
Copy link
Contributor

emilio commented Jun 26, 2023

I think so, yeah... If we require explicit sizes="auto" even for lazy images we can enforce size containment via attribute mapping / spec wording magic without too much problem.

@emilio
Copy link
Contributor

emilio commented Jun 26, 2023

@zcorpan checking if there's a width or height specified is not enough, you need to make sure that the width is absolute right?

@zcorpan
Copy link
Member Author

zcorpan commented Jun 26, 2023

auto and percentage are problematic (unless contain: size). I'm not sure which units for <length> are problematic, if any. cqw might be?

@bfgeek
Copy link
Member

bfgeek commented Jun 26, 2023

auto and percentage are problematic (unless contain: size). I'm not sure which units for are problematic, if any. cqw might be?

Keep in mind the min-width defaults to auto which can affect things even when width: 0px for example:
https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=11820

@eeeps
Copy link
Contributor

eeeps commented Jun 26, 2023

@progers:

Don't we still have the cycle issues with size containment approaches that don't constrain all axes?

Do you have an example where intrinsic height affects the concrete width of an <img> with contain: inline-size and writing-mode: horizontal? (Maybe something with columns/wrapping?)

If so... I wonder how container queries get around this (or how the problems are different).

@bfgeek
Copy link
Member

bfgeek commented Jun 26, 2023

If so... I wonder how container queries get around this (or how the problems are different).

Container queries don't persist "state" outside of layout (unless done by some web-developer script). The primary difference here (similar to content-visibility) is that "state" (what image you've selected). Put another way if you tear everything down, and run style+layout you'll end up with the same result each time.

@eeeps
Copy link
Contributor

eeeps commented Jun 26, 2023

@bfgeek that makes a high-level kind of sense. The takeaway is, authors generally wouldn't be able use auto-sizes on content they don't know the intrinsic aspect ratio of. That will severely limit use (69% of <img>s lack an extrinsic height in CSS and 72% of images lack height and width attributes in HTML).

@progers
Copy link

progers commented Jun 27, 2023

A nice aspect of requiring size containment is that it's conservative and could be relaxed in the future based on developer feedback.

@progers two requests per image which hasn't been acceptable in the past.

With auto sized natural dimensions, the typical behavior would just load one image. In cases where layout depends on the initial request, a second request is needed but it's progressive enhancement. The spec change for this would be to replace "concrete object size ignoring natural dimensions" with "auto sized natural dimensions" which is determined after some image in the srcset loads. Having a stateful srcset does seem strange though.

@zcorpan
Copy link
Member Author

zcorpan commented Jun 27, 2023

@emilio:

If we require explicit sizes="auto" even for lazy images we can enforce size containment via attribute mapping / spec wording magic without too much problem.

This seems feasible, at least if the auto keyword is required to be first in sizes without a media condition.

@eeeps:

Do you have an example where intrinsic height affects the concrete width of an <img> with contain: inline-size and writing-mode: horizontal?

I'm also curious about this! (Should be writing-mode: horizontal-tb btw.)

@progers:

In cases where layout depends on the initial request, a second request is needed but it's progressive enhancement.

I think performance-savvy developers at least will consider it a bug and will have to either not use auto-sizes or be mindful about how they use CSS to avoid double image requests. IMO it's not acceptable.

@bfgeek
Copy link
Member

bfgeek commented Jun 27, 2023

Do you have an example where intrinsic height affects the concrete width of an with contain: inline-size and writing-mode: horizontal?

It's relatively rare - but can happen. ("easiest" is likely with grid-layout - which has two passes, the row sizes from the first pass can affect the column sizes of the 2nd pass).

@progers
Copy link

progers commented Jun 27, 2023

Scrollbars are another example:

<div style="width: 125px; height: 100px; overflow-y: auto;">
  <img loading=lazy
      sizes=auto
      srcset="120x90.png 120w, 125x125.png 125w"
      style="width: 100%; contain: inline-size;">
</div>

@zcorpan
Copy link
Member Author

zcorpan commented Jun 27, 2023

Thanks!

Based on the comments so far, I suggest we do this:

  • disallow omitted sizes (document conformance)
  • presentational hint for sizes="auto" (or sizes="auto, ...") mapping to contain: size
  • in the sizes parsing algorithm: if the computed value of contain is not size, then map auto to 100vw. Otherwise use the concrete object size (even if it's 0).

This means it's not possible to use sizes="auto" without size containment, which is unfortunate, but anything else seems like it can result in unstable resource selection and double downloads in some cases.

@progers
Copy link

progers commented Jun 27, 2023

I am worried that size containment may not work for the common usecase where the image's height depends on the image. Is there data I could gather to determine if this is a requirement? For example, would it be useful to categorize usecases using lazysizes in httparchive data?

I looked at example pages using srcset's w descriptor (link) and a lot of these could not be implemented with size containment. https://arabic.cnn.com is one of the first examples and the srcset images shrink in height as the browser window is dynamically sized smaller. I also looked into lazysizes.js and found it has double-downloading behavior for the scrollbar example above, and seems to prevent infinite loops by not aggressively tracking layout changes.

@eeeps
Copy link
Contributor

eeeps commented Jun 28, 2023

@progers Shrinking in height as the browser window is dynamically resized is not a problem, as long as both dimensions are extrinsic. In this case, they are. We have one dimension (width: 100%) and an aspect ratio (computed aspect-ratio: auto 780 / 439 -- those numbers come from the height and width attributes on <img>). So when I add img { contain: size; } to the page, nothing changes.

I've been thinking about the clearest way to teach this proposal and "if you're using auto-sizes, use width and height attributes on img" seems very clear. I think that covers almost all cases (even the extremely common one where max-width: 100% is used instead of width: 100% to achieve flexible image layout size), and is simple enough to implement, as long as you know the aspect ratio of the embedded resource.

I do however worry that there are many contexts in which HTML is being authored by a person or system that doesn't know that aspect ratio. Templates, user-generated content...

I'll spend some time tomorrow querying the HTTP Archive to see how many images that use srcset and w descriptors rely on intrinsic height.

@bfgeek
Copy link
Member

bfgeek commented Jun 28, 2023

One thing that could be considered is setting the aspect-ratio property as a presentational hint after the first image has been selected - it'd trigger another layout - but might be ok? Unclear.

@zcorpan
Copy link
Member Author

zcorpan commented Jun 28, 2023

Fixing #3981 might help here, in that images with a different width than what was declared in srcset will still render at the width specified by sizes. So for sizes=auto, the width shouldn't change after loading the image. We basically ignore the image's natural width, but we can still apply the natural ratio from the image. No need for size containment??

@eeeps
Copy link
Contributor

eeeps commented Jun 28, 2023

So I did some querying, and tied myself into quite a few knots. As discussed, determining whether a given <img>’s layout size is dependent on its intrinsic dimensions using only its DOM and CSSOM objects is hard, and I now see that my HTTP Almanac analysis was pretty naive. Anyways, here's what I've got:

Here are my takeaways:

  • ~80% of image elements with srcset + w descriptors have both height and width HTML attributes.
  • Almost 90% of image elements with srcset + w descriptors have either a non-auto height in CSS or both height and width attributes in HTML. This is accounted for in the "there was some kind of extrinsic height, probably?" column, although as discussed in this thread, there are many edge cases.
  • Most image elements with srcset + w descriptors are ultimately laid out in a way that depends on their intrinsic height (~70% end up with default computed height, max-height, and min-height values -- meaning that many of those height HTML attributes are reset to height: auto in CSS), but because of the prevalence of HTML height and width attributes, the UA stylesheet aspect-ratio reserves space for them pre-load. If we applycontain: size, this mechanism would be what ultimately determines most of these <img>'s heights.
  • Relatively very few of these images seem to be left completely to their intrinsic widths. It would still be nice not to affect heights if we don't have to.

(also I hope @zcorpan’s right about #3981 and all of this is moot!)

@zcorpan
Copy link
Member Author

zcorpan commented Jun 30, 2023

Checking the cases in this thread assuming we fix #3981:

@progers
Copy link

progers commented Jun 30, 2023

@zcorpan, could you expand on what you are thinking a little bit? I think you are saying that, with #3981 fixed, the width won't change with auto sizes. Would auto sizes need to require an aspect ratio (possibly via width+height attributes) to ensure that the height also does not change?

@zcorpan
Copy link
Member Author

zcorpan commented Jun 30, 2023

In my mind, we would set the natural width of the element to whatever the sizes attribute resolves to, and then ignore the natural width of the loaded image. For sizes=auto, for the first layout the image shouldn't be loaded yet, so you can get the layout width and set the natural width to the layout width after parsing sizes. When the image loads, the width stays the same.

For the height, I thought we could maybe honor the image's aspect ratio. If it's still an issue (e.g. for grid layout) to use the image's height or aspect ratio, we can require width/height attributes, but then we're back to similar behavior as size containment and developers need to know the aspect ratio beforehand. Also width/height attributes is not a guarantee that height does not change, since CSS can set width/height/aspect-ratio to something else.

It's a bit hard to reason about this without being able to test. 🙂

@eeeps
Copy link
Contributor

eeeps commented Jun 30, 2023

Trying to wrap my head around what's acceptable/not acceptable here.

In the scrollbar example with the "sizes sets the intrinsic width" proposal, we are running srcset selection twice, but I guess it's not a problem because (as long as we only load larger resources):

  1. srcset selection is guaranteed to only run twice, and not again and again &c
  2. srcset selection is guaranteed not to pick a different resource on the second run

Those two are related... but I guess my question is, what exactly are we trying to prevent? Loading multiple resources AND running srcset selection an unbounded number of times both seem like blockers to me, but I want to confirm that understanding (and confirm that running srcset selection twice and possibly triggering one re-layout after load, is acceptable).

@eeeps
Copy link
Contributor

eeeps commented Jun 30, 2023

If the scrollbar example is ok in this proposal, I guess the question becomes, can intrinsic height increasing ever make an <img>'s clientWidth LARGER?

@zcorpan
Copy link
Member Author

zcorpan commented Jul 5, 2023

IMO the requirements are:

  • it should be possible to use any CSS layout/sizing features
  • an img should only load one resource on page load

Nice to have:

  • unknown aspect ratio: allow specifying only the width of the image, take the height (or aspect ratio) from the image
  • easy adoption: it should be possible to change existing images to sizes=auto without needing other changes
  • minimal magic: the behavior should be polyfillable and explainable/understandable in terms of existing features

It seems we can't get all of the nice to haves. If we remove "unknown aspect ratio" and say that authors need to specify correct aspect ratio with width and height attributes (or with CSS), we can get most of the others.

Given @eeeps research in httparchive, the vast majority of images already have a specified aspect ratio, so easy adoption should be possible in those cases, even if we use size containment.

To avoid duplicate loads and avoid magic, I think using size containment is best. In which case, #3981 is not a blocker, and we can do #9448 (comment)

@emilio
Copy link
Contributor

emilio commented Jul 5, 2023

  • in the sizes parsing algorithm: if the computed value of contain is not size, then map auto to 100vw. Otherwise use the concrete object size (even if it's 0).

How does that work? Does sizes parsing update style sync? That seems wrong. Or is it using an existing style (whatever we were laid out with?).

I think we should probably enforce that sizes="auto" forces (not only maps to) contain: size. I don't think we have precedents for !important attribute mappings, but they should be implementable. Otherwise a magic adjustment seems not-hard-to-do either.

@eeeps
Copy link
Contributor

eeeps commented Jul 5, 2023

Another research note:

I was worried that even though ~80% of <img>s with srcset+w have height and width HTML attributes, not all of those attributes would be accurate representations of the aspect ratio of the eventually-loaded resource. This worry was unfounded; on mobile (where accuracy is worse), 75% are as right as they can be, and 98% are accurate to within a couple of percentage points.

@zcorpan
Copy link
Member Author

zcorpan commented Jul 6, 2023

  • in the sizes parsing algorithm: if the computed value of contain is not size, then map auto to 100vw. Otherwise use the concrete object size (even if it's 0).

How does that work? Does sizes parsing update style sync? That seems wrong. Or is it using an existing style (whatever we were laid out with?).

The latter. Should we have a note in the spec?

I think we should probably enforce that sizes="auto" forces (not only maps to) contain: size. I don't think we have precedents for !important attribute mappings, but they should be implementable. Otherwise a magic adjustment seems not-hard-to-do either.

Hmm interesting, yeah that would be an alternative and then we don't need to check computed value.

Is it helpful to have stricter syntax constraints around auto so that we can use a selector in the UA stylesheet like:

img:is([sizes="auto" i], [sizes^="auto," i]) { contain: size !important; }

edit: should be :is()

@zcorpan
Copy link
Member Author

zcorpan commented Jul 6, 2023

@eeeps thank you!

@johannesodland
Copy link

Is it helpful to have stricter syntax constraints around auto so that we can use a selector in the UA stylesheet like:

img:has([sizes="auto" i], [sizes^="auto," i]) { contain: size !important; }

Remember that sizes can be defined on any of the <source> elements in the <picture> element.
I don't think the selected source is exposed to CSS selectors, so it might be hard to write a UA rule?

<picture>
  <source srcset="..." sizes=2400px>
  <img sizes=auto loading=lazy>
</picture>

@zcorpan
Copy link
Member Author

zcorpan commented Jul 6, 2023

@johannesodland indeed! Good call.

Ideally the applied style rules don't change from initial layout. Resource selection for lazy images happens later, so having that affect UA style isn't good.

We can require sizes=auto on the img for auto-sizes to work on source (and then sizes can be optional on source).

zcorpan added a commit that referenced this issue Jul 6, 2023
The 'concrete object size ignoring natural dimensions' concept was not implementable. This change makes `sizes=auto` required on `img` to use auto-sizes (still optional on `source`)
and forces size containment so that the image size is the same before and after an image has loaded, to prevent double downloads.

Fixes #9448.
@zcorpan
Copy link
Member Author

zcorpan commented Jul 6, 2023

PR: #9493

@eeeps
Copy link
Contributor

eeeps commented Jul 6, 2023

My own feelings and a tiny, informal poll lead me to believe that disappearing images without width + height will be worrying and confusing to authors who apply sizes=auto without understanding the contain: size mechanism, or the reasoning behind it.

One thing we might be able to do to help here is add a contain-intrinsic-size to the UA stylesheet rule set. I guess this is equivalent to changing the default object size for these elements. I would suggest either 300px 150px to be consistent with <video> or 200px 200px to capture the most common image aspect ratio (1:1) at the (rounded) most common pixel count.

@zcorpan
Copy link
Member Author

zcorpan commented Jul 7, 2023

@eeeps thanks, yeah agreed it's a bit surprising. Adding contain-intrinsic-size seems ok, what do others think? 300x150 seems better to match the default size of most other elements (canvas, video, iframe...). That it's not most common is ok, we want authors to set the width/height.

zcorpan added a commit that referenced this issue Sep 5, 2023
The 'concrete object size ignoring natural dimensions' concept was not implementable. This change makes `sizes=auto` required on `img` to use auto-sizes (still optional on `source`)
and forces size containment so that the image size is the same before and after an image has loaded, to prevent double downloads.

Fixes #9448.
zcorpan added a commit that referenced this issue Nov 30, 2023
The 'concrete object size ignoring natural dimensions' concept was not implementable. This change makes `sizes=auto` required on `img` to use auto-sizes (still optional on `source`)
and forces size containment so that the image size is the same before and after an image has loaded, to prevent double downloads.

Fixes #9448, fixes #9648, and fixes #9649.
Lruihao added a commit to hugo-fixit/FixIt that referenced this issue Jan 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

7 participants