Skip to content

Commit 4e44eab

Browse files
[Agent] perf(ui): SwiftUI performance audit — reduce scroll stutter (#3480)
* perf(ui): SwiftUI performance audit — reduce scroll stutter and Realm overhead * perf(ui): eliminate per-frame re-renders and lazy-collection misuse - ConsoleGamesView: ForEach now uses Results<PVGame> directly instead of .toArray().filter{} — avoids O(n) array allocation every render - CustomPageIndicator: remove GeometryReader wrapper; use .frame(maxWidth:.infinity) instead to avoid per-frame layout pass - HomeContinueSection: cache PVSaveState footer lookup in @State; only refresh on page/focus change, not every body re-render - HomeView: move scroll offset tracking into a reference-type ScrollTracker so previousOffset mutations never trigger HomeView body re-evaluation; isSearchBarVisible written only when it changes - GameItemView: cache discCount (relatedFiles.toArray()) on onAppear instead of re-computing on every body evaluation Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): address Copilot review — bounded concurrency and search task cleanup - Cap concurrent Realm reloads to 4 in refreshFromRealmChanges() to prevent thread/memory spikes on large libraries - Apply same bounded concurrency cap to loadAllGames() startup load - Cancel searchDebounceTask on .onSubmit to avoid redundant post-submit query; also cancel on .onDisappear to prevent stale background tasks - Trigger loadGamesIfNeeded for all systems in TVMediaSystemsView.task so systems not yet loaded appear in the grid without depending on the lazy-load side effect of shelf rows Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(ui): address Copilot review — scroll perf & state safety - ConsoleGamesView: use absoluteString for view identity to avoid Optional(...) in the .id() string - TVMediaMainView: build ordered refresh list (selected system first), add defer to reset isLoading on cancellation, update stale comment in systemsWithGames - HomeContinueSection: centre indicators without ScrollView when they fit; only use ScrollView when pages > maxVisibleIndicators Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Copilot review — stable view IDs, ScrollTracker, disc count - ConsoleGamesView: use trueArtworkURL?.absoluteString instead of optional string interpolation (both PVGame call sites) - TVMediaMainView: task(id:) now uses full system identifiers list, not just count, to catch same-count replacements/reorders - HomeView: drop ObservableObject from ScrollTracker; hold via @State to eliminate unused subscription overhead - GameItemView: add onChange(of: relatedFiles.count) to keep cachedDiscCount in sync when files are imported while cell is visible - changelog: reflect all fixes accurately Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Copilot round-3 review — optional URL IDs and actor clarity - Fix remaining 4 ConsoleGamesView call sites that still used \`\(game.trueArtworkURL)\` (Optional interpolation); all cell .id modifiers now use \`trueArtworkURL?.absoluteString ?? ""\` - Clarify thread-safety in loadGamesForSystemAsync: remove redundant \`await MainActor.run\` (fn is already @mainactor) and add doc comment explaining actor isolation for concurrent TaskGroup callers - Update changelog to reflect all call sites fixed and actor change Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address Copilot review — icon reactivity, focus, disc-count dedup - TVMediaSystemsView: make icon-load task reactive to systemsWithGames changes (previously ran once on appear when cache may be empty) - TVMediaSystemsView: assign focusedSystemID via onChange when first systems appear, not only onAppear when list may still be empty - GameItemView: extract refreshCachedDiscCount() to eliminate duplicated disc-count computation between onAppear and onChange handlers Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use [String] identity for icon-load task to avoid join collisions .map(\.identifier).joined() can produce the same string for different identifier arrays (e.g. ["ab","c"] == ["a","bc"]). [String] is Hashable/Equatable and correctly distinguishes all arrays. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: use defer to reset isLoading on task cancellation Ensures TVMediaSystemDetailView's loading state resets even when the .task is cancelled (e.g. view disappears before loadContent finishes). Consistent with the defer pattern already used in loadAllGames. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent f477d54 commit 4e44eab

7 files changed

Lines changed: 360 additions & 181 deletions

File tree

.changelog/3478.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
### Fixed
2+
- **TVMediaMainView scroll performance**`refreshFromRealmChanges()` now only reloads games for the selected and already-loaded systems (instead of all systems sequentially), reducing Realm queries on each write from O(n) to O(loaded).
3+
- **TVMediaHomeView startup time**`loadAllGames()` now loads system game lists concurrently (via `TaskGroup`) instead of one system at a time, speeding up the home shelf render.
4+
- **Search debounce**`TVMediaSearchView` now debounces the search text `onChange` by 300ms so Realm is not queried on every keystroke.
5+
- **Icon-load scroll stutter**`TVMediaSystemShelfRow` and `TVMediaSystemHeader` no longer hold a shared `@ObservedObject` on `SystemIconLoader`; icon rendering is delegated to a new `TVMediaSystemIconView` that scopes redraws to the icon only, preventing full-shelf invalidation when unrelated system icons load.
6+
- **Stable view identity in game grids**`ConsoleGamesView` grid and list cells now use `"\(id)_\(trueArtworkURL)"` as their SwiftUI identity instead of the full model `hashValue`, so only artwork changes force view recreation rather than every Realm write touching playCount/lastPlayed.
7+
- **ForEach ID stability in recent searches**`TVMediaSearchView` recent-search chips now use the search string as their ForEach ID instead of array offset, preventing unnecessary redraws on search history reordering.
8+
- **Live Realm fallback removed from hot path**`systemsWithGames` computed property no longer calls `system.games.count` (a live Realm query) as a fallback, using only the pre-loaded snapshot cache instead.
9+
- **Lazy Results in ConsoleGamesView**`showGamesGrid` and `showGamesList` with `Results<PVGame>` now pass the Realm `Results` directly to `ForEach` instead of calling `.toArray().filter {}` on every render, eliminating full-library array allocation on each scroll frame.
10+
- **GeometryReader removed from CustomPageIndicator** — page dot indicator no longer wraps its content in a `GeometryReader` to measure container width; replaced with `.frame(maxWidth: .infinity)` centering, removing per-frame layout measurement during paging.
11+
- **Cached footer save-state lookup**`HomeContinueSection` now caches the current `PVSaveState` in `@State` and only re-resolves it on page/focus changes, avoiding a live Realm query in every body render.
12+
- **Scroll frame re-renders eliminated**`HomeView` scroll-offset tracking moved to a reference-type `ScrollTracker` so that `previousOffset` mutations no longer trigger `HomeView` body re-evaluation on every pixel of scroll; `isSearchBarVisible` is now only written when its value actually changes.
13+
- **discCount cached in GameItemView** — multi-disc indicator count is computed once on `onAppear` instead of calling `relatedFiles.toArray()` on every body re-render of each visible game cell.

.changelog/3480.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
### Changed
2+
- **SwiftUI scroll performance** — Reduced scroll stutter: all `ConsoleGamesView` cell identities now consistently use `trueArtworkURL?.absoluteString ?? ""` instead of string-interpolating an optional (which produced `Optional(...)` strings and caused spurious view recreation), `TVMediaSystemsView` task identity uses the full system-identifiers list instead of count alone (fixes missed updates on same-count replacements), `TVMediaLibraryModel` ordered refresh prioritises the visible system, all loading task blocks use `defer { isLoading = false }` to guarantee loading state resets on task cancellation, and `CustomPageIndicator` centres indicators without a `ScrollView` when all fit in the viewport.
3+
- **ScrollTracker** — Removed unnecessary `ObservableObject` conformance; `ScrollTracker` is now a plain reference type held via `@State`, eliminating subscription overhead while keeping the same no-redraw semantics.
4+
- **Disc indicator**`GameItemView` now refreshes `cachedDiscCount` via `onChange(of: game.relatedFiles.count)` so the multi-disc badge stays accurate if files are added while the cell is on-screen; disc-count computation extracted into `refreshCachedDiscCount()` to eliminate duplication between `onAppear` and `onChange`.
5+
- **Actor isolation**`loadGamesForSystemAsync` is explicitly `@MainActor` with Realm reads isolated in a detached task; mutations to `gamesBySystemIdentifier` are direct assignments on the main actor (no redundant `MainActor.run` wrapper).
6+
- **Icon loading reactivity**`TVMediaSystemsView` icon-load task now uses `systemsWithGames` identifiers as its `id` so icons are requested for systems that become visible after the initial load (previously the task ran once on appear when the cache might still be empty).
7+
- **Initial focus assignment**`TVMediaSystemsView` now also assigns `focusedSystemID` in `onChange(of: systemsWithGames.count)` so focus is set once the first systems appear, even if the cache was empty during `.onAppear`.

PVUI/Sources/PVSwiftUI/Consoles/ConsoleGamesView.swift

Lines changed: 73 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@ struct ConsoleGamesView: SwiftUI.View {
885885
}
886886
}
887887
/// Use compound ID so view recreates when artwork URL changes
888-
.id("\(game.id)_\(game.trueArtworkURL)")
888+
.id("\(game.id)_\(game.trueArtworkURL?.absoluteString ?? "")")
889889
.focusableIfAvailable()
890890
.contextMenu {
891891
GameContextMenu(
@@ -924,8 +924,11 @@ struct ConsoleGamesView: SwiftUI.View {
924924
) {
925925
launchGame(md5: model.md5)
926926
}
927-
/// Use model hash as ID so view recreates when artwork URL changes
928-
.id(model.hashValue)
927+
/// Recreate the tile only when the artwork URL changes, not on every
928+
/// model field update (playCount, lastPlayed, etc.). Using the full
929+
/// model.hashValue caused unnecessary view identity changes and scroll
930+
/// stutter on Realm writes.
931+
.id("\(model.id)_\(model.trueArtworkURL?.absoluteString ?? "")")
929932
.focusableIfAvailable()
930933
.contextMenu {
931934
if let live = liveGame(for: model) {
@@ -948,44 +951,46 @@ struct ConsoleGamesView: SwiftUI.View {
948951
private func showGamesGrid(_ games: Results<PVGame>) -> some View {
949952
ScrollViewReader { proxy in
950953
LazyVGrid(columns: columns, spacing: 10) {
951-
// Custom styling for grid items
952-
ForEach(games.toArray().filter { !$0.isInvalidated }, id: \.id) { game in
953-
GameItemView(
954-
game: game,
955-
constrainHeight: false,
956-
viewType: .cell,
957-
sectionContext: .allGames,
958-
isFocused: Binding(
959-
get: {
960-
!game.isInvalidated &&
961-
gamesViewModel.focusedSection == .allGames &&
962-
gamesViewModel.focusedItemInSection == game.id
963-
},
964-
set: {
965-
if $0 && !game.isInvalidated {
966-
gamesViewModel.focusedSection = .allGames
967-
gamesViewModel.focusedItemInSection = game.id
954+
// Use Results<PVGame> directly (lazy) — avoid .toArray() which materialises all games
955+
ForEach(games, id: \.id) { game in
956+
if !game.isInvalidated {
957+
GameItemView(
958+
game: game,
959+
constrainHeight: false,
960+
viewType: .cell,
961+
sectionContext: .allGames,
962+
isFocused: Binding(
963+
get: {
964+
!game.isInvalidated &&
965+
gamesViewModel.focusedSection == .allGames &&
966+
gamesViewModel.focusedItemInSection == game.id
967+
},
968+
set: {
969+
if $0 && !game.isInvalidated {
970+
gamesViewModel.focusedSection = .allGames
971+
gamesViewModel.focusedItemInSection = game.id
972+
}
968973
}
974+
)
975+
) {
976+
Task.detached { @MainActor in
977+
SceneCoordinator.shared.launchGame(game.freeze())
969978
}
970-
)
971-
) {
972-
Task.detached { @MainActor in
973-
SceneCoordinator.shared.launchGame(game.freeze())
974979
}
975-
}
976-
/// Use compound ID so view recreates when artwork URL changes
977-
.id("\(game.id)_\(game.trueArtworkURL)")
978-
.focusableIfAvailable()
979-
.contextMenu {
980-
GameContextMenu(
981-
game: game,
982-
rootDelegate: rootDelegate,
983-
contextMenuDelegate: self
984-
)
985-
}
980+
/// Use compound ID so view recreates when artwork URL changes
981+
.id("\(game.id)_\(game.trueArtworkURL?.absoluteString ?? "")")
982+
.focusableIfAvailable()
983+
.contextMenu {
984+
GameContextMenu(
985+
game: game,
986+
rootDelegate: rootDelegate,
987+
contextMenuDelegate: self
988+
)
989+
}
986990
#if !os(tvOS) && !os(watchOS)
987-
.onDrag { game.romDragProvider() }
991+
.onDrag { game.romDragProvider() }
988992
#endif
993+
}
989994
}
990995
}
991996
}
@@ -1019,7 +1024,7 @@ struct ConsoleGamesView: SwiftUI.View {
10191024
}
10201025
}
10211026
/// Use compound ID so view recreates when artwork URL changes
1022-
.id("\(game.id)_\(game.trueArtworkURL)")
1027+
.id("\(game.id)_\(game.trueArtworkURL?.absoluteString ?? "")")
10231028
.focusableIfAvailable()
10241029
.contextMenu {
10251030
GameContextMenu(
@@ -1060,8 +1065,11 @@ struct ConsoleGamesView: SwiftUI.View {
10601065
) {
10611066
launchGame(md5: model.md5)
10621067
}
1063-
/// Use model hash as ID so view recreates when artwork URL changes
1064-
.id(model.hashValue)
1068+
/// Recreate the tile only when the artwork URL changes, not on every
1069+
/// model field update (playCount, lastPlayed, etc.). Using the full
1070+
/// model.hashValue caused unnecessary view identity changes and scroll
1071+
/// stutter on Realm writes.
1072+
.id("\(model.id)_\(model.trueArtworkURL?.absoluteString ?? "")")
10651073
.focusableIfAvailable()
10661074
.contextMenu {
10671075
if let live = liveGame(for: model) {
@@ -1083,29 +1091,32 @@ struct ConsoleGamesView: SwiftUI.View {
10831091
@ViewBuilder
10841092
private func showGamesList(_ games: Results<PVGame>) -> some View {
10851093
LazyVStack(spacing: 8) {
1086-
ForEach(games.toArray().filter { !$0.isInvalidated }, id: \.id) { game in
1087-
GameItemView(
1088-
game: game,
1089-
constrainHeight: false,
1090-
viewType: .row,
1091-
sectionContext: .allGames,
1092-
isFocused: Binding(
1093-
get: {
1094-
!game.isInvalidated &&
1095-
gamesViewModel.focusedSection == .allGames &&
1096-
gamesViewModel.focusedItemInSection == game.id },
1097-
set: { if $0 && !game.isInvalidated { gamesViewModel.focusedItemInSection = game.id} }
1098-
))
1099-
{
1100-
loadGame(game)
1101-
}
1102-
/// Use compound ID so view recreates when artwork URL changes
1103-
.id("\(game.id)_\(game.trueArtworkURL)")
1104-
.focusableIfAvailable()
1105-
.contextMenu { GameContextMenu(game: game, rootDelegate: rootDelegate, contextMenuDelegate: self) }
1094+
// Use Results<PVGame> directly (lazy) — avoid .toArray() which materialises all games
1095+
ForEach(games, id: \.id) { game in
1096+
if !game.isInvalidated {
1097+
GameItemView(
1098+
game: game,
1099+
constrainHeight: false,
1100+
viewType: .row,
1101+
sectionContext: .allGames,
1102+
isFocused: Binding(
1103+
get: {
1104+
!game.isInvalidated &&
1105+
gamesViewModel.focusedSection == .allGames &&
1106+
gamesViewModel.focusedItemInSection == game.id },
1107+
set: { if $0 && !game.isInvalidated { gamesViewModel.focusedItemInSection = game.id} }
1108+
))
1109+
{
1110+
loadGame(game)
1111+
}
1112+
/// Use compound ID so view recreates when artwork URL changes
1113+
.id("\(game.id)_\(game.trueArtworkURL?.absoluteString ?? "")")
1114+
.focusableIfAvailable()
1115+
.contextMenu { GameContextMenu(game: game, rootDelegate: rootDelegate, contextMenuDelegate: self) }
11061116
#if !os(tvOS) && !os(watchOS)
1107-
.onDrag { game.romDragProvider() }
1117+
.onDrag { game.romDragProvider() }
11081118
#endif
1119+
}
11091120
}
11101121
}
11111122
}
@@ -1222,8 +1233,7 @@ struct ConsoleGamesView: SwiftUI.View {
12221233
) {
12231234
launchGame(md5: game.md5)
12241235
}
1225-
/// Use model hash as ID so view recreates when artwork URL changes
1226-
.id(game.hashValue)
1236+
.id("\(game.id)_\(game.trueArtworkURL?.absoluteString ?? "")")
12271237
.focusableIfAvailable()
12281238
.contextMenu {
12291239
if let live = liveGame(for: game) {
@@ -1598,8 +1608,7 @@ extension ConsoleGamesView {
15981608
) {
15991609
launchGame(md5: game.md5)
16001610
}
1601-
/// Use model hash as ID so view recreates when artwork URL changes
1602-
.id(game.hashValue)
1611+
.id("\(game.id)_\(game.trueArtworkURL?.absoluteString ?? "")")
16031612
.focusableIfAvailable()
16041613
.contextMenu {
16051614
if let live = liveGame(for: game) {

0 commit comments

Comments
 (0)