Hook into the requestAnimationFrame for UI changes

[KwintenP]

Hi Dom,

First of all kudos on the lib! Looks really nice!

I looked at the code and there was one thing I noticed. You use the auditTime operator to wait 16ms before updating for example the selectedItems or the selectBox. This way, Angular will only update the UI every 16ms which makes sense, 60FPS ftw.

I was just wondering if it would make even more sense to hook the updates you want to do into the requestAnimationFrame hook. Using the subscribeOn operator this should be fairly easy. You could update your code from:

selectBox$.pipe(
  auditTime(AUDIT_TIME),
  withLatestFrom(mousemove$, ...),
  filter(() => this.selectOnDrag),
  filter(({ selectBox }) => selectBox.width > MIN_WIDTH || selectBox.height > MIN_HEIGHT),
  tap(({ selectBox, event }) => this.selectItems(selectBox, event)),
  takeUntil(this.destroy$)
).subscribe();

to

selectBox$.pipe(
  auditTime(AUDIT_TIME),
  withLatestFrom(mousemove$, ...)
  filter(() => this.selectOnDrag),
  filter(({ selectBox }) => selectBox.width > MIN_WIDTH || selectBox.height > MIN_HEIGHT),
  tap(({ selectBox, event }) => this.selectItems(selectBox, event)),
  takeUntil(this.destroy$),
  subscribeOn(animationFrame),
).subscribe();

By adding the subscribeOn(animationFrame), RxJS will schedule this execution to be run as a callback in the requestAnimationFrame and thus just before repainting. I'm not 100% sure if this makes absolute sense in the context of Angular, since you would only update properties on the scope of your component and it still Angular that needs to update the DOM. But since zone.js monkey-patches requestAnimationFrame, Angular will run CD on this hook so I guess it makes sense.

As I am not 100% sure, any feedback, input is more than welcome.

If we can agree this could enhance the library, I'd be happy to create a PR.

Let me know what you think :)

[cartant]

You could make this a little bit simpler by taking advantage of the fact that auditTime accepts a Scheduler:

selectBox$.pipe(
  auditTime(0, animationFrameScheduler),
  withLatestFrom(mousemove$, ...),
  filter(() => this.selectOnDrag),
  filter(({ selectBox }) => selectBox.width > MIN_WIDTH || selectBox.height > MIN_HEIGHT),
  tap(({ selectBox, event }) => this.selectItems(selectBox, event)),
  takeUntil(this.destroy$)
).subscribe();

Then you wouldn't need the 16 ms constant.

[KwintenP]

Hi Nicholas,

I'm not 100% sure this will 'fix the problem' (quotes as this is a micro optimisation so not really a problem).

I thought about this as well, this will schedule all the changes to the selectBox$ on the animationFrame scheduler. However, imagine a worst case (and probably unlikely) scenario that you have 1000 events coming in to the selectBox$ and scheduling them all to be executed on the 'animationFrame'. When fired, these 1000 events will be executed in sync and all of the operators after the auditTime operator. If this sync handling of the 1000 events and all of the operators takes more than 16ms, we are actually dropping frames.

The use of auditTime here is to make sure that there is only a single event every 16ms to be handled during the animationFrame hook. So by placing it on 0, this seems to open up some new issues.

By using subscribeOn(animationFrame), the execution of a single event (thx to the auditTime) is delayed on that hook, and all the operators have already fired in that case.

Hope I'm making sense :) What do you think?

[cartant]

Kwinten, it's been a while since I last poked around in the Scheduler-related parts of the RxJS source, but I'm pretty sure that when using a scheduler, an action is always scheduled to be executed asynchronously. So specifying 0 as the duration for the auditTime call won't see events handled synchronously.

Internally, auditTime uses audit with a duration selector that returns a timer - created using the duration and scheduler arguments passed to auditTime. From memory, only the most-recently-received element is kept in auditTime and when the timer emits, that element is emitted.

So if 1000 events are received synchronously, only the last should be emitted and only when requestAnimiationFrame invokes the callback it was passed. In that callback, the scheduler's current time will be checked against the action's due time - which will have been the time at which the action was scheduled plus 0 - and if the current time is greater than or equal to the due time, the action is executed.

auditTime(0, animationFrameScheduler) should behave in manner very similar to auditTime(16), but the element emissions should always be in-sync with the browser's refresh rate - rather than just 16 milliseconds after the initial element is received.

I'll likely look though the source later to verify that my understanding is correct (I could always be wrong). Unless I have it completely misremembered or misunderstood the implementation, I'll post some links to the source to try to better explain things.

[KwintenP]

Hi Nicholas,

I think my explanation was not 100% clear. What you are describing is 100% correct AFAIK. My scenario was a little different though:

Let's take the following code:

interval(1).pipe(
   auditTime(0, animationFrame)
).subscribe();

For simplicity sake, lets assume the browser has just done a repaint and will again in 16ms when this code starts. The interval will, before the requestAnimationFrame fires again, emit 15 values. All these 15 values are delivered async and will all be scheduled on the animationFrame scheduler to be executed (which always happens async as you stated).

After 16ms, the requestAnimationFrame scheduler fires and all the queued actions, in this case 15, will get executed. And, I believe, that this poses a problem. In fact, you only want the last one to get executed, not the 14 previous ones. Using auditTime in combination with 0 time frame will give you this problem.

The scenario I was describing was when you have a 1000 values delivered async 'before the auditTime' , which schedules a 1000 tasks on the animationFrame. That makes the 1000 values to be executed in 'sync' when the 'requestAnimationFrame' is triggered.

[cartant]

Kwinten, I'll re-read your comment in the morning - it's late, here - but I'm not convinced that anything is scheduled in audit other than the timer that's returned by the duration selector.

See the implementation of _next.

Until the timer emits - that is, until the next animation frame - _next simply stores the most recently emitted value from the source and waits for the timer. When the timer emits, clearThrottle is called, emitting a value to the destination. So I cannot see how more than one value can make it past audit, whether values are emitted synchronously or not.

[d3lm]

Great discussion!

For what it's worth, I don't think that it would be useful to use the animationFrame scheduler on the stream where I essentially select the items while dragging.

This is the stream I am talking about https://github.com/d3lm/ngx-drag-to-select/blob/master/src/lib/src/select-container.component.ts#L154-L166.

This stream is not really UI facing and I would argue that it only really makes sense to apply the animationFrame scheduler to streams that affect rendering, and this would be the selectBoxStyles$ Observable. https://github.com/d3lm/ngx-drag-to-select/blob/master/src/lib/src/select-container.component.ts#L168-L175

This one doesn't use auditTime tho. Here it would probably make sense to use subscribeOn and then use the animationFrame scheduler to schedule the events whenever requestAnimationFrame would fire. This is the only UI facing stream that affects rendering.

I am mainly using auditTime to throttle the calculations a bit so that I only check roughly every 16ms which items need to be selected, depending on whether they are within the selection rectangle or not.

[KwintenP]

Nicholas, you are completely right :).

I looked at the code snippet you referenced and now I get it. I misinterpreted how auditTime works. I thought that if you did e.g. auditTime(10, animationFrame) it would wait 10ms and after 10ms schedule the last event on the animationFrame. While in reality it uses the animationFrame in the durationSelector. Hence my confusion with the multiple events.

Should have checked the sources sooner but didn't have time at work today. Thx for the explanation, cleared things up a lot :)

[cartant]

@d3lm I've not given it too much thought, but I don't see the harm of using the animation frame scheduler. It's the fact that you chose 16 milliseconds that suggests requestAnimationFrame is appropriate. The scheduler is, after all, just a timing mechanism.

@d3lm and @KwintenP One thing that you should do though, is use observeOn instead of subscribeOn. The latter only runs the subscription on the specified scheduler. That is, it delays the subscription to the source using the scheduler. Whereas observeOn will ensure the next, error and complete handlers run on the specified scheduler - and I imagine that's what you really want.