Skip to content

Run snapshot post-layout state steps in resizeObserver loop #11613

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Run snapshot post-layout state steps in resizeObserver loop #11613

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions source
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4063,6 +4063,7 @@ a.setAttribute('href', 'https://example.com/'); // change the content attribute
<li><dfn data-x-href="https://drafts.csswg.org/cssom-view/#evaluate-media-queries-and-report-changes">evaluate media queries and report changes</dfn></li>
<li><dfn data-x-href="https://drafts.csswg.org/cssom-view/#scroll-a-target-into-view">Scroll a target into view</dfn></li>
<li><dfn data-x-href="https://drafts.csswg.org/cssom-view/#scroll-to-the-beginning-of-the-document">Scroll to the beginning of the document</dfn></li>
<li><dfn data-x-href="https://drafts.csswg.org/cssom-view/#run-snapshot-post-layout-state-steps">run snapshot post-layout state steps</dfn></li>
<li>The <dfn data-x="event-resize" data-x-href="https://drafts.csswg.org/cssom-view/#eventdef-window-resize"><code>resize</code></dfn> event</li>
<li>The <dfn data-x="event-scroll" data-x-href="https://drafts.csswg.org/cssom-view/#eventdef-document-scroll"><code>scroll</code></dfn> event</li>
<li>The <dfn data-x="event-scrollend" data-x-href="https://drafts.csswg.org/cssom-view/#eventdef-document-scrollend"><code>scrollend</code></dfn> event</li>
Expand Down Expand Up @@ -114579,6 +114580,14 @@ import "https://example.com/foo/../module2.mjs";</code></pre>
<ol>
<li><p>Recalculate styles and update layout for <var>doc</var>.</p></li>

<li><p><span>Run snapshot post-layout state steps</span> for <var>doc</var>. <ref>CSSOMVIEW</ref></p></li>

<li><p>If any snapshotted state changed:</p>
<ol>
<li><p>Recalculate styles and update layout for <var>doc</var>.</p>
</ol>
</li>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine, but it might be worth at least acknowledging the trade-offs we're making by making it run on every iteration. Also I wonder if we should update content-visibility before or after this...

E.g., what about:

  • Update style and layout.
  • Do initial content visibility determination and scroll snapshotting.
  • If we had an initial determination, or snapshotting changed, continue.
  • ...

Might be a bit easier to reason about?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I just ignored the content-visibility part. Would it make sense to also move the viewport proximity to the content-visibility spec and say that it's done when asked to "Run snapshot post-layout state steps"?

Is the continue part for content-visibility guaranteed to terminate, though? I'm pretty sure it would not be for snapshotting scroll-animations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is, afaict, since you eventually run out of elements that need the initial determination done.

Hmm, it'd be a bit weird that how much a scroll animation advances would depend on whether there are any resize observers or so, I think that means we probably want to do this step only once (maybe at the end so it can react to Resize observers, like the focus fix up?).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's done at the end, it would mean resize observers will not see scroll-animations, scroll-state queries, and anchor-positioning taken into account. I don't know what's worst. Doing it once at the start is another option.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, indeed. Maybe just do it once on the first iteration of the loop? Any change by a resize observer would be for next frame, which seems fair?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, basically:

  • Set didSnapshotPostLayoutState to false
  • While true:
    • Recalculate styles and update layout for doc
    • If didSnapshotPostLayoutState is false
      • Run snapshot post-layout state steps for doc.
      • Set didSnapshotPostLayoutState to true.
      • If any snapshotted state changed:
        • continue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fine. Do we want to merge with the initial c-v determination or so, or move that into the snapshot-post-layout state steps? That does run on every step of the loop r/n...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding @vmpstr to get the input on whether it's intentional/important that the c-v determination runs for every iteration of resize observers.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I had imagined that this should run once after at least the first round of resize observers so that animations could use the size set by the resizeobserver in the initially rendered frame.

resize observers will not see scroll-animations, scroll-state queries, and anchor-positioning taken into account

This is really tricky, however I think for the use cases we have now, running after makes more sense.

  • scroll driven animations usually care more about observing the output of resize observers and are not likely to change resize observer determining things since that is inefficient to animate.
  • scroll-state queries again are usually used for subtle non size affecting things and are more likely to depend on the resize observer output as that may determine whether something is e.g. stuck.
  • anchor positioning is the one case where i could see it often being used to determine a size, however given that we already allow for a stale size on the compositor it seems like it wouldn't be too unexpected to have it not update until the next frame.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If running after resizeObservers needs to be done, we can:

  1. If we run it for all resizeObserver iterations, run it at the same time as content-visibility, and even make content-visibility a "run snapshot post-layout steps".
  2. If we run it once, we can do one of:
    a. run it right after broadcasting resize observations, if it hasn't been run before.
    b. run it once when we no longer have active resize observations, and continue if necessary.


<li><p>Let <var>hadInitialVisibleContentVisibilityDetermination</var> be false.</p></li>

<li>
Expand Down