the update intersection observations steps should happen after requestAnimationFrame callbacks and layout

[dbaron]

The section on patching the event loop spec says that the insertion of the new step for processing IntersectionObserver should happen before the step for requestAnimationFrame callbacks.

This differs from the change which has already been edited into the HTML spec, which says it should happen after the requestAnimationFrame callbacks, but before the update of the style and layout.

Both of these feel wrong to me. I think it should run after both the requestAnimationFrame callbacks and after the natural update of the layout, since rAF is primarily a writing callback (so should be early, before the natural style/layout), whereas IntersectionObserver is primarily a reading callback (so should be late, after the natural style/layout). This ordering makes the primary use of the callback less likely to cause extra flush/update cycles.

Changing this probably requires a bit of investigation of both current behavior, and perhaps of whether what's described in the HTML spec actually reflects reality (which last time I was involved in a detailed discussion of it, it seemed not to be the case).

I got here from w3ctag/design-reviews#197.

[dbaron]

@slightlyoff points out that the records delivered are out-of-date by a refresh cycle or more, which actually changes some of the arguments here, but I need to dig into that further.

[travisleithead]

This might be interesting for @toddreifsteck to follow as well.

[szager-chromium]

I think the HTML spec is correct: IO should run after rAF, but before "update the rendering."

Please correct me if I'm wrong, but "update the rendering" doesn't mean "update style and layout." As I understand it, the spec has no concept of "update style and layout". From the point of view of the spec, when DOM is modified the new style and layout is immediately available to javascript.

My understanding is that "update the rendering" means "push pixels to the screen." If that's the case, then I think it's correct to run the IO algorithm before "update the rendering".

[szager-chromium]

On Tue, Nov 7, 2017 at 4:15 PM, Ryosuke Niwa notifications@github.com wrote:

Both of these feel wrong to me. I think it should run after both the requestAnimationFrame callbacks and after the natural update of the layout, since rAF is primarily a writing callback (so should be early, before the natural style/layout), whereas IntersectionObserver is primarily a reading callback (so should be late, after the natural style/layout). This ordering makes the primary use of the callback less likely to cause extra flush/update cycles.

But what happens if IntersectionObserver mutates DOM. Are we going to cache the state of the document right before invoking IntersectionObserver, and paint that version?? That is going to be extremely cumbersome to implement.

Keep in mind that there are two separate timing considerations:

1) When does IntersectionObserver measure intersections? 2) When does IntersectionObserver deliver its notifications (i.e., run callbacks)?

(1) Happens during frame generation. This is fine because it's a read-only operation. If a new notification is generated, it is not delivered immediately. Instead...

(2) When a notification is generated during (1), IntersectionObserver will PostTask to deliver the observation. So, the callback will run during regular js execution time, after the frame is finished.

IntersectionObserver is NOT intended to drive frame-precise layout in the way that rAF is. It's intended to provide a notification after a frame has already been displayed.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/w3c/IntersectionObserver/issues/263#issuecomment-342667375, or mute the thread https://github.com/notifications/unsubscribe-auth/AF8gV-Tcf6wx2cybuLrpOoitRe9c9ZT0ks5s0PKKgaJpZM4QDpNX .

[dbaron]

Please correct me if I'm wrong, but "update the rendering" doesn't mean "update style and layout." As I understand it, the spec has no concept of "update style and layout". From the point of view of the spec, when DOM is modified the new style and layout is immediately available to javascript.

My understanding is that "update the rendering" means "push pixels to the screen." If that's the case, then I think it's correct to run the IO algorithm before "update the rendering".

I agree the spec should be a lot clearer here, but I believe the intent is the opposite, and that "update the rendering or user interface ... to reflect the current state" does include updating style and layout. At least, that's how I've always read it.

I think any serious attempt to specify the rendering loop couldn't possibly make the assumption that all DOM modifications lead to instantaneous updates to style and layout. I thought the specification defined the concept of flushing these somewhere, but I can't currently find it.

[dbaron]

Also, I think my statement that the IntersectionObserver steps involve reading and not writing is even stronger given that the script callbacks aren't actually run during those steps. If only the steps in the spec run, then I think it's clear that only reading happens during those steps, and therefore they should occur after style and layout updating happens.

[dbaron]

OK, I guess I see now that the spec's placement of the updates in the list would make sense given the interpretation of the HTML spec by @szager-chromium in https://github.com/w3c/IntersectionObserver/issues/263#issuecomment-342711088 , whereas with my interpretation of the HTML spec in https://github.com/w3c/IntersectionObserver/issues/263#issuecomment-346200513 I think my proposed movement of the IntersectionObserver updates after the update the rendering steps. So I think resolving this depends on whatwg/html#3246, which in turn probably depends on other things...

[dbaron]

This was modified a bit by #331 (which I propose changing a little further in #336). And a number of remaining issues are covered by whatwg/html#3246.

But I think one pretty basic issue remains here, which is that I was suggesting that the update intersection observations steps should occur not only after requestAnimationFrame (which #331 does) but also after the natural update of layout (which it doesn't change).

[szager-chromium]

Let's see if we can wrap this up so it's not looming over the design review.

The current Event Loop spec has the following:

10. Update the Rendering ...
    11. For each fully active Document in docs, run the animation frame callbacks for that Document, passing in now as the timestamp.
    12. For each fully active Document in docs, run the update intersection observations steps for that Document, passing in now as the timestamp.
    13. Invoke the mark paint timing algorithm for each Document object in docs.
    14. For each fully active Document in docs, update the rendering or user interface of that Document and its browsing context to reflect the current state.
11. (maybe start an idle period) ...

I don't really see how it makes sense to move IntersectionObserver any later in that list, @dbaron, WDYT?

[szager-chromium]

I think step 14 in "Update the rendering" is the really mysterious one. It updates both the "rendering or user interface" and the "browsing context".

"Update the rendering or user interface" sounds like pushing pixels to me, which -- at least in chromium -- definitely happens after IntersectionObserver runs (and I think that ordering makes sense).

I'm not sure what "Update the browsing context" means here, so I don't know if it should happen before or after IntersectionObserver.

[dbaron]

So the point I've been making (see, e.g., https://github.com/w3c/IntersectionObserver/issues/263#issuecomment-346200513) is that I think the "update intersection observations steps" should happen after the UA does layout. (The places where script is run might also do things that flush layout, but there's a default point where layout happens, and I think that should be before the update intersection observations steps.)

[rniwa]

FWIW, HTML5 spec's current model is that style & layout are always up-to-date so update the rendering means to update the rendering on the screen as suggested in https://github.com/w3c/IntersectionObserver/issues/263#issuecomment-342711088

[dbaron]

I don't think it's clear that that's the current spec model; I think it's at best unspecified. (And certainly none of the spec's editors said in whatwg/html#3246 that that's the current spec model.)

[rniwa]

I don't think it's clear that that's the current spec model; I think it's at best unspecified.

I think it's pretty clear from the existing API that it needs to be that way. If someone modifies CSS rules then invokes offsetTop, left, etc... in the middle of resize event or scroll event handler, those events would see the updated dimensions, not some stale state from the past. In fact, this is pretty much how all browsers behave.

[dbaron]

I'm not disagreeing with the idea that APIs return up-to-date state.

[dbaron]

What I'm saying is that I've always seen that "update the rendering" statement in the spec as the start of a so-far-very-incomplete attempt to specify the flushing and batching that browsers do, and I've always interpreted it to say that that's where the default (i.e., before painting) style and layout flushes happen.

[falsandtru]

I met this problem. IntersectionObserver always reports all observation targets. This behavior on the spec makes IntersectionObserver completely useless.

[falsandtru]

I'm combining IntersectionObserver and requestAnimationFrame as follows:

  async function display() {
    let start = Date.now();
    let delta = 0;
    let time = 0;
    let resource = 100;
    for (const el of update(document.getElementById('editor').value)) {
      el.parentNode
         ? observer.observe(el)
         : observer.unobserve(el);
      delta = Date.now() - start;
      time += delta;
      resource = resource - delta > 0
        ? resource - delta
        : void await new Promise(requestAnimationFrame) || 300;
      start = Date.now();
    }
    return time += Date.now() - start;
  }