From 6ec6478c8fb80790d43e090779649141d079f54f Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 14 Oct 2026 17:14:52 -0400 Subject: [PATCH 1/3] Map colour mappers made from the same colours are equal Replays rebuild every frame, and since #304 they make the map's colour mappers inside that per-frame build. A mapper had no equality, so each frame's looked new to flutter_svg, which reloaded and redrew both maps (the map and its revealed copy) every frame and lost the engine's cached raster of them. Replay playback went from 4.8 to 8.2 ms of raster a frame (144 Hz budget: 6.9 ms); with equality it is back to 5.0 ms. Co-Authored-By: Claude Opus 5.5 --- lib/widgets/map_svg_color_mapper.dart | 15 +++++++++++ test/map_svg_color_mapper_test.dart | 38 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 test/map_svg_color_mapper_test.dart diff --git a/lib/widgets/map_svg_color_mapper.dart b/lib/widgets/map_svg_color_mapper.dart index 50dda7e6..fcb865bd 100644 --- a/lib/widgets/map_svg_color_mapper.dart +++ b/lib/widgets/map_svg_color_mapper.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart' show mapEquals; import 'package:flutter/widgets.dart'; import 'package:flutter_svg/flutter_svg.dart'; import 'package:icarus/providers/user_preferences_provider.dart'; @@ -48,4 +49,18 @@ class MapSvgColorMapper extends ColorMapper { final alpha = (color.a * 255.0).round().clamp(0, 255); return replacement.withAlpha(alpha); } + + // The SVG loader and its picture cache compare mappers, so a mapper made + // again with the same colours, as views do on every build (a replay builds + // every frame), must equal the last one. If it didn't, the map would be + // loaded and drawn again from scratch. + @override + bool operator ==(Object other) => + other is MapSvgColorMapper && mapEquals(other.replacements, replacements); + + @override + int get hashCode => Object.hashAllUnordered([ + for (final MapEntry(:key, :value) in replacements.entries) + Object.hash(key, value), + ]); } diff --git a/test/map_svg_color_mapper_test.dart b/test/map_svg_color_mapper_test.dart new file mode 100644 index 00000000..7b0a527d --- /dev/null +++ b/test/map_svg_color_mapper_test.dart @@ -0,0 +1,38 @@ +import 'package:flutter_svg/flutter_svg.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:icarus/providers/user_preferences_provider.dart'; +import 'package:icarus/widgets/map_svg_color_mapper.dart'; + +void main() { + final palette = MapThemePalette( + baseColorValue: 0xFF271406, + detailColorValue: 0xFFB27C40, + highlightColorValue: 0xFFF08234, + ); + + // A map widget rebuilt with a mapper made again from the same palette must + // keep its loaded picture: flutter_svg reloads whenever the loader changes, + // and the loader compares mappers. + test('a mapper made again from the same palette loads the same picture', () { + SvgAssetLoader loader(MapSvgColorMapper mapper) => + SvgAssetLoader('assets/maps/ascent_map.svg', colorMapper: mapper); + + for (final make in [ + MapSvgColorMapper.forPalette, + MapSvgColorMapper.revealed, + ]) { + final again = make(palette.copyWith()); + expect(again, make(palette)); + expect(again.hashCode, make(palette).hashCode); + expect(loader(again), loader(make(palette))); + } + }); + + test('mappers for different colours differ', () { + final other = palette.copyWith(baseColorValue: 0xFF102030); + expect(MapSvgColorMapper.forPalette(other), + isNot(MapSvgColorMapper.forPalette(palette))); + expect(MapSvgColorMapper.revealed(palette), + isNot(MapSvgColorMapper.forPalette(palette))); + }); +} From b2b40e37c5fabd59cf5709d796322e92242e7d31 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 14 Oct 2026 17:17:27 -0400 Subject: [PATCH 2/3] Mapper test: name what it checks; listing order must not matter Co-Authored-By: Claude Opus 5.5 --- test/map_svg_color_mapper_test.dart | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/test/map_svg_color_mapper_test.dart b/test/map_svg_color_mapper_test.dart index 7b0a527d..5a309e2e 100644 --- a/test/map_svg_color_mapper_test.dart +++ b/test/map_svg_color_mapper_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/painting.dart'; import 'package:flutter_svg/flutter_svg.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:icarus/providers/user_preferences_provider.dart'; @@ -13,7 +14,7 @@ void main() { // A map widget rebuilt with a mapper made again from the same palette must // keep its loaded picture: flutter_svg reloads whenever the loader changes, // and the loader compares mappers. - test('a mapper made again from the same palette loads the same picture', () { + test('a mapper made again from the same palette gives an equal loader', () { SvgAssetLoader loader(MapSvgColorMapper mapper) => SvgAssetLoader('assets/maps/ascent_map.svg', colorMapper: mapper); @@ -28,6 +29,14 @@ void main() { } }); + test('the order colours were listed in does not matter', () { + const red = Color(0xFFFF0000), blue = Color(0xFF0000FF); + const one = MapSvgColorMapper({1: red, 2: blue}); + const other = MapSvgColorMapper({2: blue, 1: red}); + expect(other, one); + expect(other.hashCode, one.hashCode); + }); + test('mappers for different colours differ', () { final other = palette.copyWith(baseColorValue: 0xFF102030); expect(MapSvgColorMapper.forPalette(other), From ab755167be1ec8c2a24fd0fc92d48282d1f85d45 Mon Sep 17 00:00:00 2001 From: Dara Adedeji Date: Wed, 14 Oct 2026 17:25:51 -0400 Subject: [PATCH 3/3] Mapper colours are unmodifiable, since mappers compare by them Co-Authored-By: Claude Opus 5.5 --- lib/widgets/map_svg_color_mapper.dart | 12 +++++++----- test/map_svg_color_mapper_test.dart | 4 ++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/widgets/map_svg_color_mapper.dart b/lib/widgets/map_svg_color_mapper.dart index fcb865bd..bf3f2405 100644 --- a/lib/widgets/map_svg_color_mapper.dart +++ b/lib/widgets/map_svg_color_mapper.dart @@ -13,24 +13,26 @@ class MapSvgSourceColors { } class MapSvgColorMapper extends ColorMapper { - const MapSvgColorMapper(this.replacements); + MapSvgColorMapper(Map replacements) + : replacements = Map.unmodifiable(replacements); MapSvgColorMapper.forPalette(MapThemePalette palette) - : replacements = { + : replacements = Map.unmodifiable({ MapSvgSourceColors.base.toARGB32(): palette.baseColor, MapSvgSourceColors.detail.toARGB32(): palette.detailColor, MapSvgSourceColors.highlight.toARGB32(): palette.highlightColor, - }; + }); /// The map in [palette] as cones reveal it: the same art with its floor /// in [revealedFloorColor] (see `lib/view_cone/cone_coverage.dart`). MapSvgColorMapper.revealed(MapThemePalette palette) - : replacements = { + : replacements = Map.unmodifiable({ MapSvgSourceColors.base.toARGB32(): revealedFloorColor(palette), MapSvgSourceColors.detail.toARGB32(): palette.detailColor, MapSvgSourceColors.highlight.toARGB32(): palette.highlightColor, - }; + }); + /// Unmodifiable: mappers are compared by these, so they never change. final Map replacements; @override diff --git a/test/map_svg_color_mapper_test.dart b/test/map_svg_color_mapper_test.dart index 5a309e2e..cd5684d7 100644 --- a/test/map_svg_color_mapper_test.dart +++ b/test/map_svg_color_mapper_test.dart @@ -31,8 +31,8 @@ void main() { test('the order colours were listed in does not matter', () { const red = Color(0xFFFF0000), blue = Color(0xFF0000FF); - const one = MapSvgColorMapper({1: red, 2: blue}); - const other = MapSvgColorMapper({2: blue, 1: red}); + final one = MapSvgColorMapper(const {1: red, 2: blue}); + final other = MapSvgColorMapper(const {2: blue, 1: red}); expect(other, one); expect(other.hashCode, one.hashCode); });