w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.38k stars 642 forks source link

[css-images-4] Should `contain-intrinsic-size` affect `object-fit`? #10116

Open eeeps opened 4 months ago

eeeps commented 4 months ago

This question came up during sizes=auto spec/implementation work.

Here's a test page: https://codepen.io/eeeps/pen/mdgWNJo?editors=1101

WebKit thinks that it should, Chromium thinks that it should, and Firefox refuses to do any kind of object-fitting on size-contained elements.

Notably, @mirisuzanne thinks that it shouldn't, because contain-intrinsic-size doesn't actually change the natural dimensions of replaced elements (Image.naturalWidth/naturalHeight are unaffected), but simply tells the layout algorithm to proceed as if those were the natural dimensions.

I agree with her, and think UAs should use the actual natural dimensions that you get from Image.naturalWidth/naturalHeight, and not the contain-intrinsic-size dimensions, when painting object-fit objects into contain:sized elements. I think we need some more spec language somewhere here, although admittedly my understanding of some of the concepts here is provisional at best.

mirisuzanne commented 4 months ago

The reason for object-fit and related properties is to help align and size the image content inside a box that has different dimensions, or a different aspect ratio - because of some other specified styles. When we start applying those box layout dimensions to the image content, we lose the purpose of the feature.

tabatkins commented 4 months ago

Agreed, 'object-fit' 'itself clearly requires working on the actual image dimensions, or else it's worthless. The c-i-s dimensions are just to force layout stability. They only have an effect on the image's dimensions as a side-effect, since images default to stretching into their layout dimensions.

I'd have to poke around to figure out what, if anything, needs fixing in the specs; the language around intrinsic/natural dimensions is a bit wonky and there probably needs to be more "layers" of terminology here.

Loirooriol commented 4 months ago

Some previous discussion in https://github.com/w3c/csswg-drafts/issues/6257

mirisuzanne commented 4 months ago

We could close one of the issues, and add agenda+ to the other? It seems like there's consensus this is broken. And it seems like it should be clarified somewhere.

vmpstr commented 4 months ago

Is this a consequence of contain-intrinsic-size or size containment? Size containment says that

Replaced elements must be treated as having an natural width and height of 0 and no natural aspect ratio.

contain-intrinsic-size definition says:

These properties allow elements with size containment to specify an explicit intrinsic inner size, causing the box to size as if its in-flow content totals to a width and height matching the specified explicit intrinsic inner size (rather than sizing as if it were empty).

I couldn't find the reference for contain-intrinsic-size and natural size interactions.

In particular, in the codepen example (on Chromium), I can't get a different effect if I change contain-intrinsic-size to something like 800px 400px. It always remains a square.

eeeps commented 4 months ago

@vmpstr Try refreshing? This has been flaky for me before. Not sure if that's CodePen's fault, or Chromiums. Anyways here's proof that it does sometimes have an effect: https://o.img.rodeo/c2axgu9ms1l3ojujbjjn.png https://codepen.io/eeeps/pen/abxWVmN?editors=1101

Loirooriol commented 4 months ago

It always remains a square

Open devtools and set display: none, then unset it.

Is this a consequence of contain-intrinsic-size or size containment?

Size containment, which may be affected by contain-intrinsic-size.

I couldn't find the reference for contain-intrinsic-size and natural size interactions.

See the resolution in https://github.com/w3c/csswg-drafts/issues/7519#issuecomment-1202721367.

But as I argued in https://github.com/w3c/csswg-drafts/issues/6257#issuecomment-850709682, this is just for sizing the <img>. Once we know its size, the spec says to lay out normally. So object-fit should work. And since the example uses width and height, contain-intrinsic-size should have no effect at all.

vmpstr commented 4 months ago

Got it working, thanks all!

And yes, it makes sense that contain-intrinsic-size affects the img element size not the interaction with object-fit

eeeps commented 3 months ago

First stab at a Web Platform Test https://github.com/eeeps/wpt/tree/object-fit-contain-size

mirisuzanne commented 3 months ago

I think the desired resolution here would be what @bfgeek proposed in https://github.com/w3c/csswg-drafts/issues/6257#issuecomment-849150205 - though it's not clear to me which spec (if any) needs to be clarified. Maybe we're just confirming this, and adding the WPT to prove it? :)

css-meeting-bot commented 3 months ago

The CSS Working Group just discussed [css-images-4] Should `contain-intrinsic-size` affect `object-fit`?, and agreed to the following:

The full IRC log of that discussion <TabAtkins1> argh, be there one sec
<fantasai> EricP: object-fit uses the natural size, but there are two natural aspect ratios we care about
<fantasai> EricP: there's the actual natural aspect ratio of the object, and the one we use in layout (affected by contain-intrinsic-size)
<fantasai> EricP: wanted to do auto sizes for ? and were using contain-intrinsic-size
<fantasai> EricP: If you don't do that, you will load every resource in srcset, very bad
<fantasai> EricP: When shipped in Chrome Beta, a number of sites were also using sizes=auto and those images became distorted
<fantasai> EricP: We thought maybe set contain-intrinsic-size in UA stylesheet
<fantasai> EricP: But realized maybe we need two concepts, the intrinsic size for layout vs the intrinsic size for drawing
<vmpstr> q+
<miriam> q+
<TabAtkins> +1 to being explicit in both sites
<astearns> ack vmpstr
<fantasai> EricP: object-fit is looking at the actual intrinsic size, contain is for layout only
<fantasai> vmpstr: Is object-fit the only property that should be affected by actual natural size?
<fantasai> miriam: I'm thinking in the reverse way, the containment features should only remove the natural sizing for layout purposes
<schenney> q+
<fantasai> miriam: the answer is, anything not about laying out the box should respect the natural size
<fantasai> +1
<florian> +1 to miriam
<astearns> ack miriam
<schenney> q-
<fantasai> miriam: This also solves some other problems we've had in the past, where no way to remove the aspect ratio of the image and give it a new one
<fantasai> -> https://www.w3.org/TR/css-sizing-4/#aspect-ratio
<fantasai> miriam: hard to have the image contribute some dimensions without contributing its aspect ratio or nothing
<fantasai> miriam: we discussed awhile back, and ian made a proposal similar here, basically that containment should only contain for purposes of layout, nothing else
<astearns> ack fantasai
<dholbert> scribe+ dandclark
<dandclark> fantasai: When we the aspect ratio prop we added the concept of preferred aspect ratio, which is different from natural aspect ratio. Can we hook into that concept? Agree contain shouldn't hook into natural aspect ratio.
<eeeps> q+
<astearns> ack eeeps
<dandclark> eeeps: I do think we need aspect ratio, and width, and height. Ratio alone isn't enough for modes like object fit scale down
<fantasai> astearns: so can we update the proposed resolution to use natural vs preferred aspect ratio terms?
<dandclark> fantasai: Let's specify what we're trying to get here, and Tab and I can deal with the vocab aspects of it. We're trying to say that contain removes natural aspect ratio for purposes of [missed]. Tab and I can figure out what that means in specs.
<fantasai> s/[missed]/sizing/
<fantasai> PROPOSED: `contain` removes the natural aspect ratio / width / height only for the purposes of sizing and layout
<fantasai> dholbert: maybe clarify that it doesn't impact how object-fit works
<dandclark> fantasai: For sizing and layout of the box, and i.e. object fit is not affected
<fantasai> PROPOSED: `contain` removes the natural aspect ratio / width / height only for the purposes of sizing and layout of the box (and object-fit is therefore not affected)
<fantasai> astearns: Sounds like we're in agreement, might need some wordsmithing to put in the specs
<fantasai> astearns: any objections?
<fantasai> RESOLVED: `contain` removes the natural aspect ratio / width / height only for the purposes of sizing and layout of the box (and object-fit is therefore not affected)
<fantasai> astearns: Any other notes for the minutes?
<fantasai> miriam: notes in the linked issues
eeeps commented 1 week ago

Ran into this today in Safari and submitted https://bugs.webkit.org/show_bug.cgi?id=276681