html icon indicating copy to clipboard operation
html copied to clipboard

img sizes="auto" skewing images using object-fit: contain

Open joemcgill opened this issue 1 year ago • 21 comments

What is the issue with the HTML Standard?

While testing the implementation for auto sizes in the latest Chrome betas (v121) I noticed that images using object-fit: contain were getting skewed. I've created a reduced test case to demonstrate the behavior I'm seeing.

I noticed in https://github.com/whatwg/html/issues/4654#issuecomment-495266624 that @aFarkas mentioned some known quirks with calculating height with object-fit, so perhaps this is expected behavior? Wondering if @zcorpan has any advice here.

joemcgill avatar Jan 19 '24 19:01 joemcgill

As I understand it, the problem here is that the object-fit spec calls for contain and cover to look at the element’s natural aspect ratio.

Authors should be able to set this intrinsic size with contain-intrinsic-size but there appears to be a Chrome bug preventing this from working, currently.

Once the Chrome bug is fixed, this should be work-around-able. However: would it be web compatible to have the <img width height> attributes set the natural size of the <img> in the same way that they do, for <video>, which does not suffer from this problem? https://codepen.io/eeeps/pen/bGZWZBq

eeeps avatar Jan 19 '24 20:01 eeeps

Actually this does appear to be fixable with contain-intrinsic-size? I swear it wasn't an hour ago, but perhaps I just made a typo? https://codepen.io/eeeps/pen/qBvmvwE

The codepen I linked above (https://codepen.io/eeeps/pen/bGZWZBq) is also working "properly" for me now. An hour ago the first dog face (in the <img>) was squished to 2:1 within its 1:1.5 box, I promise! It had large green letterbox bars; I should have taken a screenshot.

eeeps avatar Jan 19 '24 21:01 eeeps

Hmmm, I wonder if we should have width and height attributes, if both are set (on img or the selected source), be a presentational hint to contain-intrinsic-size? Or should object-fit work differently?

@LeaVerou @tabatkins @fantasai any advice, as editors of css-images?

cc @whatwg/css @progers @tcaptan-cr

zcorpan avatar Jan 22 '24 20:01 zcorpan

@eeeps with https://codepen.io/eeeps/pen/qBvmvwE I see stretched images in Chrome Canary 121 (I got an error with updating it seems), but resizing the window made the image switch to correct aspect ratio.

I downloaded a new Chrome Canary, which was version 122 (still an error with updating), and now the image is not stretched. So maybe it was a bug that was fixed after 121?

zcorpan avatar Jan 22 '24 20:01 zcorpan

Hmmm, I wonder if we should have width and height attributes, if both are set (on img or the selected source), be a presentational hint to contain-intrinsic-size?

@progers @tcaptan-cr I see auto-sizes was unshipped in Chromium because of this issue (based on the referenced bugs), but the sites in question apparently don't have width/height attributes.

We could make auto-sizes do nothing if the img doesn't have both width and height attributes, in addition to the preshint fix. I think that would address the web compat issue.

zcorpan avatar Jan 29 '24 20:01 zcorpan

In my original example test case, it does look like the UA stylesheet was being applied and causing this issue, even though in my case the image markup does include width and height attributes. Adding the following CSS to the stylesheet fixes the issue for me:

img:is([sizes="auto" i], [sizes^="auto," i]) {
    contain-intrinsic-size: auto none;
}

It makes me wonder if the UA stylesheet is being applied too broadly here.

joemcgill avatar Jan 30 '24 00:01 joemcgill

@joemcgill

Adding the following CSS to the stylesheet fixes the issue for me:

If you also remove display: flex from the parent, the images disappear. That was the original problem that contain-intrinsic-size in the UA stylesheet was intended to solve. See https://github.com/whatwg/html/issues/9448#issuecomment-1624306851

zcorpan avatar Jan 30 '24 22:01 zcorpan

Thanks for the context, @zcorpan. I guess the thing that is unclear to me is that even in @eeeps comment that you linked to, he specifically references images without height and width, whereas the UA stylesheet applies regardless of whether the dimension attributes are present.

joemcgill avatar Jan 31 '24 21:01 joemcgill

The problem no longer occurs in Chrome 122.0.6261.57

fvwanja avatar Feb 22 '24 09:02 fvwanja

The problem no longer occurs in Chrome 122.0.6261.57

Yes, because we switched the feature back to experimental mode due to several bug reports about this issue.

tcaptan-cr avatar Feb 22 '24 17:02 tcaptan-cr

@progers @tcaptan-cr I see auto-sizes was unshipped in Chromium because of this issue (based on the referenced bugs), but the sites in question apparently don't have width/height attributes.

Yes, the ones reported didn't have width or height attributes.

We could make auto-sizes do nothing if the img doesn't have both width and height attributes, in addition to the preshint fix. I think that would address the web compat issue.

I can try it out. Do we need both width and height attributes defined or would one of them be sufficient?

tcaptan-cr avatar Feb 22 '24 18:02 tcaptan-cr

Both are needed to give a correct aspect ratio.

zcorpan avatar Feb 27 '24 15:02 zcorpan

It is not clear to me that object-fit should be using the aspect ratio given by contain-intrinsic-size.

In Images Level 3, object-fit is specified to preserve the natural aspect ratio, but contain-intrinsic-size doesn't actually change the natural dimensions; instead it tells the rest of layout to proceed as if those were the natural dimensions. But Image.naturalWidth/Height are unchanged.

In Images Level 4, object-fit does not specify how an aspect ratio should be determined, saying in a note that "How to then draw into that [concrete object size] is up to the image format".

As all browsers currently do the same thing (contain-intrinsic-size affects object-fit), we should probably better-specify that somewhere, require width and height in cases where sizes=auto, and change the UA stylesheet to something like:

img:is([sizes="auto" i], [sizes^="auto," i]) {
  contain: size !important;
  contain-intrinsic-size: attr(width px, 300px) attr(height px, 150px);
}

But I would like to hear from the CSS Images folks (@LeaVerou @tabatkins @fantasai) before we 100% commit to contain-intrinsic-size affecting object-fit behavior.

eeeps avatar Mar 20 '24 21:03 eeeps

I think it's a mistake for contain-intrinsic-size to impact object-fit, and we should clarify the spec to remove that behavior if at all possible.

mirisuzanne avatar Mar 20 '24 21:03 mirisuzanne

Correction! I said,

As all browsers currently do the same thing (contain-intrinsic-size affects object-fit)...

But I was wrong, Firefox doesn't (and also doesn't use the actual natural aspect ratio, instead object-fit just doesn't work with contain: size) https://codepen.io/eeeps/pen/dyLNEdN

eeeps avatar Mar 20 '24 22:03 eeeps

@eeeps can you file a new issue in w3c/csswg-drafts?

zcorpan avatar Mar 21 '24 09:03 zcorpan

But I was wrong, Firefox doesn't (and also doesn't use the actual natural aspect ratio, instead object-fit just doesn't work with contain: size) https://codepen.io/eeeps/pen/dyLNEdN

I filed https://bugzilla.mozilla.org/show_bug.cgi?id=1886745 , thx!

zcorpan avatar Mar 21 '24 13:03 zcorpan

If https://github.com/w3c/csswg-drafts/issues/10116 is fixed, it seems to me that auto-sizes can stay as currently specified.

zcorpan avatar Mar 28 '24 10:03 zcorpan

I agree and will re-land it in Chrome once it's fixed.

tcaptan-cr avatar Mar 28 '24 14:03 tcaptan-cr

Auto sizes was re-enabled in Chromium in https://issues.chromium.org/issues/40862170 and it looks like the image skewing issue is not longer present in my original test case.

joemcgill avatar Apr 18 '24 14:04 joemcgill

Thanks for confirming!

tcaptan-cr avatar Apr 18 '24 17:04 tcaptan-cr