[Windows] Fix PlatformTicker.Windows.cs — IsRunning always returns false - #35840
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 35840Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 35840" |
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
Expert Review — 1 findings
See inline comments for details.
| // No additional fires after Stop | ||
| Assert.Equal(firesAfterStop, fireCount); | ||
| // ~6 expected at 60Hz over 100ms; 3x subscriptions would yield ~18+ | ||
| Assert.InRange(firesAfterStop, 1, 20); |
There was a problem hiding this comment.
[major] Regression Prevention and Test Coverage — This assertion does not actually catch the duplicate-subscription regression. With the pre-fix implementation, three Start() calls can produce roughly 18 callbacks over 100ms, which still passes the current upper bound of 20. That means the test can pass while Start() is not idempotent. Tighten this to prove duplicate subscriptions are not occurring, for example by comparing against a single-start baseline or using a deterministic frame signal rather than a broad timing range.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@HarishwaranVijayakumar — new AI review results are available based on this last commit:
d449fc3. To request a fresh review after new comments or commits, comment/review rerun.
Review Sessions — click to expand
Gate — Test Before & After Fix
Gate Result: ⚠️ ENV ERROR
Platform: WINDOWS · Base: main · Merge base: 32d494c2
| Test | Without Fix (expect FAIL) | With Fix (expect PASS) |
|---|---|---|
📱 PlatformTickerTests (IsRunning_ReflectsStartAndStop, Start_IsIdempotent_NoDuplicateSubscriptions, AnimationManager_StartsTickerOnce_AcrossMultipleAdds) Category=Animation |
🔴 Without fix — 📱 PlatformTickerTests (IsRunning_ReflectsStartAndStop, Start_IsIdempotent_NoDuplicateSubscriptions, AnimationManager_StartsTickerOnce_AcrossMultipleAdds): ⚠️ ENV ERROR · 218s
No log file found
🟢 With fix — 📱 PlatformTickerTests (IsRunning_ReflectsStartAndStop, Start_IsIdempotent_NoDuplicateSubscriptions, AnimationManager_StartsTickerOnce_AcrossMultipleAdds): ⚠️ ENV ERROR · 222s
No log file found
⚠️ Failure Details
⚠️ PlatformTickerTests (IsRunning_ReflectsStartAndStop, Start_IsIdempotent_NoDuplicateSubscriptions, AnimationManager_StartsTickerOnce_AcrossMultipleAdds) without fix:Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".⚠️ PlatformTickerTests (IsRunning_ReflectsStartAndStop, Start_IsIdempotent_NoDuplicateSubscriptions, AnimationManager_StartsTickerOnce_AcrossMultipleAdds) with fix:Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".
📁 Fix files reverted (2 files)
src/Core/src/Animations/PlatformTicker.Windows.cssrc/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
UI Tests
Full UI test matrix will run (no specific categories detected from PR changes).
Pre-Flight — Context & Validation
Issue: #35840 - Windows PlatformTicker IsRunning/duplicate Rendering subscription inferred from local PR branch
PR: #35840 - PlatformTicker Windows running-state fix
Platforms Affected: windows
Files Changed: 2 implementation, 2 test/API metadata
Key Findings
- GitHub CLI is unauthenticated and web fetch is blocked, so issue/PR narrative and comments could not be retrieved; context is derived from the squashed PR commit and local gate artifacts.
- PR changes Windows
PlatformTickersoIsRunningreflects CompositionTarget subscription state andStart()/Stop()are idempotent. - Gate artifact is an environment error, not a behavioral result: Windows PlatformTicker tests failed to run both without and with fix due to
Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32". - Code review found no clear correctness error in the core fix, but identified PublicAPI BOM hygiene and a timing-sensitive device-test assertion.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 2 | Suggestions: 0
Key code review findings:
⚠️ src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt: line 1 contains a BOM before#nullable enable.⚠️ src/Core/tests/DeviceTests/Animations/PlatformTickerTests.Windows.cs: fixed-delay lower boundAssert.InRange(firesAfterStop, 1, 20)may flake when Windows composition frames are throttled.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #35840 | Add _isRunning boolean to Windows PlatformTicker, override IsRunning, and guard Start()/Stop() |
❌ Gate failed with ENV ERROR | src/Core/src/Animations/PlatformTicker.Windows.cs, src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt, src/Core/tests/DeviceTests/Animations/PlatformTickerTests.Windows.cs, src/Core/tests/DeviceTests/TestCategory.cs |
Original PR |
Code Review — Deep Analysis
Code Review — PR #35840
Independent Assessment
What this changes: PlatformTicker.Windows.cs adds a bool _isRunning backing field, overrides the inherited IsRunning property to return it, and adds guard clauses to Start()/Stop() to make both idempotent. A new test file PlatformTickerTests.Windows.cs exercises the new behavior. TestCategory.cs gains an Animation string constant. PublicAPI/net-windows/PublicAPI.Unshipped.txt gains the new override entry.
Inferred motivation: The base Ticker.IsRunning returns _timer?.Enabled ?? false, where _timer is a System.Timers.Timer that PlatformTicker.Windows.cs never creates (it uses CompositionTarget.Rendering instead). So IsRunning permanently returned false on Windows, causing AnimationManager.Add() to call Start() on every animation added — accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU every frame.
Is the approach sound? Yes. The pattern mirrors iOS exactly (_link != null). Android delegates to ValueAnimator.IsStarted. For Windows there is no native "is subscribed" query on CompositionTarget.Rendering, so a boolean field is a reasonable approach. The idempotency guards in Start()/Stop() are a natural consequence of tracking state.
Reconciliation with PR Narrative
Author claims: GitHub PR narrative could not be fetched because gh is unauthenticated and the public web fetch was blocked. Local squashed-PR commit and gate artifacts indicate the bug is Windows PlatformTicker.IsRunning always false, causing repeated starts and duplicate CompositionTarget.Rendering subscriptions.
Agreement/disagreement: The local code diff agrees with that inferred root cause.
Prior Review Reconciliation
Prior GitHub review surfaces could not be queried because gh is unauthenticated. No local prior review artifacts with ❌ Error findings were present under CustomAgentLogsTmp/PRState/35840/PRAgent/.
Blast Radius Assessment
- Runs for all instances: No — the changed subscription path activates when Windows animations start.
- Startup impact: No known startup impact;
PlatformTickeris created for animation management. - Static/shared state: No — PR fix adds instance state only.
CI Status
- Required-check result: undetermined locally because
gh pr checksrequires authentication. - Gate artifact result:
⚠️ ENV ERROR for WindowsPlatformTickerTestsboth without and with fix:Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32". - Classification: environment/tooling blocker for gate verification; do not treat as passing.
- Action taken: Continue with partial local try-fix exploration per autonomous-execution instructions.
Findings
⚠️ Warning — PublicAPI.Unshipped.txt contains a BOM
src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt line 1 has a leading UTF-8 BOM (#nullable enable) introduced by the PR. Repository PublicAPI files normally use plain #nullable enable.
⚠️ Warning — Timing-sensitive lower bound in device test
src/Core/tests/DeviceTests/Animations/PlatformTickerTests.Windows.cs asserts Assert.InRange(firesAfterStop, 1, 20) after a fixed 100 ms delay. The upper bound tests duplicate-subscription risk; the lower bound can flake if the compositor is throttled or no render frame occurs during the delay.
Failure-Mode Probing
- Multiple
AnimationManager.Add()calls while ticker is already running: PR fix prevents repeatStart()and duplicate render subscriptions. - Double
Stop(): PR fix makes it a no-op; event unsubscription is also harmless without it. Start()followed byStop()state consistency: PR fix tracks state explicitly.- Public API analyzer: new override needs a matching PublicAPI entry; the entry exists but the file encoding should be normalized.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: The PR fix appears technically sound from local code inspection, but GitHub PR context and required-check status were unavailable, and the existing gate artifact reports an environment error rather than a valid pass/fail signal. PublicAPI file hygiene and timing-sensitive test assertions should be considered before merge.
Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Delegate/sentinel subscription state (_renderingHandler != null) | ❌ FAIL / env-script error after successful build | 2 files | Avoids boolean drift but is more complex than PR fix. Runner failed with Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32". |
| 2 | try-fix-2 | Normalize event list by -= before += on every Start() | ❌ FAIL / env-script error after successful build | 2 files | More defensive against duplicate subscriptions but does unnecessary event-list churn. Same runner failure. |
| 3 | try-fix-3 | Remove Windows render-event override and inherit base Ticker timer | ❌ FAIL / env-script error after successful build; design rejected | 1 file | Not viable: changes Windows animation ticks to System.Timers.Timer, creates PublicAPI warnings for shipped overrides, and violates UI-thread/render-frame semantics. |
| PR | PR #35840 | Add _isRunning boolean, override IsRunning, guard Windows Start()/Stop() | ❌ Gate failed with ENV ERROR | 2 fix files + tests | Still the best local candidate by simplicity and platform semantics, but not empirically validated due gate/test-runner environment error. |
Cross-Pollination
| Model/Reviewer | Round | New Ideas? | Details |
|---|---|---|---|
| code-review + MAUI expert rules | 1 | Yes | Candidate 1: use the render-event delegate itself as the running-state sentinel. |
| code-review + MAUI expert rules | 1 | Yes | Candidate 2: normalize event subscription with remove-before-add instead of early-return guarding. |
| code-review + MAUI expert rules | 1 | Yes | Candidate 3: use shared base Ticker; rejected by expert self-review and PublicAPI warnings. |
| code-review + MAUI expert rules | 2 | No | Remaining plausible fixes collapse to the same root approach as the PR: preserve CompositionTarget.Rendering and add per-instance running/subscription state. Variations beyond candidates 1-2 are trivial state-token reshapes, not meaningfully different approaches. |
Exhausted: Yes
Selected Fix: PR #35840 — The PR's boolean guard is the simplest CompositionTarget-preserving solution. Candidate 1 is also plausible but not demonstrably better; candidate 2 is more defensive but less minimal; candidate 3 is architecturally worse.
Test Command Used
powershell pwsh .github\skills\run-device-tests\scripts\Run-DeviceTests.ps1 -Project Core -Platform windows -TestFilter "Category=Animation"
All three candidates compiled successfully. All three test runs failed during Windows device-test result summarization with the same environment/tooling error already seen in the gate artifact:
ext Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".
try-fix-1 Narrative
try-fix-1
Approach
Delegate/sentinel subscription state: store the exact CompositionTarget.Rendering delegate in _renderingHandler, use _renderingHandler != null for IsRunning, and unsubscribe/clear that field on Stop().
Difference from PR fix
The PR uses a separate _isRunning boolean. This candidate uses the event subscription delegate itself as the state token, coupling state to what must be unsubscribed.
Diff
`diff
diff --git a/src/Core/src/Animations/PlatformTicker.Windows.cs b/src/Core/src/Animations/PlatformTicker.Windows.cs
index 35dc7ac8e9..12dfb01cb7 100644
--- a/src/Core/src/Animations/PlatformTicker.Windows.cs
+++ b/src/Core/src/Animations/PlatformTicker.Windows.cs
@@ -5,16 +5,30 @@ namespace Microsoft.Maui.Animations
///
public class PlatformTicker : Ticker
{
-
System.EventHandler<object?>? _renderingHandler; -
/// <inheritdoc/> -
public override bool IsRunning => -
_renderingHandler != null; -
/// <inheritdoc/> public override void Start() {
-
CompositionTarget.Rendering += RenderingFrameEventHandler;
-
if (_renderingHandler != null) -
return; -
_renderingHandler = RenderingFrameEventHandler; -
CompositionTarget.Rendering += _renderingHandler; } /// <inheritdoc/> public override void Stop() {
-
CompositionTarget.Rendering -= RenderingFrameEventHandler;
-
if (_renderingHandler == null) -
return; -
CompositionTarget.Rendering -= _renderingHandler; -
_renderingHandler = null; } void RenderingFrameEventHandler(object? sender, object? args)
diff --git a/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt b/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
index cb86b4077b..15373bdabd 100644
--- a/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
+++ b/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
@@ -1,3 +1,4 @@
#nullable enable
+override Microsoft.Maui.Animations.PlatformTicker.IsRunning.get -> bool
override Microsoft.Maui.Platform.MauiPasswordTextBox.OnCreateAutomationPeer() -> Microsoft.UI.Xaml.Automation.Peers.AutomationPeer!
override Microsoft.Maui.Platform.ContentPanel.OnCreateAutomationPeer() -> Microsoft.UI.Xaml.Automation.Peers.AutomationPeer!
`
Test Results
Result: ❌ Fail / environment-script error
Command:
powershell pwsh .github\skills\run-device-tests\scripts\Run-DeviceTests.ps1 -Project Core -Platform windows -TestFilter "Category=Animation"
The candidate compiled successfully, but the runner failed during Windows result summarization with:
` ext
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Graphics.Win2D -> D:\a\1\s\artifacts\bin\Graphics.Win2D\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.Win2D.WinUI.Desktop.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Core -> D:\a\1\s\artifacts\bin\Core\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.dll
Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Release\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Controls.Xaml -> D:\a\1\s\artifacts\bin\Controls.Xaml\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Xaml.dll
TestUtils.DeviceTests -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.TestUtils.DeviceTests.dll
TestUtils.DeviceTests.Runners -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests.Runners\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.TestUtils.DeviceTests.Runners.dll
Core.DeviceTests.Shared -> D:\a\1\s\artifacts\bin\Core.DeviceTests.Shared\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.DeviceTests.Shared.dll
TestUtils.DeviceTests.Runners.SourceGen -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests.Runners.SourceGen\Release\netstandard2.0\Microsoft.Maui.TestUtils.DeviceTests.Runners.SourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
Core.DeviceTests -> D:\a\1\s\artifacts\bin\Core.DeviceTests\Release\net10.0-windows10.0.19041.0\win-x64\Microsoft.Maui.Core.DeviceTests.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:03:05.29
✓ Build succeeded
✓ App found: D:\a\1\s\artifacts\bin\Core.DeviceTests\Release\net10.0-windows10.0.19041.0\win-x64\Microsoft.Maui.Core.DeviceTests.exe
═══════════════════════════════════════════════════════════
Running Tests
═══════════════════════════════════════════════════════════
Running Windows device test app directly...
This matches eng/devices/windows.cake and avoids VSTest/testhost for MAUI Windows device apps.
�[33;1mWARNING: Windows non-Controls device tests do not support dynamic category filtering; running the full Core device test app.�[0m
Running Windows device test app directly...
Get-WindowsDeviceTestResultSummary: D:\a\1\s.github\skills\run-device-tests\scripts\Run-DeviceTests.ps1:350
Line |
350 | … $summary = Get-WindowsDeviceTestResultSummary -ResultFiles $resultFi …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".
`
Failure Analysis
The failure matches the prior gate environment error (Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".), occurring after a successful build when summarizing Windows device-test results. This does not prove the candidate behavior failed. Learned input for next candidate: prefer alternatives that remain easy to reason about statically because local test execution cannot currently produce a reliable pass/fail result.
Expert Self-Review
eviewer-findings.json contains [] (0 findings). The candidate is Windows-only, keeps the CompositionTarget event path, and adds the required PublicAPI entry.
try-fix-2 Narrative
try-fix-2
Approach
Normalize-before-subscribe: Start() removes RenderingFrameEventHandler before adding it, so repeated starts leave at most one render subscription. IsRunning is backed by _renderingSubscribed.
Difference from PR fix
The PR avoids repeated work via an early-return guard. This candidate actively normalizes the WinUI event subscription every time Start() is called, making duplicate subscriptions impossible even if prior state drifted.
Diff
`diff
diff --git a/src/Core/src/Animations/PlatformTicker.Windows.cs b/src/Core/src/Animations/PlatformTicker.Windows.cs
index 35dc7ac8e9..209316ad6e 100644
--- a/src/Core/src/Animations/PlatformTicker.Windows.cs
+++ b/src/Core/src/Animations/PlatformTicker.Windows.cs
@@ -5,16 +5,28 @@ namespace Microsoft.Maui.Animations
///
public class PlatformTicker : Ticker
{
-
bool _renderingSubscribed; -
/// <inheritdoc/> -
public override bool IsRunning => -
_renderingSubscribed; -
/// <inheritdoc/> public override void Start() { -
CompositionTarget.Rendering -= RenderingFrameEventHandler; CompositionTarget.Rendering += RenderingFrameEventHandler; -
_renderingSubscribed = true; } /// <inheritdoc/> public override void Stop() { -
if (!_renderingSubscribed) -
return; -
CompositionTarget.Rendering -= RenderingFrameEventHandler; -
_renderingSubscribed = false; } void RenderingFrameEventHandler(object? sender, object? args)
diff --git a/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt b/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
index cb86b4077b..15373bdabd 100644
--- a/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
+++ b/src/Core/src/PublicAPI/net-windows/PublicAPI.Unshipped.txt
@@ -1,3 +1,4 @@
#nullable enable
+override Microsoft.Maui.Animations.PlatformTicker.IsRunning.get -> bool
override Microsoft.Maui.Platform.MauiPasswordTextBox.OnCreateAutomationPeer() -> Microsoft.UI.Xaml.Automation.Peers.AutomationPeer!
override Microsoft.Maui.Platform.ContentPanel.OnCreateAutomationPeer() -> Microsoft.UI.Xaml.Automation.Peers.AutomationPeer!
`
Test Results
Result: ❌ Fail / environment-script error
Command:
powershell pwsh .github\skills\run-device-tests\scripts\Run-DeviceTests.ps1 -Project Core -Platform windows -TestFilter "Category=Animation"
The candidate compiled successfully, but the runner failed during Windows result summarization with:
` ext
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Essentials -> D:\a\1\s\artifacts\bin\Essentials\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Core -> D:\a\1\s\artifacts\bin\Core\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.dll
Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Release\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
TestUtils.DeviceTests -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.TestUtils.DeviceTests.dll
Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Controls.Xaml -> D:\a\1\s\artifacts\bin\Controls.Xaml\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Xaml.dll
TestUtils.DeviceTests.Runners -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests.Runners\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.TestUtils.DeviceTests.Runners.dll
Core.DeviceTests.Shared -> D:\a\1\s\artifacts\bin\Core.DeviceTests.Shared\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.DeviceTests.Shared.dll
TestUtils.DeviceTests.Runners.SourceGen -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests.Runners.SourceGen\Release\netstandard2.0\Microsoft.Maui.TestUtils.DeviceTests.Runners.SourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
Core.DeviceTests -> D:\a\1\s\artifacts\bin\Core.DeviceTests\Release\net10.0-windows10.0.19041.0\win-x64\Microsoft.Maui.Core.DeviceTests.dll
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:02:50.53
✓ Build succeeded
✓ App found: D:\a\1\s\artifacts\bin\Core.DeviceTests\Release\net10.0-windows10.0.19041.0\win-x64\Microsoft.Maui.Core.DeviceTests.exe
═══════════════════════════════════════════════════════════
Running Tests
═══════════════════════════════════════════════════════════
Running Windows device test app directly...
This matches eng/devices/windows.cake and avoids VSTest/testhost for MAUI Windows device apps.
�[33;1mWARNING: Windows non-Controls device tests do not support dynamic category filtering; running the full Core device test app.�[0m
Running Windows device test app directly...
Get-WindowsDeviceTestResultSummary: D:\a\1\s.github\skills\run-device-tests\scripts\Run-DeviceTests.ps1:350
Line |
350 | … $summary = Get-WindowsDeviceTestResultSummary -ResultFiles $resultFi …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".
`
Failure Analysis
The failure is identical to candidate 1 and the gate artifact. It occurs after successful compilation, when the runner summarizes Windows device-test result files. This does not distinguish candidate 2 from candidate 1 behaviorally. Expert-review lesson for next candidate: because executable validation is blocked, candidate selection must weigh abstraction correctness and runtime semantics more heavily than local test output.
Expert Self-Review
eviewer-findings.json contains [] (0 findings). The normalize-before-subscribe design is safe but less minimal than the PR fix because repeated Start() still touches the event list.
try-fix-3 Narrative
try-fix-3
Approach
Use base timer implementation on Windows: remove the Windows CompositionTarget.Rendering Start()/Stop() overrides so PlatformTicker inherits base Ticker running-state behavior.
Difference from PR fix
The PR preserves the Windows render-event implementation and adds state around it. This candidate changes abstraction entirely by reusing the shared System.Timers.Timer implementation.
Diff
`diff
diff --git a/src/Core/src/Animations/PlatformTicker.Windows.cs b/src/Core/src/Animations/PlatformTicker.Windows.cs
index 35dc7ac8e9..7928b10ade 100644
--- a/src/Core/src/Animations/PlatformTicker.Windows.cs
+++ b/src/Core/src/Animations/PlatformTicker.Windows.cs
@@ -1,25 +1,7 @@
-using Microsoft.UI.Xaml.Media;
namespace Microsoft.Maui.Animations
{
///
public class PlatformTicker : Ticker
{
-
/// <inheritdoc/> -
public override void Start() -
{ -
CompositionTarget.Rendering += RenderingFrameEventHandler; -
} -
/// <inheritdoc/> -
public override void Stop() -
{ -
CompositionTarget.Rendering -= RenderingFrameEventHandler; -
} -
void RenderingFrameEventHandler(object? sender, object? args) -
{ -
Fire?.Invoke(); -
}
}
}
\ No newline at end of file
`
Test Results
Result: ❌ Fail / environment-script error, with design warnings
Command:
powershell pwsh .github\skills\run-device-tests\scripts\Run-DeviceTests.ps1 -Project Core -Platform windows -TestFilter "Category=Animation"
The candidate compiled, but produced PublicAPI warnings for removing shipped overrides and then hit the same Windows result-summary failure:
` ext
Building Core Device Tests for windows
═══════════════════════════════════════════════════════════
Running: dotnet build src/Core/tests/DeviceTests/Core.DeviceTests.csproj -c Release -f net10.0-windows10.0.19041.0 /p:TreatWarningsAsErrors=false /p:RuntimeIdentifierOverride=win-x64 /p:WindowsPackageType=None /p:SelfContained=true /p:_MauiDeviceTestUnpackaged=true /p:UseMonoRuntime=false
Determining projects to restore...
All projects are up-to-date for restore.
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Graphics.Win2D -> D:\a\1\s\artifacts\bin\Graphics.Win2D\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.Win2D.WinUI.Desktop.dll
Essentials -> D:\a\1\s\artifacts\bin\Essentials\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Essentials.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
D:\a\1\s\src\Core\src\PublicAPI\net-windows\PublicAPI.Shipped.txt(2149,1): warning RS0017: Symbol 'override Microsoft.Maui.Animations.PlatformTicker.Start() -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [D:\a\1\s\src\Core\src\Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Core\src\PublicAPI\net-windows\PublicAPI.Shipped.txt(2150,1): warning RS0017: Symbol 'override Microsoft.Maui.Animations.PlatformTicker.Stop() -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [D:\a\1\s\src\Core\src\Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
Core -> D:\a\1\s\artifacts\bin\Core\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.dll
Controls.BindingSourceGen -> D:\a\1\s\artifacts\bin\Controls.BindingSourceGen\Release\netstandard2.0\Microsoft.Maui.Controls.BindingSourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Controls.Core -> D:\a\1\s\artifacts\bin\Controls.Core\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Controls.Xaml -> D:\a\1\s\artifacts\bin\Controls.Xaml\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Controls.Xaml.dll
TestUtils.DeviceTests -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.TestUtils.DeviceTests.dll
TestUtils.DeviceTests.Runners -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests.Runners\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.TestUtils.DeviceTests.Runners.dll
Core.DeviceTests.Shared -> D:\a\1\s\artifacts\bin\Core.DeviceTests.Shared\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.DeviceTests.Shared.dll
TestUtils.DeviceTests.Runners.SourceGen -> D:\a\1\s\artifacts\bin\TestUtils.DeviceTests.Runners.SourceGen\Release\netstandard2.0\Microsoft.Maui.TestUtils.DeviceTests.Runners.SourceGen.dll
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
##vso[build.updatebuildnumber]10.0.90-ci+azdo.14422400
Graphics -> D:\a\1\s\artifacts\bin\Graphics\Release\net10.0-windows10.0.19041.0\Microsoft.Maui.Graphics.dll
Core.DeviceTests -> D:\a\1\s\artifacts\bin\Core.DeviceTests\Release\net10.0-windows10.0.19041.0\win-x64\Microsoft.Maui.Core.DeviceTests.dll
Build succeeded.
D:\a\1\s\src\Core\src\PublicAPI\net-windows\PublicAPI.Shipped.txt(2149,1): warning RS0017: Symbol 'override Microsoft.Maui.Animations.PlatformTicker.Start() -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [D:\a\1\s\src\Core\src\Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
D:\a\1\s\src\Core\src\PublicAPI\net-windows\PublicAPI.Shipped.txt(2150,1): warning RS0017: Symbol 'override Microsoft.Maui.Animations.PlatformTicker.Stop() -> void' is part of the declared API, but is either not public or could not be found (https://github.com/dotnet/roslyn/blob/main/src/RoslynAnalyzers/PublicApiAnalyzers/PublicApiAnalyzers.Help.md) [D:\a\1\s\src\Core\src\Core.csproj::TargetFramework=net10.0-windows10.0.19041.0]
2 Warning(s)
0 Error(s)
Time Elapsed 00:02:51.15
✓ Build succeeded
✓ App found: D:\a\1\s\artifacts\bin\Core.DeviceTests\Release\net10.0-windows10.0.19041.0\win-x64\Microsoft.Maui.Core.DeviceTests.exe
═══════════════════════════════════════════════════════════
Running Tests
═══════════════════════════════════════════════════════════
Running Windows device test app directly...
This matches eng/devices/windows.cake and avoids VSTest/testhost for MAUI Windows device apps.
�[33;1mWARNING: Windows non-Controls device tests do not support dynamic category filtering; running the full Core device test app.�[0m
Running Windows device test app directly...
Get-WindowsDeviceTestResultSummary: D:\a\1\s.github\skills\run-device-tests\scripts\Run-DeviceTests.ps1:350
Line |
350 | … $summary = Get-WindowsDeviceTestResultSummary -ResultFiles $resultFi …
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32".
`
Failure Analysis
This candidate is not viable even before considering the result-summary blocker. Removing the Windows overrides makes shipped API entries for PlatformTicker.Start() and Stop() unresolved, and expert self-review flagged a major Windows threading/rendering semantic regression: animation ticks would move from CompositionTarget.Rendering to a System.Timers.Timer callback. The repeated runner error still prevents behavioral test results.
Expert Self-Review
eviewer-findings.json contains 1 major finding: base timer fallback violates Windows UI-thread/render-frame semantics.
Report — Final Recommendation
Comparative Report — PR #35840
Candidate ranking
| Rank | Candidate | Regression result | Assessment |
|---|---|---|---|
| 1 | pr-plus-reviewer |
Best candidate. Preserves the PR's correct Windows CompositionTarget.Rendering design, fixes IsRunning/duplicate subscription behavior, and applies the expert reviewer's major feedback by tightening the regression test so repeated Start() calls are compared against a single-start baseline. |
|
| 2 | pr |
Core implementation is sound and minimal, but the expert reviewer found the new regression test does not actually prove duplicate subscriptions are prevented because the current upper bound can still pass with roughly three subscriptions. | |
| 3 | try-fix-1 |
Also sound: uses the event delegate as the state token. Slightly more complex than the PR because it stores a delegate field, and it does not include the PR's test coverage improvements in the saved diff. | |
| 4 | try-fix-2 |
Functionally plausible and normalizes subscriptions by removing before adding, but less efficient/minimal because every Start() touches the event list even when already running. |
|
| 5 | try-fix-3 |
Not viable. Removing Windows overrides causes shipped PublicAPI warnings for Start()/Stop() and changes Windows animation ticks from CompositionTarget.Rendering to the base System.Timers.Timer, which is a semantic/threading regression. |
No candidate has a confirmed regression-test pass because the Windows runner failed during result summarization with Cannot convert the "System.Object[]" value of type "System.Object[]" to type "System.Int32". The failures are therefore ranked by compile/design evidence and static review, not by a successful behavioral gate. Candidates that failed real regression tests would rank below passing candidates, but this run produced environment errors rather than behavioral failures.
Comparative analysis
The raw PR identifies the right root cause: Windows PlatformTicker overrides Start() and Stop() to use CompositionTarget.Rendering, but did not override IsRunning, so callers saw the base timer state instead of the render-event subscription state. Adding a private running flag and guarding Start()/Stop() directly addresses that mismatch and prevents duplicate event subscriptions.
The expert reviewer found that the PR's regression test needs strengthening. Assert.InRange(firesAfterStop, 1, 20) can still pass with approximately three subscriptions over the 100ms sampling window, so the test does not independently prove idempotent Start() behavior. The pr-plus-reviewer candidate keeps the PR's implementation and tightens the test by comparing the repeated-start path to a single-start baseline.
try-fix-1 is the closest alternative. Using the stored event delegate as the subscription sentinel tightly couples state to the unsubscribe token, which is a reasonable design. However, the current PR's boolean state mirrors the existing Tizen pattern, is simpler, and is adequate because Start() and Stop() are expected to be called on the UI thread for the WinUI render event.
try-fix-2 is safe but less clean. Removing before adding makes duplicate subscriptions impossible even if state drifted, but it performs event-list mutation on every Start() call and still requires a state flag for IsRunning. There is no evidence the extra normalization buys meaningful resilience over an idempotent early return.
try-fix-3 should be rejected. It compiles only with PublicAPI warnings and changes the Windows ticker from UI render-frame callbacks to a timer-based callback path, which risks threading and frame-synchronization regressions.
Winning candidate
Winner: pr-plus-reviewer
This is the strongest candidate because it keeps the PR's correct, minimal platform fix while applying the expert reviewer's test-strengthening feedback. It is still PR-based, so no non-PR candidate diff is required for the winner manifest.
Future Action — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
…lse (#35840) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - On Windows, Ticker.IsRunning always returns false, even while an animation is actively running. This causes AnimationManager.Add() to call Ticker.Start() on every animation added, accumulating duplicate CompositionTarget.Rendering subscriptions and wasting CPU per frame. ### Root Cause of the issue - The base Ticker.IsRunning reads _timer?.Enabled, where _timer is a System.Timers.Timer used by the managed-fallback implementation. - The Windows PlatformTicker deliberately doesn't use that timer — it overrides Start()/Stop() to subscribe to CompositionTarget.Rendering instead (the correct frame-aligned mechanism on WinUI). But it only completed half the override contract: it swapped the mechanism without overriding IsRunning (or tracking its own running state). So IsRunning keeps reading a _timer field that is permanently null on this platform → always returns false. - iOS overrides it as _link != null. Android overrides it as _val.IsStarted. Windows simply ignored. ### Description of Change **PlatformTicker improvements:** * Added a private `_isRunning` field and a public `IsRunning` property to explicitly track whether the ticker is active, ensuring accurate state reporting. * Modified `Start` and `Stop` methods to be idempotent—calling `Start` multiple times or `Stop` when already stopped has no adverse effect, preventing duplicate event subscriptions or removals. **Testing:** * Introduced new tests in `PlatformTickerTests.Windows.cs` to verify the correct behavior of `IsRunning`, the idempotency of `Start`, and integration with `AnimationManager`. * Added a new test category constant `Animation` in `TestCategory.cs` for organizing animation-related tests. **Public API:** * Updated `PublicAPI.Unshipped.txt` to include the new `IsRunning` property on `PlatformTicker`. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #35839 ### Tested the behaviour in the following platforms - [x] - Windows - [ ] - Android - [ ] - iOS - [ ] - Mac | Before | After | |----------|----------| | <img src="https://github.com/user-attachments/assets/5e5b4ec8-b948-4f9e-8827-6839ac017319"> | <img src="https://github.com/user-attachments/assets/d94d3f6a-24bb-4172-a39a-73f793ca2d50"> | <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> --------- Co-authored-by: Jakub Florkowski <42434498+kubaflo@users.noreply.github.com>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details
Root Cause of the issue
Description of Change
PlatformTicker improvements:
_isRunningfield and a publicIsRunningproperty to explicitly track whether the ticker is active, ensuring accurate state reporting.StartandStopmethods to be idempotent—callingStartmultiple times orStopwhen already stopped has no adverse effect, preventing duplicate event subscriptions or removals.Testing:
PlatformTickerTests.Windows.csto verify the correct behavior ofIsRunning, the idempotency ofStart, and integration withAnimationManager.AnimationinTestCategory.csfor organizing animation-related tests.Public API:
PublicAPI.Unshipped.txtto include the newIsRunningproperty onPlatformTicker.Issues Fixed
Fixes #35839
Tested the behaviour in the following platforms