Implement the procedure for calculating an auto-aligned start time

[johannesodland]

Currently animation range start and range end is mapped to delay and endDelay of the animation effect. This causes the finished promise to resolve at the wrong time, and similarly the finished event is emitted at the wrong time.

This PR implements the procedure for calculating an auto-aligned start time. (https://github.com/w3c/csswg-drafts/pull/9181/files). It aligns the start time based on animation range start, no longer maps ranges to effect delay and end delay.

Summary of changes to proxy-animation:

• Remove the reset current time on resume flag (https://github.com/w3c/csswg-drafts/pull/9181/files)
• Update the conditions for when an animation is ready (https://drafts.csswg.org/web-animations-2/#waiting-for-the-associated-effect)
• Update timeline, startTime and currentTime setters so that they sets the auto align start time flag
• Update playInternal so that
  • it sets the auto align start time flag
  • it delays add animation until after the ready promise is created. (Otherwise it will run while playState is idle)
• Update tickAnimation so that it runs the auto align start time procedure
• Take range duration into account when
  • converting to and from CSSNuberish
  • calculating computed timing

Summary of changes to scroll-timeline-base:

• Remove renormalization callback, as it was only necessary when start and end delay was mapped to ranges
• Store additional values on new source and on resize: scrollWidth, scrollHeight, clientWidth, clientHeight
  • Values are stored once after resize
• Store ranges and update them on resize, as the auto align procedure will access them frequently

[flackr]

Overall very cool to see the auto-aligned start time calculations implemented.

I just landed support for showing the diff on test results 5a2639a . When you push another commit to this branch it should be enough to kick off the github action that will add the diff in the test results to the PR. I'm hoping this will help demonstrate that this is tested by the existing wpt tests and passes more as a result.

[flackr]

Observing every child seems a bit excessive, especially since those children could later be moved or children could be added. Do we need this change for this particular PR? This seems like something that could be isolated to its own improvement. I have ideas for how we might use intersection observer to observe positional changes of the subject would could be used in combination with resizeobserver.

[johannesodland]

It was necessary to pass some of the subtests, but we can definitely defer this.

[johannesodland]

I have dropped the child observations. It doesn't cause any regressions, so that was definitely excessive 🙏

I have, however, added a mutation observer for class and style attributes to prevent regressions in /scroll-animations/scroll-timelines/scroll-animation-inactive-timeline.html.

I hope that's OK.

[flackr]

As long as we save off the measurements of the subject and scroller, i'm not sure that there's much need to cache the named ranges. E.g. it's a pretty simple and fast calculation if we're just using saved metrics.

[johannesodland]

That might be a cleaner solution, indeed.

We just need to store some more measurements about the subject, such as top and left. calculateRange currently iterates through all offset parents up to the scroll containers offsetParent, calling offsetLeft and offsetTop to calculate the relative position of the subject. This will invalidate the layout.

I'll look into it and update the PR

[johannesodland]

It turns out that calculateRange is used in scroll-timeline-css.js to calculate keyframe offsets as well. It is used before any timelines have been instantiated, so we won't have access to any stored measurements here.

scroll-timeline/src/scroll-timeline-css.js

Lines 96 to 102 in 5a2639a

	const tokens = value.split(" ");
	if(tokens.length == 1) {
	newOffset = parseFloat(tokens[0]);
	} else {
	newOffset = relativePosition(tokens[0], container, options.subject,
	axis, options.inset, CSS.percent(parseFloat(tokens[1]))) * 100;
	}

scroll-timeline/src/scroll-timeline-css.js

Lines 54 to 58 in 5a2639a

	function relativePosition(phase, container, target, axis, optionsInset, percent) {
	const phaseRange = calculateRange(phase, container, target, axis, optionsInset);
	const coverRange = calculateRange('cover', container, target, axis, optionsInset);
	return calculateRelativePosition(phaseRange, percent, coverRange);
	}

We could refactor calculateRange to separate measurements and calculations, so that it can be used both places.
The measurment and calculation of the subjects relative position should also be included in the stored measurements to prevent reflow:

scroll-timeline/src/scroll-timeline-base.js

Lines 522 to 532 in 5a2639a

	let top = 0;
	let left = 0;
	let node = target;
	const ancestor = container.offsetParent;
	while (node && node != ancestor) {
	left += node.offsetLeft;
	top += node.offsetTop;
	node = node.offsetParent;
	}
	left -= container.offsetLeft + container.clientLeft;
	top -= container.offsetTop + container.clientTop;

If we do this, the stored measurements will grow to include.

const measurements = {
  source: {
    scrollLeft,
    scrollTop,
    scrollWidth,
    scrollHeight,
    clientWidth,
    clientHeight,
    writingMode
  },
  subject: {
    top,
    left,
    offsetWidth,
    offsetHeight
  }
}

I'll start to look into it, hopefully next week.

[flackr]

This sounds good to me. I was expecting we would include all required measurements in the structure so that once we've stored it we never access any layout inducing properties again.

I have a longer term goal to do this for all timelines before setting the progress in any animations so that we don't dirty the layout from one timeline update before the next one does more calculations but we don't have to do that in this patch.

[johannesodland]

👍

We are already storing source measurements and calculated ranges per source element (and not per timeline) before updating the timelines and ticking the animations. I don't think refactoring will change this, so we'll store both source measurements, subject measurements and relative positions, before we're updating the timelines and animations :)

Atm we're only observing changes to the source element. I'm not sure how we'll do this once we start observing the subject, but we'll look into this when we get there :)

[johannesodland]

@flackr I have refactored the code now to separate measurement and range calculations.

On resize and style changed callbacks we first measure the source element any timeline subjects, before updating timelines and animations. Ranges are now calculated on the fly based on the measurements.

I have also switched to using weak refs for the set of timelines that are connected to a source element, so that timelines are not prevented from being garbage-collected.

[johannesodland]

Overall very cool to see the auto-aligned start time calculations implemented.

I just landed support for showing the diff on test results 5a2639a . When you push another commit to this branch it should be enough to kick off the github action that will add the diff in the test results to the PR. I'm hoping this will help demonstrate that this is tested by the existing wpt tests and passes more as a result.

Nice :)
I've been running tests and diffing by hand. It will be nice to have this automated 🙌

[flackr]

Sorry to go back and forth on this - I think the ResizeObserver observing each child is better than observing changes to style / class attributes which indirectly can affect size. It would be good to also have a mutationobserver observe added / removed children but this can be done separately.

[johannesodland]

Don't be, I'm truly happy for all the input.

We can definitely observe added/removed children. I think now is a good time to include that, but I can file a separate PR for that if you'd like that.

About observing the style attribute:

There are a few tests that toggle whether the timelines are active through toggling the overflow style property.

• https://github.com/web-platform-tests/wpt/blob/55a2476161170898d9f19db51918353b2e2e9640/scroll-animations/scroll-timelines/scroll-animation-inactive-timeline.html#L61
• https://github.com/web-platform-tests/wpt/blob/55a2476161170898d9f19db51918353b2e2e9640/scroll-animations/scroll-timelines/scroll-animation-inactive-timeline.html#L89

There were regressions on a few of these tests, as we now have to wait for a startTime/holdTime before committing a pending play task.

I added the mutation observer to detect when the overflow property changed through inline styles. The approach is admittedly naive and triggers unnecessary updates, while only detecting a limited subset of the style changes we're interested in.

We can:

1. drop the observer and live with the few regressions, or
2. come up with a smarter way to detect the changes 🤔 Let me know if you have any good ideas 😀

[flackr]

I can file a separate PR for that if you'd like that.

No need! I was worried about properly cleaning up listeners when children are removed / observing new children but the former is handled by the disconnect call, and the latter we can make part of the mutationobserver later.

I added the mutation observer to detect when the overflow property changed through inline styles. The approach is admittedly naive and triggers unnecessary updates, while only detecting a limited subset of the style changes we're interested in.

Ah I understand, thanks for the explanation. I'm okay with keeping this in for now. For these things which are for handling edge cases correctly for tests but not required most of the time I'd like to eventually put them all behind some flag. E.g. if people are using this in production scenarios they would probably prefer not to pay any additional costs and instead carefully avoid those edge cases or manually trigger an update themselves.

[johannesodland]

Using a flag to toggle this would be nice.

I'll keep observation of the "style" attribute then, but remove observation of the "class" attribute, as that's not necessary for passing the tests.

[flackr]

This looks amazing! Can't wait to get this landed!

[flackr]

This looks great! Thank you! I'll land this before my PR so as to not create any conflicts needing resolving.