Repository navigation
fix(🍏): ask a display link made after a recycle or a trip for the full rate - #4350
YevheniiKotyrlo wants to merge 2 commits into
Conversation
…l rate requestFullFrameRate asked once per view: stopDisplayLink invalidated the link when the view left its superview or was recycled, but kept _fullFrameRateRequested, so the next link never asked and ran at 60 Hz on a ProMotion display. Fabric recycles SkiaViews, so a canvas mounted into a recycled view presented at half the display's rate, as did one a scroll view that clips its subviews took away and brought back. stopDisplayLink now clears the flag with the link it invalidates.
MeasuredA canvas animating on every UI frame, measured as #4273 measured it, with a probe in A new link asks for the full rate. iPad Pro 12.9" (4th generation, iPadOS 27, 120 Hz):
When a scroll view with
A link made before its view has a window, which a probe-only build forces by scheduling a frame at the end of The iPad runs a link that asks for nothing at 120 Hz anyway, so the rate it presents at does not move there; the iPhone 14 Pro in #4273 runs such a link at 60. No ProMotion iPhone was available here, so this PR's rate on one is not measured: its ticks a second after a recycle would show it. An iPhone 12 Pro Max (iOS 27, 60 Hz) asks for nothing on either side and ticks at 60. A frame whose link exists no longer pays for the request. What
This PR's range sits below Checking the link's range on every frame insteadWith The probeEvidence only: https://github.com/YevheniiKotyrlo/react-native-skia-evidence/tree/main/pr-4350. |
…ic-canvas-image-view wcandillon#4324 now only keeps a view's native view, and what it shows, out of its superview: its recording slots, colour helpers, display link reset and view registry cleanup became wcandillon#4348, wcandillon#4349, wcandillon#4350 and wcandillon#4351. This branch carries wcandillon#4324 as it stands, and wcandillon#4349, whose helpers the region readers in setup.tsx build on; stopDisplayLink leaves the reset to wcandillon#4350.
wcandillon
left a comment
There was a problem hiding this comment.
That's a good catch. Can we remove the _fullFrameRate flag instead?
Something like this:
- (void)requestFullFrameRate {
#if !TARGET_OS_VISION
- if (_fullFrameRateRequested) {
+ if (_displayLink.preferredFrameRateRange.maximum != 0) {
return;
}
UIScreen *screen = self.window.screen;
if (screen == nil) {
return;
}
- _fullFrameRateRequested = true;
const float maxFps = (float)screen.maximumFramesPerSecond;
if (maxFps > 60) {
_displayLink.preferredFrameRateRange =
CAFrameRateRangeMake(60, maxFps, maxFps);
}
#endif
}
requestFullFrameRate runs where scheduleFrame makes a link, and in didMoveToWindow for a link made before its view had a window, in place of every scheduleFrame behind _fullFrameRateRequested, so the flag and stopDisplayLink's reset go. Checking the link's preferredFrameRateRange on every frame instead would read the range each frame, and on a 60 Hz display, which never gets one, the window's screen too.
|
Checking the range on every frame added time to every frame: on the iPad the range's getter took 0.7–0.9 µs a frame, and on an iPhone 12 Pro Max, a 60 Hz display that |
Problem
On a ProMotion iPhone, a canvas presents at 60 Hz once its
SkiaViewwas recycled, or taken out of its superview and put back.requestFullFrameRate(#4273) asks a display link for the display's full rate once per view:stopDisplayLinkinvalidates the link when the view leaves its superview or is recycled, and the nextscheduleFramemakes a new one, which then never asks; on the iPhone 14 Pro #4273 measured, such a link fires at 60 Hz. Fabric recyclesSkiaViews, so a canvas mounted into a recycled view presents at half the display's rate, and so does one a scroll view withremoveClippedSubviewstakes away and brings back.Solution
requestFullFrameRateruns wherescheduleFramemakes a link, and fromdidMoveToWindowfor a link made while its view had no window, in place of everyscheduleFramebehind_fullFrameRateRequested. Each link asks for itself, so the link made after a recycle or a trip asks for the full rate like the first one, and the flag goes with nothing to reset. A frame whose link exists no longer calls the request.Tests
None in the repo can show it: CI's iOS simulator has no 120 Hz display, and no test target reaches the display link. On an iPad Pro, the link made after a recycle asks for 120 where
main's asks for nothing, as does the one made after a trip on #4324's head; the iPad runs either at 120, and no ProMotion iPhone was available, so the rate a canvas presents at on one is not measured with this PR. WhatscheduleFramespends on the display link a frame goes from 1.48 to 0.88 µs on the iPad and from 1.13 to 0.63 µs on a 60 Hz iPhone 12 Pro Max: the evidence comment.Verification
clang-format clean; the example's iOS Release build. Every changed line is outside macOS, so CI's macOS build compiles
SkiaView.mmas onmain.Related
Split out of #4324, whose views come back to their superview with a new display link. #4328 moves
SkiaView.mm's display link and its full-rate request into a window's presentation; whichever lands second carries this fix into it.Base
mainfeee40afc.