feat: waved (shader-based) flags #454
Conversation
- Set kotlin jvm target to version 24. - Upgrade gradle wrapper to version 9.2.0.
- Added FLTEnableImpeller key to Info.plist for macOS support. - Updated Dart SDK version to ^3.10.4 in pubspec.yaml. - Added shader configuration for waved_flag.frag.
- Introduced ClippedTrianglePainter to handle Bahrain's flag design. - Adjusted flag properties in blr_painter and multi_element_painter.
This commit introduces a new shader file for the waved flag effect, including various uniforms and functions for wave computation, noise generation, and lighting effects.
- Introduced FlagShaderOptions for customizable shader effects. - Added FlagShaderDelegate for post-processing flag content. - Updated BasicFlag, IsoFlag, and related classes to support shader options.
- Added FlagShaderDelegate to BasicFlag and related extensions. - Removed shaderOptions from BasicFlag and its subclasses. - Updated relevant constructors and documentation.
- Introduced FlagShaderDelegate to IsoFlag and BasicFlag classes. - Updated constructors to accept shader parameters. - Adjusted imports and documentation for clarity.
- Removed shader-related properties and references from BasicFlag, IsoFlag, and related extensions to streamline flag rendering. - Updated imports to reflect changes in shader handling.
- Introduced WavedFlagShaderDelegate for applying waved-flag shader. - Updated typedefs to include FlagShaderDelegateBuilder. - Refactored StripesPainter to improve code structure and readability.
- Refactored paintWithShader method to enhance performance by caching shader size and image. - Introduced ShaderStripesPainter for efficient flag rendering with shader effects. - Updated main.dart to simplify FlagThemeData initialization. - Adjusted imports in country_flag.dart to remove unused mappings.
- Added a dispose method to release resources in ShaderStripesPainter. - Enhanced documentation for paintStripes and applyFlagClipping methods.
Updated the alternateSymbols and disambiguateSymbol for the Argentine Peso to use "AR$" instead of previous values.
β¦ement - Replaced uniqueSimplifiedFlagsMap with smallSimplifiedFlagsMap in _items. - Added disposal of _paint.shader in WavedFlagShaderDelegate. - Added @optionalTypeArgs annotation to IsoFlag class. - Updated ShaderStripesPainter to use shader directly instead of _shader.
This commit introduces the FlagShaderSurface widget, which provides an animated, shader-driven flag display with support for real-time shader animations and customizable visual parameters.
β¦ons for skia - Updated fragment shader to include unrolled fbm2 and fbm3 functions for SkSL compatibility, improving performance and compatibility. - Adjusted comments for clarity regarding cotton drift and fabric functions.
β¦egates - Introduced tests for FlagShaderOptions to validate default and custom values. - Added tests for WavedFlagShaderDelegate to ensure correct behavior. - Created tests for ShaderStripesPainter to verify painting logic and properties. - Included tests for FlagShaderSurface to check rendering and widget behavior.
- Changed default overflowScale from 0.92 to 0.9 in FlagShaderOptions. - Added new golden tests for waved flags in flag_shader_surface_test.dart. - Introduced new flag type 'waved' in flag_type.dart. - Added corresponding golden images for waved flags.
β¦overage - Updated FlagShaderDelegate and ShaderStripesPainter for better null safety checks. - Adjusted formatting for Dart 3.8 compliance. - Added tests for painter reuse and shader functionality.
Updated the formatting of the testWidgets function parameters for Dart 3.8 compliance.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
π WalkthroughWalkthroughAdds shader-driven flag rendering: new FlagShaderDelegate/FlagShaderOptions, FlagShaderSurface widget, WavedFlagShaderDelegate and GLSL shader; painter pipeline changes (ShaderStripesPainter, StripesPainter refactors); Iso/CountryFlag shader plumbing and helpers; tests and example updates; minor data/config tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App/Widget
participant FSS as FlagShaderSurface
participant Delegate as FlagShaderDelegate\n(WavedFlagShaderDelegate)
participant Painter as ShaderStripesPainter
participant Canvas as Canvas
App->>FSS: build(item, options, optional shader)
FSS->>FSS: resolve flag from map
alt external shader provided
FSS->>Delegate: use provided delegate
else
FSS->>Delegate: create/reuse default WavedFlagShaderDelegate(vsync)
Delegate->>Delegate: load shader program & start ticker
end
FSS->>Painter: create/update ShaderStripesPainter(shader, properties)
Painter->>Painter: render flag to offscreen Image (cache)
Delegate->>Delegate: update time/uniforms on tick
Painter->>Delegate: delegate.paintWithShader(canvas, size, image)
Delegate->>Canvas: apply shader and draw (or return false)
alt shader returns false
Painter->>Canvas: draw cached image fallback
end
App->>FSS: dispose or replace shader
FSS->>Painter: dispose -> release image
FSS->>Delegate: dispose if owned
Estimated code review effortπ― 4 (Complex) | β±οΈ ~75 minutes Possibly related PRs
π₯ Pre-merge checks | β 3β Passed checks (3 passed)
βοΈ Tip: You can configure your own custom pre-merge checks in the settings. β¨ Finishing touchesπ§ͺ Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
DCM report for sealed_currenciesβ no issues found! Full report: https://github.com/tsinis/sealed_world/runs/59781412251 |
DCM report for world_flagsβ no issues found! Full report: https://github.com/tsinis/sealed_world/runs/59781537867 |
DCM report for world_countriesβ no issues found! Full report: https://github.com/tsinis/sealed_world/runs/59781612272 |
coderabbitai
bot
added
chore
dependencies
Codecov Reportβ
All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #454 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 238 164 -74
Lines 24047 22593 -1454
===========================================
- Hits 24047 22593 -1454
Flags with carried forward coverage won't be shown. Click here to find out more. β View full report in Codecov by Sentry. π New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and canβt be posted inline due to platform limitations.
β οΈ Outside diff range comments (1)
packages/world_flags/example/android/app/build.gradle.kts (1)
16-17: Change Java version from 24 to 17 for Android compatibility.
JavaVersion.VERSION_24is incompatible with the Android Gradle Plugin (AGP) 8.x used with Dart 3.10.4. AGP 8.x requires JDK 17. Java 24 causes build incompatibilities and will likely fail during compilation. UseJavaVersion.VERSION_17instead for bothsourceCompatibilityandtargetCompatibility, and update the KotlinjvmTargettoJVM_17.
π€ Fix all issues with AI agents
In @packages/world_countries/example/macos/Runner/Info.plist:
- Around line 31-32: The <true /> element following the
<key>FLTEnableImpeller</key> entry is indented with spaces (or mixed whitespace)
while the rest of the plist uses tabs; replace the leading spaces before the
<true /> line with the same tab characters used elsewhere so the <true /> tag
aligns with the fileβs tab-based indentation convention.
In @packages/world_flags/example/android/app/build.gradle.kts:
- Line 1: Remove the unused import of TimeUnit from the top of the file by
deleting the line "import java.util.concurrent.TimeUnit" so there are no unused
imports left; ensure no other code references TimeUnit and run a build or linter
to confirm the unused import warning is resolved.
In @packages/world_flags/example/lib/main.dart:
- Line 9: Replace the terse constructor call used in the example to make intent
clearer: change the BoxDecoration's borderRadius argument from the shorthand
".all(.circular(4))" to the explicit BorderRadius.all(Radius.circular(4)) form
so readers see the BorderRadius and Radius types; update the FlagThemeData(...)
invocation where BoxDecoration(borderRadius: .all(.circular(4))) is used to use
BoxDecoration(borderRadius: BorderRadius.all(Radius.circular(4))).
In @packages/world_flags/example/macos/Runner/Info.plist:
- Around line 31-32: The <key>FLTEnableImpeller</key> block has inconsistent
indentation on the <true /> line (mix of spaces and tabs); update that <true />
element to use the same tab characters as the surrounding lines so the file uses
tabs consistently, ensuring the <key>FLTEnableImpeller</key> and its <true />
child are aligned with the other plist entries.
In @packages/world_flags/lib/src/helpers/extensions/aspect_ratio_extension.dart:
- Around line 6-7: The extension method aspectRatio(double? width) contains a
redundant null-coalescing operator (?? 1) because the preceding null-check (this
== null) ensures that when the division runs, this is non-null; remove the
unreachable ?? 1 and replace the expression with width == null || this == null ?
null : width / this to simplify and clarify intent, updating the aspectRatio
function accordingly.
In
@packages/world_flags/lib/src/helpers/extensions/decorated_flag_interface_extension.dart:
- Line 2: Add the new import "aspect_ratio_extension.dart" and update the aspect
ratio getter in DecoratedFlagInterfaceExtension to delegate to
height.aspectRatio(width) instead of duplicating the calculation; ensure the
getter signature remains unchanged, keep the import present, and apply the same
refactor to the other occurrence noted (line 8) so both places use
height.aspectRatio(width) for consistency.
In @packages/world_flags/lib/src/ui/effects/flag_shader_options.dart:
- Line 38: Update the comment in flag_shader_options.dart to use American
spelling: replace "artefacts" with "artifacts" in the documentation line
referencing [waveAmplitude] so the comment reads "...increasing [waveAmplitude]
to minimize clipping artifacts." This is just a textual change in the comment
near the flag shader options documentation.
- Line 14: Update the doc comment that reads "They do *not* manage shader
lifecycles" to use the spelled form required by CIβreplace "lifecycles" with
"life cycles" or "life-cycles" in the comment associated with FlagShaderSurface
(search for the doc comment referencing [FlagShaderSurface] in
flag_shader_options.dart) so the spelling check passes; alternatively, add
"lifecycles" to the cspell dictionary if the single-word form is intentional.
In @packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart:
- Around line 187-196: In dispose(), avoid disposing the same FragmentShader
twice by handling _paint.shader and _shader safely: first null out _paint.shader
if it references _shader (e.g., if (_paint.shader == _shader) _paint.shader =
null), or alternatively dispose _paint.shader only when it's non-null and !==
_shader, then dispose _shader and set both references to null; ensure you still
dispose _ticker and call super.dispose().
In @packages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dart:
- Around line 6-10: The file currently imports Listenable four times from
different Flutter packages; remove the redundant imports and keep a single
import that exposes Listenable (for example, retain import
"package:flutter/foundation.dart" show Listenable or import
"package:flutter/widgets.dart" show Listenable) while leaving the other needed
imports (like "package:flutter/rendering.dart") intact; update the top import
block so only one show Listenable remains and remove the other three duplicate
Listenable imports.
In
@packages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dart:
- Around line 20-21: The doc comment for the _leftToRightTriangle method is
inaccurate (mentions "Czechia like triangle") and should be updated to match the
class purpose (Bahrain's serrated seam); change the comment above
_leftToRightTriangle to describe that it paints the Bahrain-style
serrated/zigzag triangle seam (left-to-right) on the flag and ensure wording
aligns with the class documentation describing Bahrain's serrated seam.
In @packages/world_flags/lib/src/ui/painters/multi_element_painter.dart:
- Line 76: The paint uses canvas.clipRect(Offset.zero & size) without
saving/restoring canvas state, so wrap the clip with canvas.save() before and
canvas.restore() after to avoid leaking the clip to subsequent painting; locate
the clip call in the MultiElementPainter.paint method (or the class paint
implementation) and insert canvas.save() just before canvas.clipRect(...) and
canvas.restore() immediately after the clipped drawing operations.
In @packages/world_flags/shaders/waved_flag.frag:
- Line 42: The LIGHT vector constant is not normalized (magnitude ~0.95), which
can bias diffuse lighting; update the declaration of LIGHT so it is a unit
vector by normalizing its components (e.g., replace the raw vec3 with a
normalized version or compute normalize(vec3(...))) and use that normalized
LIGHT in your dot(N, LIGHT) lighting calculations to ensure consistent
intensity.
In @packages/world_flags/test/helpers/flag_type.dart:
- Line 6: The test helper uses shorthand cascade constructors in the decoration
argument (simplified(decoration: BoxDecoration(borderRadius:
.all(.circular(4)))),) which reduces clarity; change the shorthand to explicit
class constructors by replacing .all(.circular(4)) with
BorderRadius.all(Radius.circular(4)) inside the BoxDecoration so the call reads
simplified(decoration: BoxDecoration(borderRadius:
BorderRadius.all(Radius.circular(4)))), improving readability in the test
helper.
In @packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart:
- Line 1: Update the file-level ignore comment so it references the actual test
helper name: replace the incorrect "wavedFlagGolden method" text with
"flagGolden method" (or simply remove the mistaken phrase) so the
ignore_for_file directive accurately matches the existing helper function name
`flagGolden` and avoids confusion.
π Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
β Files ignored due to path filters (7)
packages/world_flags/test/goldens/simplified/blr.pngis excluded by!**/*.pngpackages/world_flags/test/goldens/simplified/isl.pngis excluded by!**/*.pngpackages/world_flags/test/goldens/simplified/mhl.pngis excluded by!**/*.pngpackages/world_flags/test/goldens/waved/kor.pngis excluded by!**/*.pngpackages/world_flags/test/goldens/waved/mkd.pngis excluded by!**/*.pngpackages/world_flags/test/goldens/waved/shn.pngis excluded by!**/*.pngpackages/world_flags/test/goldens/waved/svk.pngis excluded by!**/*.png
π Files selected for processing (33)
packages/sealed_currencies/lib/src/data/currencies/ars.data.dartpackages/world_countries/example/macos/Runner/Info.plistpackages/world_flags/example/android/app/build.gradle.ktspackages/world_flags/example/android/gradle/wrapper/gradle-wrapper.propertiespackages/world_flags/example/lib/main.dartpackages/world_flags/example/lib/settings_dialog.dartpackages/world_flags/example/macos/Runner/Info.plistpackages/world_flags/example/pubspec.yamlpackages/world_flags/lib/src/data/flags_map_part_3.data.dartpackages/world_flags/lib/src/helpers/extensions/aspect_ratio_extension.dartpackages/world_flags/lib/src/helpers/extensions/decorated_flag_interface_extension.dartpackages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dartpackages/world_flags/lib/src/model/typedefs.dartpackages/world_flags/lib/src/ui/country_flag.dartpackages/world_flags/lib/src/ui/effects/flag_shader_delegate.dartpackages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/lib/src/ui/effects/flag_shader_surface.dartpackages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dartpackages/world_flags/lib/src/ui/iso_flag.dartpackages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dartpackages/world_flags/lib/src/ui/painters/basic/stripes_painter.dartpackages/world_flags/lib/src/ui/painters/custom/blr_painter.dartpackages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dartpackages/world_flags/lib/src/ui/painters/multi_element_painter.dartpackages/world_flags/lib/world_flags.dartpackages/world_flags/pubspec.yamlpackages/world_flags/shaders/waved_flag.fragpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dartpackages/world_flags/test/helpers/flag_type.dartpackages/world_flags/test/src/ui/effects/flag_shader_options_test.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dartpackages/world_flags/test/src/ui/painters/basic/shader_stripes_painter_test.dart
π§° Additional context used
π§ Learnings (23)
π Common learnings
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart:86-91
Timestamp: 2025-06-29T20:31:42.528Z
Learning: In flag stripe rendering (packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart), diagonal lines should be antialiased to avoid the "stairs" effect, while vertical and horizontal lines should not use anti-aliasing as they align perfectly with the pixel grid and don't produce jagged artifacts.
π Learning: 2025-06-29T20:31:42.528Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart:86-91
Timestamp: 2025-06-29T20:31:42.528Z
Learning: In flag stripe rendering (packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart), diagonal lines should be antialiased to avoid the "stairs" effect, while vertical and horizontal lines should not use anti-aliasing as they align perfectly with the pixel grid and don't produce jagged artifacts.
Applied to files:
packages/world_flags/lib/src/ui/painters/multi_element_painter.dartpackages/world_flags/lib/src/ui/effects/flag_shader_delegate.dartpackages/world_flags/lib/src/ui/iso_flag.dartpackages/world_flags/lib/src/data/flags_map_part_3.data.dartpackages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/example/lib/main.dartpackages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dartpackages/world_flags/lib/src/ui/painters/custom/blr_painter.dartpackages/world_flags/test/src/ui/painters/basic/shader_stripes_painter_test.dartpackages/world_flags/test/helpers/flag_type.dartpackages/world_flags/lib/src/ui/effects/flag_shader_surface.dartpackages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dartpackages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dartpackages/world_flags/test/src/ui/effects/flag_shader_options_test.dartpackages/world_flags/lib/src/model/typedefs.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dartpackages/world_flags/lib/src/helpers/extensions/decorated_flag_interface_extension.dartpackages/world_flags/lib/src/ui/painters/basic/stripes_painter.dartpackages/world_flags/example/lib/settings_dialog.dartpackages/world_flags/lib/src/ui/country_flag.dartpackages/world_flags/lib/world_flags.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dartpackages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart
π Learning: 2025-09-21T20:39:13.298Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 403
File: packages/world_countries/example/lib/ui/settings/widgets/sliders/settings_tile_slider.dart:46-55
Timestamp: 2025-09-21T20:39:13.298Z
Learning: In this project, SDK constraints are managed through aligned Dart SDK constraints rather than explicit Flutter environment constraints in pubspec.yaml files.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yaml
π Learning: 2024-10-15T20:38:35.889Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 239
File: packages/world_countries/example/README.md:15-15
Timestamp: 2024-10-15T20:38:35.889Z
Learning: When the Flutter SDK version is updated to a master version (e.g., `3.27+`) in the `README.md` file (`packages/world_countries/example/README.md`), do not update the `pubspec.yaml` files until the Flutter version becomes stable.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yamlpackages/world_flags/example/lib/settings_dialog.dart
π Learning: 2025-02-08T20:22:33.205Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 297
File: packages/sealed_currencies/CHANGELOG.md:1-1
Timestamp: 2025-02-08T20:22:33.205Z
Learning: Dart package changelogs follow a specific format for pub.dev where version headers (## X.Y.Z) start directly without a top-level heading. This is the standard format and should not be modified to include a top-level header.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yamlpackages/world_flags/lib/world_flags.dart
π Learning: 2025-06-22T19:00:24.453Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 353
File: packages/world_flags/CHANGELOG.md:0-0
Timestamp: 2025-06-22T19:00:24.453Z
Learning: In the sealed_world project, all CHANGELOG.md files (including packages/world_flags/CHANGELOG.md, packages/sealed_countries/CHANGELOG.md, etc.) follow pub.dev and official Dart guidelines for changelog formatting, which do not require a top-level heading. The format starts directly with version headings (e.g., "## 2.5.0") and should not be suggested to add top-level headings like "# Changelog".
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yamlpackages/world_flags/lib/src/ui/iso_flag.dartpackages/world_flags/lib/src/data/flags_map_part_3.data.dartpackages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/example/lib/main.dartpackages/world_flags/test/helpers/flag_type.dartpackages/world_flags/example/lib/settings_dialog.dartpackages/world_flags/lib/src/ui/country_flag.dartpackages/world_flags/lib/world_flags.dartpackages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart
π Learning: 2025-02-09T11:35:57.297Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 299
File: packages/world_flags/CHANGELOG.md:1-1
Timestamp: 2025-02-09T11:35:57.297Z
Learning: The changelog.md files in Dart/Flutter packages should follow the pub.dev style guide, which uses second-level headings (##) for version numbers at the start of the file, not top-level headings (#).
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yaml
π Learning: 2025-02-09T14:53:01.406Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 300
File: packages/world_countries/CHANGELOG.md:1-3
Timestamp: 2025-02-09T14:53:01.406Z
Learning: For pub.dev packages, changelogs should follow the pub.dev convention starting directly with version headers (##) without a top-level heading (#). This is the standard format expected by pub.dev and overrides general markdown linting rules.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yaml
π Learning: 2024-10-15T19:40:10.377Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 237
File: packages/sealed_countries/CHANGELOG.md:1-1
Timestamp: 2024-10-15T19:40:10.377Z
Learning: In this project, changelog files like `packages/sealed_countries/CHANGELOG.md` follow the official Dart guidelines, which do not require a top-level heading.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yamlpackages/world_flags/lib/src/data/flags_map_part_3.data.dartpackages/world_flags/example/lib/main.dartpackages/world_flags/example/lib/settings_dialog.dartpackages/world_flags/lib/src/ui/country_flag.dartpackages/world_flags/lib/world_flags.dart
π Learning: 2024-10-15T20:01:03.291Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 238
File: packages/world_flags/pubspec.yaml:31-31
Timestamp: 2024-10-15T20:01:03.291Z
Learning: In this project, the `pubspec.yaml` files already include source comments for dependencies, such as `# From Google` for `flutter_test` and `lints`.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/example/pubspec.yaml
π Learning: 2025-01-23T22:23:47.517Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 284
File: packages/sealed_countries/test/src/helpers/utils/typed_localization_delegate_test.dart:7-16
Timestamp: 2025-01-23T22:23:47.517Z
Learning: In Dart test files, prefer keeping compile-time constants (`const`) as-is instead of moving them to `setUp` or making them `late`, as they are immutable and benefit from compile-time optimization.
Applied to files:
packages/world_flags/pubspec.yamlpackages/world_flags/test/helpers/flag_type.dartpackages/world_flags/test/src/ui/effects/flag_shader_options_test.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dartpackages/world_flags/lib/src/ui/painters/basic/stripes_painter.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
π Learning: 2025-04-21T22:21:29.565Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 326
File: packages/world_countries/lib/src/extensions/typed_locale_extension.dart:125-141
Timestamp: 2025-04-21T22:21:29.565Z
Learning: In the Flutter SDK, assertions are disabled in release builds, so using assert(false, ...) will only crash the application in debug mode, which can be intentional for development-time error visibility.
Applied to files:
packages/world_flags/pubspec.yaml
π Learning: 2025-10-12T15:33:46.056Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 412
File: packages/world_countries/example/android/app/build.gradle.kts:14-20
Timestamp: 2025-10-12T15:33:46.056Z
Learning: Flutter 3.35.6 officially supports Java versions up to Java 25, as documented in the Flutter SDK Changelog at https://github.com/flutter/flutter/blob/main/CHANGELOG.md#3356. The changelog states: "flutter analyze --suggestions supports versions up to Java 25, Gradle 9 and AGP 9, Kotlin 2.2.20."
Applied to files:
packages/world_flags/example/android/app/build.gradle.kts
π Learning: 2025-06-22T17:32:55.576Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 353
File: packages/world_countries/lib/src/widgets/country/country_picker.dart:126-0
Timestamp: 2025-06-22T17:32:55.576Z
Learning: In the CountryPicker widget (packages/world_countries/lib/src/widgets/country/country_picker.dart), the leading parameter is intentionally nullable. When flagsMap[itemProperties.item] returns null (no flag found), no fallback widget should be displayed - this is the intended design behavior.
Applied to files:
packages/world_flags/lib/src/ui/iso_flag.dartpackages/world_flags/lib/src/data/flags_map_part_3.data.dartpackages/world_flags/example/lib/main.dartpackages/world_flags/lib/src/ui/effects/flag_shader_surface.dartpackages/world_flags/lib/src/model/typedefs.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/example/lib/settings_dialog.dartpackages/world_flags/lib/src/ui/country_flag.dartpackages/world_flags/lib/world_flags.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dartpackages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart
π Learning: 2024-10-14T20:46:43.876Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 234
File: packages/world_flags/README.md:28-28
Timestamp: 2024-10-14T20:46:43.876Z
Learning: In the documentation files (e.g., `packages/world_flags/README.md`), the term "Widget" is intentionally capitalized to align with Flutter's conventions and the slogan "Everything is a Widget".
Applied to files:
packages/world_flags/lib/src/ui/iso_flag.dartpackages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/example/lib/main.dartpackages/world_flags/test/helpers/flag_type.dartpackages/world_flags/lib/src/ui/effects/flag_shader_surface.dartpackages/world_flags/lib/src/model/typedefs.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/lib/src/ui/country_flag.dartpackages/world_flags/lib/world_flags.dartpackages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart
π Learning: 2025-06-29T20:33:00.216Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart:40-41
Timestamp: 2025-06-29T20:33:00.216Z
Learning: In Flutter/Dart, copyWith methods should follow the exact naming convention of the constructor parameters, even when this causes parameter shadowing (e.g., using `key` parameter name even though StatelessWidget has a `key` getter). This is the established idiomatic approach and consistency with constructor signatures is preferred over avoiding parameter shadowing.
Applied to files:
packages/world_flags/lib/src/ui/effects/flag_shader_options.dart
π Learning: 2025-02-09T11:33:18.006Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 299
File: packages/world_flags/README.md:131-131
Timestamp: 2025-02-09T11:33:18.006Z
Learning: The class name `CustomPainter` from Flutter SDK should be formatted with backticks, and when pluralized, the 's' should be outside the backticks (e.g., `CustomPainter`s).
Applied to files:
packages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dartpackages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dartpackages/world_flags/lib/world_flags.dart
π Learning: 2025-12-27T10:46:37.583Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 449
File: packages/l10n_currencies/lib/src/data/hr_currencies_l10n.data.dart:11-170
Timestamp: 2025-12-27T10:46:37.583Z
Learning: In the `sealed_world` repository, Croatian (hr) currency names in `packages/l10n_currencies/lib/src/data/hr_currencies_l10n.data.dart` use sentence case (first word capitalized) because they are intended as standalone UI labels, not continuous text. This design choice applies to all currency localization files in the repository.
Applied to files:
packages/sealed_currencies/lib/src/data/currencies/ars.data.dartpackages/world_flags/lib/src/ui/country_flag.dart
π Learning: 2024-10-15T19:40:08.928Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 237
File: packages/sealed_countries/pubspec.yaml:24-24
Timestamp: 2024-10-15T19:40:08.928Z
Learning: `sealed_currencies` version 1.5.1 does not introduce any breaking changes.
Applied to files:
packages/sealed_currencies/lib/src/data/currencies/ars.data.dart
π Learning: 2025-07-13T17:41:39.732Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 366
File: packages/world_countries/test/src/extensions/platform_dispatcher_extension_test.dart:1-167
Timestamp: 2025-07-13T17:41:39.732Z
Learning: In Flutter widget tests using `testWidgets`, the callback function must be marked as `async` even without `await` statements because the function signature requires `Future<void> Function(WidgetTester) callback`. Static analysis tools may incorrectly flag these as unnecessary async functions.
Applied to files:
packages/world_flags/test/src/ui/painters/basic/shader_stripes_painter_test.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dartpackages/world_flags/example/lib/settings_dialog.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
π Learning: 2025-06-21T22:06:05.721Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 353
File: packages/world_countries/lib/src/widgets/language/language_picker.dart:64-157
Timestamp: 2025-06-21T22:06:05.721Z
Learning: Flutter widgets commonly have extensive parameter lists (50+ parameters is normal) as seen in TextField, InputDecoration, Scaffold, and ThemeData. This is the established Flutter convention for providing comprehensive configuration options through constructor parameters rather than using builder patterns or parameter grouping.
Applied to files:
packages/world_flags/lib/src/model/typedefs.dartpackages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart
π Learning: 2025-09-21T19:51:29.309Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 403
File: packages/world_countries/example/lib/helpers/extensions/shadows_flag_theme_controller_extension.dart:11-12
Timestamp: 2025-09-21T19:51:29.309Z
Learning: In Flutter, the Color.opacity getter and Color.withOpacity() method are deprecated. The current recommended approach is to use Color.a for getting alpha values and Color.withValues(alpha: value) for setting alpha values. These newer methods provide better precision and are the preferred APIs.
Applied to files:
packages/world_flags/example/lib/settings_dialog.dart
π Learning: 2025-05-31T20:47:49.261Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 346
File: packages/sealed_countries/test/src/helpers/extensions/iterable_natural_language_extension_test.dart:7-41
Timestamp: 2025-05-31T20:47:49.261Z
Learning: The sealed_countries package maintains 100% test coverage. When suggesting additional tests, verify coverage metrics first as comprehensive coverage may already be achieved through existing test patterns.
Applied to files:
packages/world_flags/lib/src/ui/country_flag.dart
πͺ GitHub Actions: links_and_spell_check
packages/world_flags/lib/src/ui/effects/flag_shader_options.dart
[warning] 14-14: cspell: Unknown word 'lifecycles'.
[warning] 38-38: cspell: Unknown word 'artefacts'.
[error] 1-1: cspell-action: 2 spelling issues found in 1 of the 24 files checked.
πͺ GitHub Check: ππͺ Check Links & Spelling
packages/world_flags/lib/src/ui/effects/flag_shader_options.dart
[warning] 38-38:
Unknown word (artefacts)
[warning] 14-14:
Unknown word (lifecycles)
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: π‘οΈ Verify world_flags / β³ Verify world_flags Package
- GitHub Check: π‘οΈ Verify sealed_currencies / β³ Verify sealed_currencies Package
- GitHub Check: π‘οΈ Verify world_countries / β³ Verify world_countries Package
π Additional comments (61)
packages/world_flags/lib/src/ui/painters/custom/blr_painter.dart (2)
171-171: LGTM! Path coordinates now constrain to canvas bounds.The change from
height * 1.03toheightensures that path vertices remain within the canvas, preventing geometry from extending beyond the visible area. This aligns with the shader-based rendering improvements introduced in this PR.Also applies to: 177-177
185-189: LGTM! Centered rectangle approach improves rendering bounds.The refactor from
Rect.fromLTRB(bounds.left, 0, width * 0.99, height * 1.1)toRect.fromCenter(...)is cleaner and aligns with the shader rendering pipeline. Note that this removes the previous 10% vertical overflow (height * 1.1βheight), making the clipping region match the canvas exactly. The golden tests added in this PR should validate that the visual appearance remains correct.packages/world_flags/example/lib/main.dart (1)
32-32:smallSimplifiedFlagsMapis properly defined and available.
smallSimplifiedFlagsMapis defined as a public constant map atpackages/world_flags/lib/world_flags.dart:558with proper documentation. The map is correctly spread into the_itemsconstant in the example app.packages/world_flags/test/helpers/flag_type.dart (1)
7-7: LGTM! New waved flag type aligns with PR objectives.The addition of the
wavedenum member with a height of 60 properly supports the new shader-based waved flag rendering feature introduced in this PR.packages/sealed_currencies/lib/src/data/currencies/ars.data.dart (1)
23-24: Data correction is appropriate.The update replaces non-standard symbols with
"AR$", which is the recognized convention for disambiguating the Argentine Peso from other dollar currencies. This aligns with standard locale data (CLDR) and common international usage whereAR$is used to distinguish ARS from other "$" currencies. The implementation is correct.packages/world_countries/example/macos/Runner/Info.plist (1)
31-32: Impeller support is confirmed and properly implemented.Impeller fully supports custom GLSL fragment shaders on macOSβthe engine automatically compiles GLSL to Metal at runtime. The
waved_flag.fragshader is already well-engineered for this: it includes Impeller-specific handling (IMPELLER_TARGET_OPENGLESpreprocessor directives), uses unrolled loops (a universal GPU best practice for optimal shader performance, not just a Skia quirk), and follows Flutter'sruntime_effect.glslframework. No changes needed.packages/world_flags/shaders/waved_flag.frag (10)
1-3: LGTM!Standard Flutter fragment shader setup with appropriate GLSL version and high precision floats for consistent rendering across devices.
5-37: LGTM!Uniforms are well-organized and properly typed. Using
floatfor boolean-like values (e.g.,uLeftEdgePinned,uShadingEnabled) with 0.5 threshold comparisons is the correct approach for shader uniforms.
48-62: LGTM!The hash and noise functions are correctly implemented. The smooth interpolation using
f * f * (3.0 - 2.0 * f)is the standard Hermite smoothstep polynomial, and the seed incorporation prevents identical noise patterns across different flag instances.
64-86: LGTM!The unrolled FBM variants are a good solution for SkSL compatibility. The documentation clearly explains the constraint, and having separate
fbm2andfbm3functions allows choosing the appropriate quality/performance tradeoff at each call site.
92-116: LGTM!Wave helper functions are well-implemented. The
waveDir()safely handles near-zero input to prevent division by zero, and the mask functions (edgeRamp,borderMask,hemMask) correctly create smooth edge falloffs for natural flag pinning behavior.
173-184: LGTM!The fabric texture implementation is well-designed. The combination of FBM-based cotton texture with a procedural warp/weft weave pattern creates a realistic cloth appearance. The alternating weft offset (
floor(mod(t.x, 2.0))) correctly simulates plain weave structure.
190-217: LGTM!Wave computation correctly combines primary and secondary waves with appropriate masking. The perpendicular direction calculation for displacement (
vec2(-dir.y, dir.x)) and slope computation using the analytical derivative are mathematically sound.
223-239: LGTM!UV warping is well-implemented. The clamping to
[0.001, 0.999]prevents texture edge sampling artifacts, and the perspective foreshortening is correctly conditional on the left edge being pinned (typical pole-mounted flag scenario).
245-275: LGTM!The lighting model is well-implemented with wrapped diffuse for soft shadows, highlight falloff using
diffΒ², and a silk sheen effect. The documented fix (line 263-264) correctly usesmix(1.0, fullShade, uShadowStrength)to allow completely flat rendering whenuShadowStrengthis 0.
281-313: LGTM!The main function is well-structured with proper platform handling via
IMPELLER_TARGET_OPENGLESfor Y-coordinate flip. The luminance-based fabric protection (preventing white areas from being darkened) is a thoughtful detail, and the premultiplied alpha output is correct for Flutter's compositing pipeline.packages/world_flags/example/android/gradle/wrapper/gradle-wrapper.properties (1)
5-5: Gradle 9.2.0 compatibility with Flutter SDK 3.10.4 requires verification.Flutter SDK 3.10.4 was released in April 2023 and likely includes Android Gradle Plugin 7.x or 8.x, which may not support Gradle 9.2.0 (released October 2025). This version combination could cause build failures. Confirm that the AGP version bundled with Flutter 3.10.4 is compatible with Gradle 9.2.0, or consider updating to a newer Flutter SDK version that includes a compatible AGP.
packages/world_flags/example/android/app/build.gradle.kts (1)
40-44: The Kotlin DSL migration and JVM_24 target are correct.The migration from
kotlinOptionstokotlin { compilerOptions { ... } }follows modern Kotlin Gradle plugin best practices.JvmTarget.JVM_24is a valid constant in Kotlin 2.3.0 (as confirmed by the Kotlin Gradle plugin API), and it properly aligns with the JavaVERSION_24target set incompileOptions.packages/world_flags/example/pubspec.yaml (1)
9-9: SDK constraint aligns with parent package.The Dart SDK constraint ^3.10.4 correctly matches the parent package's constraint at
packages/world_flags/pubspec.yaml. This ensures consistent behavior across the package and its example app.packages/world_flags/lib/src/model/typedefs.dart (1)
3-14: Well-designed public API for shader delegate construction.The
FlagShaderDelegateBuildertypedef provides a clean, flexible interface for building shader delegates. The signature properly encapsulates the required dependencies (vsync for animation, options for configuration, properties for flag data) and is well-documented.packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart (5)
23-28: Constructor refactored to support repainting for shader animations.The addition of the
repaintparameter and removal ofconstenables shader-based subclasses (likeShaderStripesPainter) to trigger repaints on animation frames. This is essential for animated flag effects.
49-51: Clean separation of painting logic for extensibility.Delegating to
applyFlagClippingandpaintStripesprovides clear extension points for subclasses while maintaining the same rendering behavior. This refactoring supports the shader-based painting pipeline introduced in this PR.
53-78: LGTM: Stripe painting logic properly extracted for testing and extension.The
paintStripesmethod is correctly annotated with@protectedand@visibleForTesting, making it accessible to subclasses and test code while maintaining encapsulation.
80-94: Clipping method exposed for subclass customization.Renaming
_applyFlagClippingtoapplyFlagClippingand marking it@protectedallows shader-based painters to customize clipping behavior if needed. The implementation remains correct.
132-132: Verify necessity of additional clipRect in diagonal stripe rendering.Line 132 adds an explicit
clipRectwithin the diagonal stripe drawing logic. SinceapplyFlagClippingis already called at the start ofpaint(line 49), verify whether this additional clip is necessary or if it's defensive given the coordinate transformations.Based on learnings, diagonal stripes use antialiasing while vertical/horizontal don't. Confirm this additional clip doesn't affect the antialiasing behavior for diagonal stripes.
packages/world_flags/example/lib/settings_dialog.dart (5)
8-16: Proper Future handling with @awaitNotRequired annotation.The updated signature returns
Future<void>fromshowDialog, which is correct. The@awaitNotRequiredannotation appropriately signals that callers can invoke this method without awaiting, maintaining backward compatibility.
26-26: Default opacity optimized to showcase package flags.Changing the initial opacity from 0.5 to 1.0 makes sense for the example app, as it showcases the shader-based flag rendering by default while still allowing users to compare with the original bitmap flag via the slider.
73-81: Smart optimization: skip background image when fully opaque.The conditional decoration avoids loading the network image when
opacity == 1, improving performance and reducing bandwidth usage. This is a good UX optimization for the example app.
98-98: Enhanced slider UX with secondary track value.Adding
secondaryTrackValueto display the country's original aspect ratio provides helpful visual feedback, showing users the default value even when they've adjusted the slider.
118-118: Example app updated to showcase shader-based flag rendering.Switching from
CountryFlag.simplifiedtoFlagShaderSurfacedemonstrates the new shader-based rendering feature introduced in this PR. The API usage appears correct with the country and optional aspect ratio parameters.packages/world_flags/pubspec.yaml (2)
37-39: Shader asset correctly configured.The shader file exists at the specified path and contains valid GLSL code with proper Flutter runtime effect integration.
23-23: Remove requirement to verify Dart SDK 3.10.4 for shader feature support.Dart SDK version does not control Flutter's shader support. Shader features (e.g.,
flutter: shaders:configuration, FragmentShader APIs) are provided by the Flutter engine and SDK, not the Dart SDK. The Dart SDK version only needs to be compatible with the Flutter SDK version in use. No Dart SDK-specific verification for shader support is necessary.Likely an incorrect or invalid review comment.
packages/world_flags/lib/src/ui/country_flag.dart (1)
6-6: LGTM! Shader parameter integration looks good.The addition of the
shaderparameter to bothCountryFlag.simplifiedandCountryFlag.customconstructors properly enables shader-based rendering. The changes align with the broader shader infrastructure introduced in this PR (FlagShaderDelegate, FlagShaderOptions, FlagShaderSurface).The removal of the default
smallSimplifiedAlternativeFlagsMapassignment in the simplified constructor (line 42) and the import change (line 6) suggest intentional API surface cleanup.Also applies to: 42-42, 49-49, 81-81
packages/world_flags/test/src/ui/effects/flag_shader_options_test.dart (1)
1-232: Excellent test coverage for FlagShaderOptions.The test suite comprehensively validates all aspects of the FlagShaderOptions data class:
- Default and custom values for all 26+ configuration parameters
copyWithbehavior (no-change, partial-change, full-change)- Equality semantics and hashCode consistency
- String representation
The use of compile-time constants (
const) aligns with Dart best practices and learnings from previous reviews.packages/world_flags/lib/src/data/flags_map_part_3.data.dart (1)
2388-2388: Confirm the intentional use of transparent rectangle in Saint Lucia flag structure.The alpha channel change from
0xffto0x00for the rectangle element is consistent with a broader pattern in the codebaseβat least 4 other flags use similar transparent base colors (0x00xxxxxx). The flagLcaProperties definition maintains the opaque color in ColorsProperties (0xff65cfff) while the rectangle element uses the transparent version (0x0065cfff), suggesting intentional separation between visual colors and structural elements for shader rendering.Verify:
- The flag renders correctly in both shader and non-shader modes
- Golden tests (if applicable) have been updated to reflect this change
packages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dart (2)
12-169: LGTM! Comprehensive test coverage for WavedFlagShaderDelegate.The test suite thoroughly validates:
- Clipping behavior and contentScale calculations
- Paint operations with various size and image conditions
- Resource disposal and ChangeNotifier conformance
- Error handling via custom callbacks
All tests properly manage resource lifecycle with explicit disposal calls.
171-246: Well-designed animation and lifecycle tests.The tests validate:
- Animation timing progression with configurable speed
- Animation pause behavior when
animateis false- Time wrapping after exceeding the 10000 threshold
- Default error handling path
The
_WavedFlagShaderDelegateTestticker provider is appropriately minimal for test purposes.packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart (2)
12-118: Excellent test coverage for FlagShaderSurface widget.The tests validate:
- Rendering behavior (CustomPaint, fallback widgets, custom maps)
- Layout properties (dimensions, aspect ratio)
- Diagnostics and string representations
All assertions are clear and tests follow Flutter widget testing conventions.
120-179: Shader lifecycle and optimization tests are well-designed.The tests validate:
- Delegate swapping when options change
- External shader integration
- Painter reuse optimization (using
identical()for instance equality)The painter reuse test is particularly important for performance validation.
packages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart (2)
18-48: Well-implemented conditional rendering for waved flags.The implementation cleanly handles two rendering paths:
- Shader-based rendering via
FlagShaderSurfacefor waved flags- Traditional rendering via
IsoFlagfor other typesThe 100ms stabilization delay (line 40) is appropriate for shader animation before golden comparison.
12-16: Map consolidation is correct, but manual test verification recommended.The change correctly consolidates flag source mapsβ
smallSimplifiedFlagsMapalready includesuniqueSimplifiedFlagsMapandsmallSimplifiedAlternativeFlagsMap, so spreading it directly avoids redundancy. This matches the same pattern used inflag_shader_surface.dartandmain.dart. While the consolidation is semantically sound, running the golden tests locally is recommended to ensure no visual regressions occur with the updated flag mappings.packages/world_flags/lib/src/ui/iso_flag.dart (1)
10-10: LGTM! Non-breaking shader infrastructure addition.The changes introduce shader support to
IsoFlagwithout breaking existing API:
- Shader parameter is optional (nullable), maintaining backward compatibility
- Field is properly documented and included in diagnostics
- The
@optionalTypeArgsannotation is appropriate for the generic classThe shader field is intentionally stored but not used in
build()- the actual shader rendering occurs throughFlagShaderSurface.Also applies to: 29-29, 53-53, 81-82, 196-203
packages/world_flags/test/src/ui/painters/basic/shader_stripes_painter_test.dart (3)
23-155: Excellent painter lifecycle and resource management tests.The tests thoroughly validate:
- Painter construction with shader delegate integration
- Painting flow with live shader delegates
- Resource disposal with async initialization handling
- Cache invalidation on size changes
The use of
fail()for null checks (line 39) ensures clear test failure messages.
72-125: Well-designed cache invalidation tests.The
shouldRepainttests validate critical optimization behavior:
- Returns
truewhen shader changes (line 89)- Returns
truewhen properties change (line 108)- Returns
falsewhen nothing changes (line 121) - prevents unnecessary repaintsThese tests ensure efficient rendering by verifying correct cache invalidation logic.
157-251: Comprehensive shader options and fallback path tests.The tests validate:
- Clipping behavior via
shouldClipContentflag- Content scaling via
overflowScaleoption- Fallback rendering when shader image is unavailable
- Cached image painting with clipping
The
_ShaderStripesPainterTesttest double is appropriately minimal, implementing only the necessary interface methods for testing.packages/world_flags/lib/src/ui/effects/flag_shader_delegate.dart (1)
9-39: Well-designed delegate abstraction.The abstract
FlagShaderDelegatedefines a clean contract for shader-based rendering with proper lifecycle management (dispose) and animation support viaListenable. The separation of concerns between content scaling, clipping, and shader painting is appropriate.packages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dart (1)
28-35: Triangle path construction looks correct.The path correctly constructs a triangle from the left edge extending to a point on the right. Based on learnings, diagonal lines should be antialiased - verify that
paintCreator()enables anti-aliasing for this painter.packages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart (1)
40-56: LGTM - shader parameter addition follows established patterns.The
shaderparameter is correctly integrated intocopyWithfollowing the existing conventions for optional parameters with null-coalescing fallback.packages/world_flags/lib/src/ui/effects/flag_shader_options.dart (1)
218-279: Well-implemented value semantics.The
hashCodecorrectly decomposesOffsetinto its components for consistent hashing, and the==operator properly compares all fields. The implementations are consistent with each other.packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart (2)
61-86: Good shader program caching strategy.The static
warmUp()method with lazy initialization ensures the shader program is loaded once and reused across all delegate instances. The_initShader()properly awaits the program and notifies listeners when ready.
163-185: Robust shader painting with proper error handling.The
paintWithShadermethod correctly updates size and image uniforms each frame, catches all exceptions, and reports them via the configurableonErrorcallback. Nullifying_paint.shaderafter drawing prevents stale references.packages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dart (2)
44-81: Well-structured image caching with proper lifecycle management.The cache invalidation logic correctly considers size, scale, and clipping changes. The
_rebuildCachemethod properly disposes the previous image before creating a new one, and the synchronoustoImageSyncis appropriate for this use case.
118-135: Correct ownership semantics for resource disposal.The
shouldRepaintcorrectly compares all relevant fields, anddispose()properly manages the owned_imagewhile leaving theshaderlifecycle toFlagShaderSurfaceas documented. This clear ownership model prevents resource leaks.packages/world_flags/lib/world_flags.dart (3)
91-94: Shader components properly exported.The new shader-related components (
FlagShaderDelegate,FlagShaderOptions,FlagShaderSurface,WavedFlagShaderDelegate) are correctly exported, making them part of the public API.
204-207: CountryBhr flag updated to use ClippedTrianglePainter.This change appropriately uses the new
ClippedTrianglePainterfor Bahrain's flag, ensuring shader overflow cannot expose triangles outside the white band as documented in the painter class.
558-569: Cleaner map composition with spread operator.Using
...smallSimplifiedAlternativeFlagsMapis cleaner than duplicating entries. The documentation update for French Guiana's alternative flag is also appropriate.packages/world_flags/lib/src/ui/effects/flag_shader_surface.dart (5)
87-104: Well-designed constructor API.The constructor provides sensible defaults while allowing full customization through optional parameters. The separation of
shader(external) vsdelegateBuilder(factory pattern) gives flexibility for different use cases.
307-328: Build method is well-structured with proper fallback handling.The layout hierarchy (
SizedBoxβAspectRatioβCustomPaint) correctly constrains dimensions while respecting the flag's natural aspect ratio. ThewillChange: widget.options.animatehint is a good optimization for the rasterization cache.
300-305: Correct disposal order.Resources are properly cleaned up with
super.dispose()called last, which is the correct pattern for StatefulWidget lifecycle.
236-258: Delegate resolution and lifecycle management looks correct.The method properly handles the transition between external and internal delegates, disposing resources appropriately.
FlagShaderOptionsalready implements proper equality semantics with a completeoperator==override andhashCodegetter that covers all fields, so the options comparison at line 245 works correctly.
210-218: No action required. ThewarmUp()method is already idempotent with internal state tracking via static variables (_programLoaderand_programuse null-coalescing assignment??=). Multiple calls towarmUp()return immediately after the first load without performing redundant work.Likely an incorrect or invalid review comment.
packages/world_flags/lib/src/helpers/extensions/aspect_ratio_extension.dart
Show resolved
Hide resolved
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart
Outdated
Show resolved
Hide resolved
Updated the alternateSymbols and disambiguateSymbol for FiatCurrency to use "AR$" for consistency with the Argentine Peso representation.
- Adjusted whitespace in Info.plist for consistency. - Corrected spelling of "artifacts" in flag_shader_options.dart. - Simplified import statements in waved_flag_shader_delegate.dart. - Removed unnecessary imports in shader_stripes_painter.dart. - Deleted commented-out code in clipped_triangle_painter.dart. - Cleaned up test file by removing ignored lint rules.
There was a problem hiding this comment.
Actionable comments posted: 2
π€ Fix all issues with AI agents
In @packages/world_flags/lib/src/ui/effects/flag_shader_options.dart:
- Around line 218-246: The hashCode implementation for FlagShaderOptions is
inconsistent with operator==: replace the separate waveDirection.dx and
waveDirection.dy entries in the Object.hashAll list inside the hashCode getter
with the single waveDirection object so it matches the direct waveDirection
comparison used in operator== (update the hashCode getter referencing
waveDirection instead of its dx/dy components).
In @packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart:
- Around line 187-196: The dispose() currently disposes both _shader and
_paint.shader which can double-dispose if _paint.shader still references _shader
during an active paint; change dispose() to dispose only the canonical owner
_shader and the _ticker, avoid calling dispose() on _paint.shader, and instead
set _paint.shader = null (or leave it null if already cleared) so that _shader
is disposed exactly once; keep the comment about image ownership
(ShaderStripesPainter) intact.
π Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
π Files selected for processing (7)
packages/world_countries/example/macos/Runner/Info.plistpackages/world_flags/example/macos/Runner/Info.plistpackages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dartpackages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dartpackages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart
π§° Additional context used
π§ Learnings (12)
π Common learnings
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart:86-91
Timestamp: 2025-06-29T20:31:42.528Z
Learning: In flag stripe rendering (packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart), diagonal lines should be antialiased to avoid the "stairs" effect, while vertical and horizontal lines should not use anti-aliasing as they align perfectly with the pixel grid and don't produce jagged artifacts.
π Learning: 2025-06-29T20:31:42.528Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart:86-91
Timestamp: 2025-06-29T20:31:42.528Z
Learning: In flag stripe rendering (packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart), diagonal lines should be antialiased to avoid the "stairs" effect, while vertical and horizontal lines should not use anti-aliasing as they align perfectly with the pixel grid and don't produce jagged artifacts.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dartpackages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dartpackages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
π Learning: 2025-01-23T22:23:47.517Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 284
File: packages/sealed_countries/test/src/helpers/utils/typed_localization_delegate_test.dart:7-16
Timestamp: 2025-01-23T22:23:47.517Z
Learning: In Dart test files, prefer keeping compile-time constants (`const`) as-is instead of moving them to `setUp` or making them `late`, as they are immutable and benefit from compile-time optimization.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
π Learning: 2025-06-22T17:32:55.576Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 353
File: packages/world_countries/lib/src/widgets/country/country_picker.dart:126-0
Timestamp: 2025-06-22T17:32:55.576Z
Learning: In the CountryPicker widget (packages/world_countries/lib/src/widgets/country/country_picker.dart), the leading parameter is intentionally nullable. When flagsMap[itemProperties.item] returns null (no flag found), no fallback widget should be displayed - this is the intended design behavior.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart
π Learning: 2025-02-08T23:57:46.308Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 298
File: packages/sealed_countries/lib/src/model/country/submodels/idd.dart:23-24
Timestamp: 2025-02-08T23:57:46.308Z
Learning: Assertion messages in Dart should use backticks (`) for code references, not square brackets ([]), as they are runtime strings for debugging and not dartdoc references.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/lib/src/ui/effects/flag_shader_options.dart
π Learning: 2025-07-13T17:41:39.732Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 366
File: packages/world_countries/test/src/extensions/platform_dispatcher_extension_test.dart:1-167
Timestamp: 2025-07-13T17:41:39.732Z
Learning: In Flutter widget tests using `testWidgets`, the callback function must be marked as `async` even without `await` statements because the function signature requires `Future<void> Function(WidgetTester) callback`. Static analysis tools may incorrectly flag these as unnecessary async functions.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
π Learning: 2025-06-29T20:33:00.216Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart:40-41
Timestamp: 2025-06-29T20:33:00.216Z
Learning: In Flutter/Dart, copyWith methods should follow the exact naming convention of the constructor parameters, even when this causes parameter shadowing (e.g., using `key` parameter name even though StatelessWidget has a `key` getter). This is the established idiomatic approach and consistency with constructor signatures is preferred over avoiding parameter shadowing.
Applied to files:
packages/world_flags/lib/src/ui/effects/flag_shader_options.dart
π Learning: 2025-02-09T11:33:18.006Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 299
File: packages/world_flags/README.md:131-131
Timestamp: 2025-02-09T11:33:18.006Z
Learning: The class name `CustomPainter` from Flutter SDK should be formatted with backticks, and when pluralized, the 's' should be outside the backticks (e.g., `CustomPainter`s).
Applied to files:
packages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dart
π Learning: 2025-04-21T22:21:29.565Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 326
File: packages/world_countries/lib/src/extensions/typed_locale_extension.dart:125-141
Timestamp: 2025-04-21T22:21:29.565Z
Learning: In the Flutter SDK, assertions are disabled in release builds, so using assert(false, ...) will only crash the application in debug mode, which can be intentional for development-time error visibility.
Applied to files:
packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
π Learning: 2025-03-22T23:30:30.348Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 314
File: packages/world_countries/lib/src/extensions/text_field_extension.dart:85-86
Timestamp: 2025-03-22T23:30:30.348Z
Learning: In Dart with null safety, non-nullable parameters (those without a question mark in their type declaration) are guaranteed by the compiler to never be null, making runtime null checks redundant for such parameters.
Applied to files:
packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
π Learning: 2025-06-29T20:30:14.357Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/example/lib/main.dart:77-79
Timestamp: 2025-06-29T20:30:14.357Z
Learning: The user (tsinis) prefers runtime type checks (`is`) over casting (`as`) in UI code like ValueListenableBuilder for safety and defensive programming, even when the type is expected to be known at compile time.
Applied to files:
packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
π Learning: 2024-10-15T18:54:17.918Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 235
File: packages/world_countries/README.md:243-243
Timestamp: 2024-10-15T18:54:17.918Z
Learning: In the 'sealed_world' project, adding additional examples or specific features to demonstrate accessibility is not necessary because Flutter has limited accessibility features.
Applied to files:
packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: DCM report for sealed_currencies
- GitHub Check: π‘οΈ Verify sealed_currencies / β³ Verify sealed_currencies Package
- GitHub Check: π‘οΈ Verify world_countries / β³ Verify world_countries Package
- GitHub Check: π‘οΈ Verify world_flags / β³ Verify world_flags Package
- GitHub Check: Agent
π Additional comments (28)
packages/world_flags/example/macos/Runner/Info.plist (1)
31-32: LGTM - Consistent with world_countries example.The Impeller flag is enabled consistently across both example apps, which is correct. Same verification notes apply as in packages/world_countries/example/macos/Runner/Info.plist.
packages/world_countries/example/macos/Runner/Info.plist (1)
31-32: TheFLTEnableImpellerflag name is correct and matches the official Flutter documentation. The syntax is also properly formatted. This configuration is appropriate for the example app and no issues were found.packages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dart (3)
1-36: LGTM! Clean implementation of clipped triangle painter.The implementation correctly:
- Clips the canvas to flag bounds to prevent shader overflow (line 15)
- Applies aspect ratio compensation to maintain proportions (line 21)
- Constructs a right-pointing triangle with proper path geometry (lines 27-31)
- Returns the required
FlagParentBoundsstructure (line 34)The class is well-documented and follows the expected
ElementsPainterpattern. Pending verification of the null-safety and antialiasing concerns noted above, this is a solid addition.
22-23: > Likely an incorrect or invalid review comment.
27-32: No action required. Antialiasing is already properly enabled for the diagonal edges. ThepaintCreator()method returns aPaint()object with default settings, which includesisAntiAlias = true. This is correct for rendering the diagonal triangle edges and prevents the "stairs" effect.packages/world_flags/lib/src/ui/effects/flag_shader_options.dart (4)
1-68: Well-structured immutable configuration class.The class is correctly annotated with
@immutableand uses allfinalfields with sensible defaults. The documentation provides clear usage guidance and recommendations for GPU-intensive scenarios.
146-161: toString implementation is comprehensive.All 24 fields are included for debugging visibility.
163-216: copyWith method correctly follows constructor parameter naming.Based on learnings, this is the idiomatic Dart approach for copyWith methods even when parameter shadowing occurs.
248-280: Equality implementation is correct.The identity check short-circuit and field-by-field comparison are properly implemented. The ignore comment for
avoid-complex-conditionsis reasonable given the nature of equality operators in large value classes.packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart (7)
1-22: Comprehensive test setup with proper widget structure verification.The test correctly verifies that
FlagShaderSurfacerenders aCustomPaintdescendant, which is the expected rendering mechanism for shader-based flags.
24-47: Fallback and alternative map tests cover edge cases well.Testing both the
orElsefallback when an item isn't found and thealternativeMapcustomization ensures the widget handles various configuration scenarios correctly.
49-82: Dimension and aspect ratio tests validate layout behavior.Tests properly verify that custom
width,height, andaspectRatioare applied to the internalSizedBoxandAspectRatiowidgets.
84-116: Diagnostic tests ensure debuggability.Testing
toStringShortanddebugFillPropertiesimproves developer experience when debugging widget trees.
118-146: Shader delegate lifecycle is properly managed.The tests correctly create delegates with
vsync: testerand manually calldispose()after the test, which is the correct pattern for externally-managed shader delegates.
148-177: Painter reuse test verifies optimization.Testing that the same painter instance is reused when the delegate remains unchanged ensures the widget avoids unnecessary allocations.
179-186: Golden tests provide visual regression coverage.The iteration over multiple
WorldCountryvariants ensures shader rendering is visually validated across different flag designs.packages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dart (6)
14-31: Clear image ownership documentation and proper repaint wiring.The class documentation clearly establishes that the painter owns
_imageand the shader delegate must not dispose it. Passingshaderas therepaintlistenable correctly triggers repaints when the shader notifies listeners.
41-49: Paint method has defensive fallback.The null check for
imagewith a fallback tosuper.paint()provides resilience, though the comment correctly notes this is practically unreachable after_ensureCache.
51-78: Cache management is correct and efficient.The cache invalidation logic properly checks all relevant state (size, scale, clip), and
_rebuildCachecorrectly disposes the previous image before creating a new one, preventing memory leaks.
80-91: Centered scaling transform is correctly implemented.The translate-scale-translate pattern properly scales around the center point, which is the expected behavior for flag rendering.
93-113: Shader and fallback rendering paths handle clipping consistently.Both
_renderWithShaderand_drawCachedImageapply the same clipping behavior whenshouldClipContentis true, ensuring visual consistency regardless of which path is taken.
115-132: shouldRepaint and dispose are properly implemented.The
shouldRepaintoverride correctly compares all relevant state, anddispose()fully cleans up resources while clearly documenting that the shader delegate is not disposed here.packages/world_flags/lib/src/ui/effects/waved_flag_shader_delegate.dart (6)
14-35: Well-documented image ownership contract.The class documentation clearly states that images passed to
paintWithShaderare not owned by this delegate, preventing accidental double-disposal.
37-64: Static shader caching and warm-up pattern is appropriate.The static
_programand_programLoaderensure the shader is loaded only once across all delegate instances. ThewarmUpmethod allows pre-loading before instantiation if needed.
88-116: Ticker management handles animation state correctly.The
_updateTickermethod properly stops/starts the ticker based onoptions.animate, and_handleTickaccumulates time with wraparound at 1e4 to prevent floating-point precision issues.
118-153: Shader uniform configuration is comprehensive.All uniform indices are set consistently with clear comments grouping related parameters. The wave direction normalization prevents division-by-zero with the
len > 0.001check.
163-185: paintWithShader correctly updates size and sampler each frame.The comment on lines 167-169 correctly explains why caching must be avoided for the image sampler. The try-catch with
on Objectprovides comprehensive error handling including non-Exception errors.
52-55: Verify shader path resolution in test and production environments.The shader path differs based on
FLUTTER_TESTenvironment variable. Ensure the test path"shaders/waved_flag.frag"is correct relative to the test working directory, and the production path includes the package prefix.#!/bin/bash # Verify shader file exists at both paths fd -t f "waved_flag.frag" packages/world_flags
There was a problem hiding this comment.
Pull request overview
This PR introduces shader-based waving flag animations to the world_flags package, adding a GPU-accelerated visual effect that simulates fabric motion on flag surfaces.
Key Changes:
- Adds
FlagShaderSurfacewidget withWavedFlagShaderDelegatefor real-time GPU-based flag animations - Introduces
FlagShaderOptionsfor controlling wave parameters, turbulence, lighting, and fabric appearance - Updates SDK constraint from ^3.9.2 to ^3.10.4 and Android Gradle configuration for compatibility
Reviewed changes
Copilot reviewed 34 out of 41 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/world_flags/shaders/waved_flag.frag | Fragment shader implementing wave displacement, fabric texture, and lighting effects |
| packages/world_flags/lib/src/ui/effects/* | Core shader delegate, options, and surface widget implementation |
| packages/world_flags/lib/src/ui/painters/basic/shader_stripes_painter.dart | Painter that caches flag content and feeds it to shader delegates |
| packages/world_flags/test/src/ui/effects/* | Comprehensive test coverage for shader components |
| packages/world_flags/pubspec.yaml | SDK version bump and shader asset registration |
| packages/world_flags/lib/world_flags.dart | Export new shader-related classes |
| packages/world_flags/lib/src/ui/painters/custom/clipped_triangle_painter.dart | New painter for Bahrain flag to prevent shader overflow artifacts |
| packages/world_flags/example/* | Example app updates to demonstrate shader usage and configuration updates |
Comments suppressed due to low confidence (2)
packages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dart:1
- Comment references 'Dart 3.8 formatting' but the current Dart SDK constraint in pubspec.yaml is ^3.10.4. Consider updating the comment for consistency or removing it if the formatting is already applied.
packages/world_flags/test/src/ui/effects/waved_flag_shader_delegate_test.dart:1 - Comment references 'Dart 3.8 formatting' but the current Dart SDK constraint in pubspec.yaml is ^3.10.4. Consider updating the comment for consistency or removing it if the formatting is already applied.
π‘ Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Updated waveDirection parameter to be passed as a single object. - Adjusted golden widget test to account for waved flag rendering on non-Linux platforms. - Added a comment to clarify the purpose of the test assertion.
There was a problem hiding this comment.
Actionable comments posted: 2
π€ Fix all issues with AI agents
In
@packages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart:
- Line 40: The pump call inside golden_widget_tester_extension.dart that waits
for 100ms when isWaved is true lacks an explanatory comment; add a brief comment
above the line `if (isWaved) await pump(const Duration(milliseconds: 100));`
stating that waved flags use shader-based animations which require a short
stabilization window for GPU shaders and frame compositing to settle, explain
that 100ms was chosen empirically to allow one or two animation frames and
shader warm-up on typical test CI hosts, and note that the value may be adjusted
if flaky renderings appear on different CI environments.
- Line 44: The waved golden path currently prepends "../" causing incorrect
resolution; update the call using matchesGoldenFile so it does not add the "../"
prefix (remove the ternary that yields "../$filePath" for isWaved), i.e., always
pass filePath (use the existing isWaved variable only if needed for other logic
but do not alter the path), targeting the matchesGoldenFile invocation where
isWaved and filePath are used.
π Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
π Files selected for processing (3)
packages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart
π§° Additional context used
π§ Learnings (8)
π Common learnings
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart:86-91
Timestamp: 2025-06-29T20:31:42.528Z
Learning: In flag stripe rendering (packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart), diagonal lines should be antialiased to avoid the "stairs" effect, while vertical and horizontal lines should not use anti-aliasing as they align perfectly with the pixel grid and don't produce jagged artifacts.
π Learning: 2025-06-29T20:31:42.528Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart:86-91
Timestamp: 2025-06-29T20:31:42.528Z
Learning: In flag stripe rendering (packages/world_flags/lib/src/ui/painters/basic/stripes_painter.dart), diagonal lines should be antialiased to avoid the "stairs" effect, while vertical and horizontal lines should not use anti-aliasing as they align perfectly with the pixel grid and don't produce jagged artifacts.
Applied to files:
packages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
π Learning: 2024-10-14T20:46:43.876Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 234
File: packages/world_flags/README.md:28-28
Timestamp: 2024-10-14T20:46:43.876Z
Learning: In the documentation files (e.g., `packages/world_flags/README.md`), the term "Widget" is intentionally capitalized to align with Flutter's conventions and the slogan "Everything is a Widget".
Applied to files:
packages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
π Learning: 2025-02-08T23:57:46.308Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 298
File: packages/sealed_countries/lib/src/model/country/submodels/idd.dart:23-24
Timestamp: 2025-02-08T23:57:46.308Z
Learning: Assertion messages in Dart should use backticks (`) for code references, not square brackets ([]), as they are runtime strings for debugging and not dartdoc references.
Applied to files:
packages/world_flags/lib/src/ui/effects/flag_shader_options.dartpackages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart
π Learning: 2025-06-29T20:33:00.216Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 360
File: packages/world_flags/lib/src/helpers/extensions/iso_flag_extension.dart:40-41
Timestamp: 2025-06-29T20:33:00.216Z
Learning: In Flutter/Dart, copyWith methods should follow the exact naming convention of the constructor parameters, even when this causes parameter shadowing (e.g., using `key` parameter name even though StatelessWidget has a `key` getter). This is the established idiomatic approach and consistency with constructor signatures is preferred over avoiding parameter shadowing.
Applied to files:
packages/world_flags/lib/src/ui/effects/flag_shader_options.dart
π Learning: 2025-01-23T22:23:47.517Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 284
File: packages/sealed_countries/test/src/helpers/utils/typed_localization_delegate_test.dart:7-16
Timestamp: 2025-01-23T22:23:47.517Z
Learning: In Dart test files, prefer keeping compile-time constants (`const`) as-is instead of moving them to `setUp` or making them `late`, as they are immutable and benefit from compile-time optimization.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
π Learning: 2025-06-22T17:32:55.576Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 353
File: packages/world_countries/lib/src/widgets/country/country_picker.dart:126-0
Timestamp: 2025-06-22T17:32:55.576Z
Learning: In the CountryPicker widget (packages/world_countries/lib/src/widgets/country/country_picker.dart), the leading parameter is intentionally nullable. When flagsMap[itemProperties.item] returns null (no flag found), no fallback widget should be displayed - this is the intended design behavior.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
π Learning: 2025-07-13T17:41:39.732Z
Learnt from: tsinis
Repo: tsinis/sealed_world PR: 366
File: packages/world_countries/test/src/extensions/platform_dispatcher_extension_test.dart:1-167
Timestamp: 2025-07-13T17:41:39.732Z
Learning: In Flutter widget tests using `testWidgets`, the callback function must be marked as `async` even without `await` statements because the function signature requires `Future<void> Function(WidgetTester) callback`. Static analysis tools may incorrectly flag these as unnecessary async functions.
Applied to files:
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dartpackages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart
β° Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: π‘οΈ Verify world_countries / β³ Verify world_countries Package
- GitHub Check: π‘οΈ Verify world_flags / β³ Verify world_flags Package
- GitHub Check: π‘οΈ Verify sealed_currencies / β³ Verify sealed_currencies Package
π Additional comments (5)
packages/world_flags/lib/src/ui/effects/flag_shader_options.dart (1)
1-279: LGTM! Well-implemented immutable data class.The implementation is thorough and correct:
- All 25 fields are consistently included in
toString,copyWith,hashCode, and the equality operator.- The
@immutableannotation andconstconstructor are appropriate for this configuration class.- Documentation is comprehensive with helpful usage examples and performance guidance (lines 17-38).
- Direct floating-point equality (lines 253-277) is acceptable here since these are explicitly-set configuration values rather than computed results.
packages/world_flags/test/src/ui/effects/flag_shader_surface_test.dart (1)
1-188: LGTM! Comprehensive test coverage.The test suite is well-structured and thorough:
- Covers core functionality: rendering, fallback behavior, custom configurations, and diagnostics.
- Properly manages resources with explicit
dispose()calls for shader delegates (lines 135-136, 145).- Tests painter reuse behavior (lines 148-177), which is important for performance.
- Includes golden tests for multiple countries with appropriate test assertion handling (line 181).
packages/world_flags/test/helpers/extensions/golden_widget_tester_extension.dart (3)
19-19: LGTM! Clean conditional widget selection.The
isWavedflag and conditional widget creation provide a clear, maintainable approach to selecting the appropriate rendering path for different flag types.Also applies to: 28-30, 34-34
43-43: Consider clarifying the skip condition and reason message.The skip logic now excludes all waved flags on non-Linux platforms, meaning waved flags are only golden-tested on Linux. While this may be intentional due to shader rendering differences, the reason message (
"Non-Linux platforms rendering those flags slightly differently") is somewhat ambiguous β it doesn't explicitly clarify that all waved flags are skipped, not just those in the ignore list.The conditional type matcher correctly selects
FlagShaderSurfacefor waved flags andIsoFlag<T, BasicFlag>for non-waved cases.Consider clarifying the reason message:
π Suggested clarification
- reason: "Non-Linux platforms rendering those flags slightly differently", + reason: "Non-Linux platforms render these flags or shader-based waved flags slightly differently",Also applies to: 45-47
12-16: No actionable issues remain. The map construction at lines 12-16 correctly uses bothsmallSimplifiedFlagsMapandsmallSimplifiedLanguageFlagsMapalong with the explicitFiatEur()entry for the test fixture.Likely an incorrect or invalid review comment.
Type of Change
Checks
Please look at the following checklist to ensure that your PR can be accepted quickly:
Summary by CodeRabbit
New Features
Updates
Tests
βοΈ Tip: You can customize this high-level summary in your review settings.