-
Notifications
You must be signed in to change notification settings - Fork 689
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-images-4] Should contain-intrinsic-size
affect object-fit
?
#10116
Comments
The reason for |
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. |
Some previous discussion in #6257 |
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. |
Is this a consequence of
contain-intrinsic-size definition says:
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. |
@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 |
Open devtools and set
Size containment, which may be affected by
See the resolution in #7519 (comment). But as I argued in #6257 (comment), this is just for sizing the |
Got it working, thanks all! And yes, it makes sense that |
[css-images] [css-contain] `object-fit` should use the "real" natural aspect ratio, and `contain: size` should not ruin this w3c/csswg-drafts#10116 w3c/csswg-drafts#6257
First stab at a Web Platform Test https://github.com/eeeps/wpt/tree/object-fit-contain-size |
I think the desired resolution here would be what @bfgeek proposed in #6257 (comment) - 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? :) |
The CSS Working Group just discussed
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 |
[css-images] [css-contain] `object-fit` should use the "real" natural aspect ratio, and `contain: size` should not ruin this w3c/csswg-drafts#10116 w3c/csswg-drafts#6257
[css-images] [css-contain] `object-fit` should use the "real" natural aspect ratio, and `contain: size` should not ruin this w3c/csswg-drafts#10116 w3c/csswg-drafts#6257
* Test interaction between object-fit and contain:size [css-images] [css-contain] `object-fit` should use the "real" natural aspect ratio, and `contain: size` should not ruin this w3c/csswg-drafts#10116 w3c/csswg-drafts#6257 * Change image-rendering: crisp-edges to pixelated Chromium doesn’t support `crisp-edges` * Remove whitespace to make linter happy
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore `LayoutReplaced::IntrinsicSize`, used by `LayoutReplaced::ComputeObjectFitAndPositionRect`, should return the actual intrinsic size of a replaced element. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5442533 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Traian Captan <tcaptan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1287761}
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5442533 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Traian Captan <tcaptan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1287761}
The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5442533 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Traian Captan <tcaptan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1287761}
…d `contain: size`, a=testonly Automatic update from web-platform-tests Test interaction between `object-fit` and `contain: size` (#45563) * Test interaction between object-fit and contain:size [css-images] [css-contain] `object-fit` should use the "real" natural aspect ratio, and `contain: size` should not ruin this w3c/csswg-drafts#10116 w3c/csswg-drafts#6257 * Change image-rendering: crisp-edges to pixelated Chromium doesn’t support `crisp-edges` * Remove whitespace to make linter happy -- wpt-commits: ae05e1ac2530c57f3e6205c50d6176b4875098ff wpt-pr: 45563
…d `contain: size`, a=testonly Automatic update from web-platform-tests Test interaction between `object-fit` and `contain: size` (#45563) * Test interaction between object-fit and contain:size [css-images] [css-contain] `object-fit` should use the "real" natural aspect ratio, and `contain: size` should not ruin this w3c/csswg-drafts#10116 w3c/csswg-drafts#6257 * Change image-rendering: crisp-edges to pixelated Chromium doesn’t support `crisp-edges` * Remove whitespace to make linter happy -- wpt-commits: ae05e1ac2530c57f3e6205c50d6176b4875098ff wpt-pr: 45563
… aspect ratio, a=testonly Automatic update from web-platform-tests object-fit should use the "real" natural aspect ratio The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5442533 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Traian Captan <tcaptan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1287761} -- wpt-commits: 30a7e26a85a65544133fd257e8af644f785e04f7 wpt-pr: 45723
… aspect ratio, a=testonly Automatic update from web-platform-tests object-fit should use the "real" natural aspect ratio The resolution of issue 10116 [1] "Should contain-intrinsic-size affect object-fit?" was that: "`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)". Therefore, `LayoutReplaced::ComputeObjectFitAndPositionRect` should use the natural size rather than the overridden intrinsic size. After analyzing all callsites of `LayoutReplaced::IntrinsicSize`, it was found that they all should use the natural size rather than the overridden intrinsic size, so this core function was updated. [1] w3c/csswg-drafts#10116 Low-Coverage-Reason: TRIVIAL_CHANGE - no effective change Bug: 332775468 Change-Id: I9cbdcb45a392145586a3ee7b3f06f46ce2301931 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5442533 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Traian Captan <tcaptan@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/main@{#1287761} -- wpt-commits: 30a7e26a85a65544133fd257e8af644f785e04f7 wpt-pr: 45723
Ran into this today in Safari and submitted https://bugs.webkit.org/show_bug.cgi?id=276681 |
A patch was submitted to the WebKit bug today. https://bugs.webkit.org/show_bug.cgi?id=276681#c2 So perhaps soon we will have three implementations, tests, but no spec yet. |
Even though clarifying the spec would be good, it already implies the resolved behavior, as I explained in #6257 (comment) |
https://bugs.webkit.org/show_bug.cgi?id=276681 <rdar://problem/131866042> Reviewed by Antti Koivisto and Tim Nguyen. This change aligns WebKit with w3c/csswg-drafts#10116 resolution: "'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)". * LayoutTests/TestExpectations: * Source/WebCore/rendering/RenderReplaced.cpp: (WebCore::RenderReplaced::paint): Canonical link: https://commits.webkit.org/289287@main
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 thecontain-intrinsic-size
dimensions, when paintingobject-fit
objects intocontain:size
d 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.The text was updated successfully, but these errors were encountered: