anselmh / object-fit

Polyfill (mostly IE) for CSS object-fit property to fill-in/fit-in images into containers.
MIT License
996 stars 93 forks source link

slow #13

Closed yukulele closed 9 years ago

yukulele commented 9 years ago

obect-fit.js is very slow on firefox

I think the probleme is getMatchedCSSRules polyfill


Bountysource

anselmh commented 9 years ago

Yeah it definitely has some performance issues and indeed this is the polyfill. Unfortunately there’s no alternative to it yet (or I just didn’t found one?). If you know a way how to fix it, please let me know. I’ll leave this open for suggestions and to remind myself to search for a better solution.

toobulkeh commented 9 years ago

so by very slow I guess you mean this is the same 10 second delay that we're having... :facepunch:

anselmh commented 9 years ago

I at least found some information what causes the trouble and makes Firefox slow and unresponsive when dealing with multiple images here. It’s the selector matching and generally getting the computedStyles from the elements and re-applying them.

If anyone has an idea how to optimize this, please let me know or send a PR. Happy to review and merge it.

O-Zone commented 9 years ago

Did anyone figure out whether this actually IS the same as https://github.com/anselmh/object-fit/issues/8, and are anyone working on a fix right now? I have a Bootstrap 3 site running the polyfill, and unfortunately I can confirm that this IS a problem in IE and FF.

anselmh commented 9 years ago

It is the same. I’m sorry, I couldn’t find enough time to make the alternative implementation of the CSS parser. If anyone is willed to help, reach out to me and I’ll let you know what’s the plan.

toobulkeh commented 9 years ago

@anselmh describe the plan here, I might have time, just not sure if I have the knowledge.

anselmh commented 9 years ago

So the plan is to replace the CSS matcher / parser completely. The currently used script causes the trouble seen in bad performance on Firefox and IE. Replacing it with a better, proper solution might work and solve these problems.

I already created an issue describing what needs to be done here: https://github.com/anselmh/object-fit/issues/19

thany commented 9 years ago

I've got a workaround. It may not work for everyone, but for me, on my computer, on FF 35, replacing this:

var replacedElementStyles = objectFit.getComputedStyle(replacedElement);
var replacedElementDefaultStyles = objectFit.getDefaultComputedStyle(replacedElement);

With this:

var replacedElementStyles = { top: 0, right: 0, bottom: 0, left: 0, width: 0, height: 0, overflow: 0 },
    replacedElementDefaultStyles = replacedElementStyles;

Makes it a billion times faster.

anselmh commented 9 years ago

Yes it does work in some cases. The main issue about this is that it doesn’t inherit other styles that were used on these elements before the x-tag got applied.

But maybe I could offer this as an option, you’re right.

I digged a little deeper into the issue and one way is to work around using the native CSSOM. While this has the effect of a better performance it would increase the polyfill weight from 18kb minified, non-gzipped to 48-50kb. I think that’s not really worth the trade-off so I’ll do a bit more research on how to optimize the polyfill when used on more than just a couple of elements on a site.

thany commented 9 years ago

Oh well, just one more FF version to go and IE will be the only remaining non-supporting browser. Then you can fairly safely abuse IE-only functions ;)

anselmh commented 9 years ago

Yep, I know and that’s so awesome. Still the IE issue around and unfortunately IE sucks at the very same functions as Firefox does. And they have no plan yet on implementing object-fit so it won’t happen in the near future.

O-Zone commented 9 years ago

Everyone in here interested in a real and long lasting solution should also remember to log in here: https://status.modern.ie/objectfitandobjectposition and whine about IE not planning on implementing object-fit. I'm not sure it will help, since there are lots of other features they should implement, and the competition is strong - but it won't hurt to try! ;-)

anselmh commented 9 years ago

Duplicate of #22 so closing this now. Discussion is going on in the other issue.