I would like that the API we use for vsync platforms to be the implementation of a generalized interface that we also use for the fallback timer. For inspiration I think we can can follow the API that we use for clipboard. For instance, when you do not install your own custom RefreshDriver, you would fall back to the timer version.

I would like that the API we use for vsync platforms to be the implementation of a generalized interface that we also use for the fallback timer. For inspiration I think we can can follow the API that we use for clipboard. For instance, when you do not install your own custom RefreshDriver, you would fall back to the timer version.

@mrobinson Thanks for your suggession, but I don't understand clearly. In fact, the newly-added BeginFrameSource is a generalized interface and the DefaultBeginFrameSource is the fallback timer. Let me guess, do you think that on platforms with Vsync integrated, the entire RefreshDriver could be feature-gated, just like the clipboard?

Besides, this PR's current change try to add a general frame tick observer and notification mechanism. However, this might be slightly beyond the scope of this PR. If a type similar to RefreshDriverDelegate were added, solely for the purpose of RefreshDriver observing frame ticks, perhaps the intent of this PR could be made clearer.

For inspiration I think we can can follow the API that we use for clipboard. For instance, when you do not install your own custom RefreshDriver, you would fall back to the timer version.

Could you elaborate here to make sure we are on the same page? As far as I can see there currently is a ClipboardProvider trait, but no way for the embedder to install a custom clipboard provider. Instead messages are sent to/from the embedder.

I'm not fully up to date, so maybe I'm missing something, but for me it would make sense for the embedder to be able to install a custom refresh driver, which is then also responsible for calling wake() / informing the embedder of a new frame tick. @coding-joedow Is there any usecase for further observers?

Could you elaborate here to make sure we are on the same page? As far as I can see there currently is a ClipboardProvider trait, but no way for the embedder to install a custom clipboard provider. Instead messages are sent to/from the embedder.

There is a default ClipboardDelegate and then the embedder can call WebView::set_clipboard_delegate() to override it. My thought is that the the default RefreshDriver would use a timer (the current thing that we have) and then the embedder could override it by installing another RefreshDriver which would be driven by the vsync on platforms that support it. Thus we would expose a RefreshDriver trait and the embedder could implement it.

Currently the RefreshDriver has a pretty simple API, but I'm not entirely sure it is reasonable for vsync-based rendering so maybe a discussion about what the trait API looks like is in order.

I'm not fully up to date, so maybe I'm missing something, but for me it would make sense for the embedder to be able to install a custom refresh driver, which is then also responsible for calling wake() / informing the embedder of a new frame tick. @coding-joedow Is there any usecase for further observers?

I think there is no other usecases for now.

Currently the RefreshDriver has a pretty simple API, but I'm not entirely sure it is reasonable for vsync-based rendering so maybe a discussion about what the trait API looks like is in order.

Yes, the current RefreshDriver contains some animations logic except for the timer thread: 1) send TickAnimations message to script thread; 2) limit the rendering rate of animations. We need to clarify the role of RefreshDriver and how its implementation interacts with the compositor.

In my humble opinion, if RefreshDriver becames a trait that can be optionally implemented by embedders, it should solely be responsible for notifying frame ticks and providing an API to start or stop notifications to avoid unnecessary execution of tick notification handlers.

Currently the RefreshDriver has a pretty simple API, but I'm not entirely sure it is reasonable for vsync-based rendering so maybe a discussion about what the trait API looks like is in order.

Yes, the current RefreshDriver contains some animations logic except for the timer thread: 1) send TickAnimations message to script thread; 2) limit the rendering rate of animations. We need to clarify the role of RefreshDriver and how its implementation interacts with the compositor.

@mrobinson @jschwe Do you think it's would be better to move out these animations logic from RefreshDriver ?

Currently the RefreshDriver has a pretty simple API, but I'm not entirely sure it is reasonable for vsync-based rendering so maybe a discussion about what the trait API looks like is in order.

Yes, the current RefreshDriver contains some animations logic except for the timer thread: 1) send TickAnimations message to script thread; 2) limit the rendering rate of animations. We need to clarify the role of RefreshDriver and how its implementation interacts with the compositor.

@mrobinson @jschwe Do you think it's would be better to move out these animations logic from RefreshDriver ?

Sorry for the late reply. Yes, perhaps the animation logic should move out or we should create a RefreshDriverDelegate which handles the inner logic.

Yes, perhaps the animation logic should move out or we should create a RefreshDriverDelegate which handles the inner logic.

Thanks for your reply. I prefer the first idea, but as @jschwe mentioned, there doesn't seem to be a particular reason to introduce a general frame tick notification mechanism at this point. Therefore, I will update this PR based on the RefreshDriverDelegate idea.

Apologies for taking so long to get to this. I have more time now for API-related work.

@coding-joedow You are totally correct that this change is very close to what I was thinking. I have a few minor changes to suggest around the design (in order to make the API surface a bit simpler) and naming, but I would love to tackle those this week.

BTW, with this change and a few other minor ones, I was able to enable flinging for on desktop with --simulate-touch-events turned on. I think this will finally enable us to have a very consistent touch event handling story on all platforms!

Apologies for taking so long to get to this. I have more time now for API-related work.

@coding-joedow You are totally correct that this change is very close to what I was thinking. I have a few minor changes to suggest around the design (in order to make the API surface a bit simpler) and naming, but I would love to tackle those this week.

BTW, with this change and a few other minor ones, I was able to enable flinging for on desktop with --simulate-touch-events turned on. I think this will finally enable us to have a very consistent touch event handling story on all platforms!

Nice, this sounds very promising. I'm a bit busy at the moment and can't handle this PR right now. Sorry, and feel free to implement your ideas based on this patch, or create a new PR.

I've pushed my modifications to this branch which slightly reworks the original design. In addition, it integrates the old WebView::on_vsync signal into this new design, thereby allowing all platforms that have touch event handling to support fling (this may need to be disabled for Android, which I think also supports fling natively). Notably this allows testing touch and fling using --simulate-touch-events on desktop.

cc @jschwe

fwiw, I tested that patch on a Pinephone Pro running Mobian, and that works very well!

@mukilan I've think I've addressed your review comments. PTAL.

🔨 Triggering try run (#18649147210) for OpenHarmony