Closed
Bug 1200611
Opened 9 years ago
Closed 8 years ago
object-fit fails on layerized elements (e.g. during opacity transition)
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: polarathene-signup, Assigned: mstange)
References
Details
(Keywords: css3)
Attachments
(4 files)
User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/44.0.2403.157 Safari/537.36 Steps to reproduce: Used the 'object-fit: cover' css property, combined with css transition (animate.css). Tested on Windows 8.1 x64 FF39/40 and FFDev42. Actual results: Tried out the 'object-fit: cover' css property on a small project. When testing on Firefox noticed the style was being disabled temporarily during transitions. You can find a CodePen showing this behaviour: http://codepen.io/polarathene/pen/MwNazK I also came across a StackOverflow user who experienced the same issue and posted a js-fiddle here: http://stackoverflow.com/questions/28939689/firefox-css3-object-fit-cover-strange-behaviour-during-transition Expected results: The image should keep respecting 'object-fit: cover', not readjusting during a transition.
Reporter | ||
Updated•9 years ago
|
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=084441e904d1&tochange=b4fbeba78a7d Daniel Holbert — Bug 1099450: Turn on pref "layout.css.object-fit-and-position.enabled" (enabling "object-fit" & "object-position" properties). r=dbaron
This is from http://codepen.io/polarathene/pen/MwNazK, with the codepen HTML mess cleaned up, and as many libraries as I could removed, and the remaining one (animate.min.css) inlined. It's still too complicated to analyze usefully.
Flags: needinfo?(dbaron)
(And please not that in general, I strongly prefer attached testcases to those at codepen and similar sites, both since those sites include a bunch of additional stuff, and since attached testcases are actually more sure of being preserved and not changed.)
Reporter | ||
Comment 4•9 years ago
|
||
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #2) > Created attachment 8655510 [details] > reporter's testcase > > This is from http://codepen.io/polarathene/pen/MwNazK, with the codepen HTML > mess cleaned up, and as many libraries as I could removed, and the remaining > one (animate.min.css) inlined. > > It's still too complicated to analyze usefully. I understand my CodePen being to complicated to isolate the problem. I am fairly certain it can be simplified down to using `object-fit: cover` on an image with width/height adjusted to see a visual difference from the source image, then apply a css transition. The js-fiddle is far more simpler at the provided StackOverflow link, direct link here: http://jsfiddle.net/sytwrd9L/1/ I created the report at 3am, will look into attaching an isolated test case with bare minimum for you.
Reporter | ||
Comment 5•9 years ago
|
||
(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #3) > (And please not that in general, I strongly prefer attached testcases to > those at codepen and similar sites, both since those sites include a bunch > of additional stuff, and since attached testcases are actually more sure of > being preserved and not changed.) Apologies, though I also provided the Github Repo without all the minified code, I can still provide that if useful. I've taken the linked JS-Fiddle, removed JQuery and added 2 more tests. It appears to be related to transitions on the opacity property, I've also verified that this is incorrect behaviour is happening on Chrome 44 as well. Why my CodePen is unaffected in Chrome despite similar transition logic to the JS-Fiddle is unknown. Chrome has another issue with transform3d with `object-fit: cover` this is the 3rd test provided and FireFox handles it fine. JS-Fiddle: http://jsfiddle.net/sytwrd9L/6/ Attached(bug_object-fit_cover.html) is an html file containing the JS/CSS similar to the linked example, though with an additional delay before removing the 'active' class, otherwise the transition does not occur.
Reporter | ||
Comment 6•9 years ago
|
||
JS-Fiddle equivalent: http://jsfiddle.net/sytwrd9L/6/
Comment 7•9 years ago
|
||
Thanks - I think that testcase (and its dependence on opacity changing, in particular) means we aren't honoring 'object-fit' for layerized elements (one of our optimized graphics codepaths). I can add this to my list of bugs to look into when I'm back from vacation, but someone else may get to it before me.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(dholbert)
Summary: object-fit: cover fails during transition → object-fit: cover fails on layerized elements (e.g. during opacity transition)
Ah, makes sense. I'd guess that we're creating an ImageLayer in that case.
Comment 9•9 years ago
|
||
(Also: despite comment 1, this isn't actually a regression from bug 1099450. That's just where support for "object-fit" was turned on, so it's when the attached testcase went from not-honoring-object-fit-at-all to only honoring it some of the time. Updating dependency to make this depend on bug 624647 (the main 'object-fit' bug), rather than blocking the pref-flip bug.)
Comment 10•9 years ago
|
||
Flags: needinfo?(dholbert)
Comment 11•9 years ago
|
||
Chrome actually hits what looks like the same bug on the 2nd and 3rd attached testcases here. (from comment 6 and comment 10)
Comment 12•9 years ago
|
||
I suspect the code that needs fixing here is the "destRect" we're choosing in nsDisplayImage::ConfigureLayer: http://mxr.mozilla.org/mozilla-central/source/layout/generic/nsImageFrame.cpp?rev=ae625cd90875#1690 The second half of that function seems to assume that the image will be stretched to fill nsDisplayImage::GetDestRect, but that's not true if 'object-fit' is set. We do somewhat-similar calculations in nsDisplayImage::GetLayerState -- that may need fixing as well.
Comment 13•9 years ago
|
||
I filed https://code.google.com/p/chromium/issues/detail?id=531739 on the Chrome version of this bug. (I'm setting needinfo=me to circle back to this after I've finished processing vacation-email backlog and worked through high-priority items.)
Updated•9 years ago
|
Flags: needinfo?(dholbert)
Comment 15•9 years ago
|
||
The bread in this example will fall back to the specified dimensions: https://d157rqmxrxj6ey.cloudfront.net/natewiebe13/14723/
Updated•8 years ago
|
Flags: needinfo?(dholbert)
Summary: object-fit: cover fails on layerized elements (e.g. during opacity transition) → object-fit fails on layerized elements (e.g. during opacity transition)
Assignee | ||
Comment 17•8 years ago
|
||
Bug 1200611 - Size ImageLayers correctly for <img>s using object-fit. r?dholbert This changes nsDisplayImage::GetDestRect and nsImageFrame::PredictedDestRect to return the true dest rect of the image, without intersecting it with the image content box. The only caller of these functions that actually requires a rectangle that's within the image's content bounds is nsDisplayImage::GetOpaqueRegion, so I'm changing that to intersect with the display item's bounds. I'm pretty sure nsImageFrame::MaybeDecodeForPredictedSize() also prefers the unclipped dest rect, because it cares about the scale at which the image is painted, and not about which parts of the image are painted.
Attachment #8695287 -
Flags: review?(dholbert)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → mstange
Status: NEW → ASSIGNED
Comment 18•8 years ago
|
||
Comment on attachment 8695287 [details] MozReview Request: Bug 1200611 - Size ImageLayers correctly for <img>s using object-fit. r?dholbert https://reviewboard.mozilla.org/r/27049/#review24449 Thank you for taking this! I was hoping to get to fixing this today or tomorrow, but now I don't need to. \o/ r=me ::: layout/reftests/bugs/1200611-1-ref.html:3 (Diff revision 1) > +<title>Make sure the ImageLayer is sized correctly when using object-fit</title> Nit: Consider putting "Reference: " in the reference case's <title>, so it's easier to tell which file is which when viewing them in two tabs side-by-side (because often, the URL doesn't fit in the URLbar, so you can't easily see the filename).
Attachment #8695287 -
Flags: review?(dholbert) → review+
Assignee | ||
Comment 19•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=feba9d06da3c
Assignee | ||
Comment 20•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/ca6c81a2c6c2ac3183f31afb437fc88534022f44 Bug 1200611 - Size ImageLayers correctly for <img>s using object-fit. r=dholbert
Comment 21•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/ca6c81a2c6c2
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
Comment 22•8 years ago
|
||
Markus, would you be up for having this backported to aurora? (which does happen to be nearly-beta) Given that we've had 2 dupes of this filed, it looks like people are hitting this in the real world, and it'd be nice to fix it 6 weeks sooner.
Flags: needinfo?(mstange)
Comment 24•8 years ago
|
||
NI flag removed because FF45 is going to be release in a few days.
Flags: needinfo?(mstange)
You need to log in
before you can comment on or make changes to this bug.
Description
•