v2: jumpy in Firefox

[tomByrer]

v2-dev today, Firefox DevEd, 46.0a2 (2016-02-15), OSX

Resizing the test page, the images (most obviously lizards & Minimum cropping region) will resize smaller than the section 'viewport' & jump back into their correct size. Oddly, sometimes won't happen if refreshed or after switching tabs & switching back. Settings on bottom don't seem to affect.

Works smooth in Chrome & Safari.

Might be part of their tweaking the renderer, so sort of known issue there. https://hacks.mozilla.org/2016/02/smoother-scrolling-in-firefox-46-with-apz/ Also might be part of fixing another bug? https://github.com/scottjehl/picturefill/blob/master/CONTRIBUTING.md#faq--frequent-issues

---

Thanks for the "Minimum cropping region"; exactly what I've been looking for!

[jonom]

Thanks for the report, have been able to replicate in 44.0.2. Seems to be intermittent so that's annoying :) If you find a fix feel free to open a PR, otherwise I'll look in to it when I get time.

[tomByrer]

Thanks for the fast reply!

Hmmmm Could be a timing issue &/or needs extra processing trigger when resized in Fx. I noticed you don't use requestAnimationFrame for throttling; is there a reason for that?

[jonom]

I noticed you don't use requestAnimationFrame for throttling; is there a reason for that?

Hadn't occurred to me but browser support is a bit of a factor... not concerned about IE8 but would prefer to keep IE9 support.

It does look like this is a throttling issue, if I turn throttling off it's nice and smooth in Firefox. I think most of the latest browsers have throttling built in to the resize event which reduces the need to throttle the resize function, but would be good to have it configurable.

Any chance you want to open a pull request to introduce requestAnimationFrame support with setTimeout fallback? Example here: https://developer.mozilla.org/en-US/docs/Web/Events/resize

[tomByrer]

Well, MS EOL'd all IE<11; you'll get a nasty note when you open the browser, & IE9 is only 1% of the browser market now... either way there are polyfills. I was thinking about lowering the framerate from requestAnimationFrame's 60FPS to something around 25FPS. Less calculations, redraws, less battery, & not sure if it would be noticeable.

[jonom]

I'm not too worried about battery life etc. since we're only talking about what's happening during the resize event. If someone is resizing their browser for more than a few seconds at a time they're doing it wrong :) but yeah 25fps would be fine by me. I changed the CSS in this version so that images stretch instead of moving in to weird places and exposing the background in between adjustFocus() calls - so keeping a high frame rate isn't so important.

I'm a fan of progressive enhancement so if it's not hard to provide a fallback and support older browsers that's what I'd rather do.

[tomByrer]

Starting to look at the code; mind if I refactor a bit first? Don't want to be picky, but I believe I can find a better variable name than a. ;)

I also noticed that you use transform-origin but not transform. I thought transform might work better, since it is GPU accelerated, & I think that top, left, etc are still not GPU accelerated. Have you tried switching yet?

[jonom]

Ha, well I know what a means :) wanted it to be short but admit it's not very transparent. Yeah you can do some refactoring, just make sure there's a good reason for anything you do and do a separate commit for each piece of new functionality so I can follow along easier. Have been meaning to try using transform if supported by browser just haven't got around to it. (Note: yes it's possible to set transform origin with this plugin but the plugin itself doesn't make use of that setting) There's an open PR for that it's just out of date if you want to look at it. Look forward to seeing what you come up with!

[tomByrer]

do a separate commit for each piece of new functionality so I can follow along easier

Sure thing, but I'm used to being told to 'squash' commits so I'll have to adjust. Prefer smaller PRs, or 1 big PR? I'm thinking the refactoring should be a first, separate PR.

[tomByrer]

First thing I did was to reduce the image file sizes (I'm kinda nerdy about optimization). But Github is down, so I'll send in a PR later.

On Sun, Feb 21, 2016 at 9:04 AM, Jono Menz notifications@github.com wrote:

Ha, well I know what a means :) wanted it to be short but admit it's not very transparent. Yeah you can do some refactoring, just make sure there's a good reason for anything you do and do a separate commit for each piece of new functionality so I can follow along easier. Have been meaning to try using transform if supported by browser just haven't got around to it. (Note: yes it's possible to set transform origin with this plugin but the plugin itself doesn't make use of that setting) There's an open PR for that it's just out of date if you want to look at it. Look forward to seeing what you come up with!

— Reply to this email directly or view it on GitHub https://github.com/jonom/jquery-focuspoint/issues/46#issuecomment-186850617 .

[tomByrer]

Seems some of my refactoring is also optimizing. 1 if statement removed.

Sorry I already made a mess in my commits, but you can test if I blew anything up yet ;)

Noticed that you are using an object for saving data, seems variables are faster, so that will be next on my STDs. eg data.shiftPrimary into dataShiftPrimary or maybe just shiftPrimary. Since resizing hits this code many times/second, should help a bit.

[jonom]

1 if statement removed.

:+1:

Noticed that you are using an object for saving data, seems variables are faster, so that will be next on my STDs.

I'm luke-warm on this one, the idea of keeping it all in an object is I can easily get a read out of it for debugging. If you can demonstrate a significant performance gain I would be keen to see it done but otherwise I'd rather not sacrifice the convenience.

[tomByrer]

Could just remove the dot; keep readability & improve perf. Last time I checked "jsperf object vs variable" variables won. Even if I cap those calculations at 60FPS, that is still alot of lookups.

[tomByrer]

Seems removing the data object was only a slight improvement, likely as much as removing 3/4 of the element lookups. Likely modern JS engines automatically do optimizations like this, but I put in code anyhow; less horizontal noise when eyeballing.

[tomByrer]

New PRs, but you can still read the old PR if you want to view each step. Might add back the removal of the data object later when I can test better on a bigger screen.

[tomByrer]

Thanks for accepting PRs; I hope they truely make it more readable for you also. Just happens to save a few CPU.

I'm thinking what is needed is a debounce rather then throttling. Hard to explain; I think of a debounce like a state machine with an input hooked into a timer & another hooked into the what you want to limit.

So if there is 1 or more changes with in a time period, only the last update will pass though at the end of the timer. The timer will only be started when there is at least 1 input, otherwise it will send nothing (I used to use this to update knobs' values) Visual examples: A B C lodash example

What do you think?

[jonom]

Cheers Tom.

Whether to use debounce or throttle probably depends on context... for me I would always want to use throttle because I want to re-render as often as possible, hopefully fast enough to be smooth and look like it's happening in real time. Using debounce would mean the containers would only adjust once when you're done resizing the window, and images would be stretched until that moment, so it's not the most visually appealing option. If you use debounce there won't be much gain in making performance optimisations because calls to adjustFocus() will be spread apart by a long distance.

Certainly could see some people preferring debounce though - could add another setting to let people choose between debounce or throttle?

[tomByrer]

Could switch. But in general you only want to calculate at max target refresh, which is 60FPS (though I think sometimes perception can go down to 15FPS). Extra calculations are wasted at best, or can slow down performance. So you only want to render at most your target FPS. Debouncers should always send values after n-time after a trigger, or else they would not be a debouncer.

Guess I'll have to demo.

[jonom]

I think you're mixing up debounce and throttle... although the definition probably shifts depending on where you read it. See: https://css-tricks.com/the-difference-between-throttling-and-debouncing/ throttle is used for rate limiting (e.g. once every 50ms while the user is resizing the window), debounce is used to delay execution and do it a single time (e.g. only once the user has stopped resizing the window).

[jonom]

So I tried implementing requestAnimationFrame but unfortunately it doesn't fix the FF jumpiness bug. See https://github.com/jonom/jquery-focuspoint/commit/40cb1480421bd2ba6fcd0a485eb8b1dbcc80d2cf

Something I discovered with requestAnimationFrame is there's no way to limit the frame rate... so the existing setTimeOut might be better when you want to enforce a low frame rate, but requestAnimationFrame is probably better for a high one.

I'm able to reproduce that bug pretty consistently by the way by just visiting a different tab then coming back to a FocusPoint page. Always works initially and after a re-load it seems.

[jonom]

Okay thinking about all this frame rate stuff, another approach would be to not let people set their own frame rate, let requestAnimationFrame do that and fallback to 15fps with setTimeout. This could have the effect of increasing the animation rate to be greater than the resize event rate, but capped (I assume) at 60fps. Implemented here: https://github.com/jonom/jquery-focuspoint/commit/6595a5a8db6460e4a540ef6528da397fe6d7f30c any thoughts? Since we're only talking about window resizing here I think this is probably an okay approach, and seems to perform quite well for me. Note that there's a huge performance hit if you have dev tools open in chrome so if you test this make sure they're shut.

I removed css transitions for width and height from the test file in this commit and now can't reproduce the jumpiness bug in Firefox so maybe that was part of it? I thought I tried that already but who knows.

[tomByrer]

I'm not sure where you get debouce = "only once the user has stopped resizing the window"? From the CSSTricks article you linked:

Debouncing enforces that a function not be called again until a certain amount of time has passed without it being called. As in "execute this function only if 100 milliseconds have passed without it being called."

eg, the following debounces every 5ms:

 1 2 12345     1 23 
----2----5--------3

Thanks for the extra coding; let me log into my other computer & check. I'll keep in mind about devtools... might have to dig up another FPS timer somewhere...

[jonom]

Say it takes you 2 seconds of dragging to resize a window, during that 2 seconds the resize event is continuously firing - let's say every 10ms. If you set a function to debounce with a delay of 250ms the function won't execute at all during that 2 seconds, it will just fire once at the 2.25 second mark. This is because it doesn't pass the test execute this function only if 250 milliseconds have passed without it being called (not run). Each time it asks if the function has been called within the last 250 milliseconds the answer will be yes, because it will have been called within the last 10 milliseconds.