Mark Color as immutable and make Font a readonly struct - #33824
Conversation
There was a problem hiding this comment.
Pull request overview
This PR enhances type safety and documents immutability guarantees for two core Graphics types by adding explicit immutability markers.
Changes:
- Added
[ImmutableObject(true)]attribute to theColorclass to document its existing immutability - Converted the
Fontstruct fromstructtoreadonly structand changed auto-properties from{ get; private set; }to{ get; }
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Graphics/src/Graphics/Color.cs | Added ImmutableObjectAttribute to formalize Color's existing immutability contract |
| src/Graphics/src/Graphics/Font.cs | Converted to readonly struct with get-only properties, matching patterns used in other MAUI readonly structs |
| [DebuggerDisplay("Red={Red}, Green={Green}, Blue={Blue}, Alpha={Alpha}")] | ||
| [TypeConverter(typeof(Converters.ColorTypeConverter))] | ||
| [ImmutableObject(true)] | ||
| public class Color |
There was a problem hiding this comment.
For Immutability wouldn't make sense to change this to be record? I'm not sure if it will be a binary breaking change thought
There was a problem hiding this comment.
record is C# syntax and doesn't alter the object signature, but implements IEquatable for free
There was a problem hiding this comment.
but roslyn can detect that, so why not
| /// <summary> | ||
| /// Determines whether the specified <see cref="Color"/> is equal to the current color using byte-precision comparison. | ||
| /// </summary> | ||
| public virtual bool Equals(Color other) |
There was a problem hiding this comment.
I'm confused. Why isn't this override?
9a551d3 to
b155ba1
Compare
b155ba1 to
dbca93f
Compare
dbca93f to
b20ac49
Compare
|
Hi @PureWeen — the builds are now passing (macOS Debug/Release, Windows Debug/Release, Helix unit tests all green ✅). The only failures are 'Run Integration Tests Build macOS' and 'RunOnAndroid' which are pre-existing on the net11.0 branch baseline. Regarding the Could you re-review when you get a chance? |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 33824Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 33824" |
- Add [ImmutableObject(true)] attribute to Color class to document its immutability
- Convert Font from struct to readonly struct
- Change Font properties from { get; private set; } to { get; }
- Change Color from class to record class for IEquatable<Color> support - Add explicit Equals(Color) using ToInt() to preserve byte-precision semantics - All equality paths (==, !=, Equals) use consistent int comparison - Add PublicAPI.Unshipped.txt entries for compiler-generated record members
…rd class CS8872: 'Color.Equals(Color)' must allow overriding because the containing record is not sealed. The virtual modifier is mandatory for record class types.
…lor record - GetHashCode() now returns ToInt() to be consistent with Equals(Color? other), which also uses ToInt() for byte-precision comparison. Previously GetHashCode() hashed raw float values while Equals() quantized to bytes, violating the contract (two equal Colors could have different hash codes). - Equals(Color? other) now checks EqualityContract to properly distinguish Color from derived record types, matching standard record equality semantics. - Parameter type changed from Color to Color? to match the record-generated Equals pattern and allow proper null handling. - Updated PublicAPI.Unshipped.txt for all TFMs to reflect the nullable parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
18af7df to
f6643c9
Compare
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description
This PR improves type safety and documents immutability for core Graphics types.
Intent
These changes enable future XAML Source Generator (XSG) enhancements. By explicitly marking types as immutable, the source generator can make better optimization decisions, such as:
Changes
Color class: Added
[ImmutableObject(true)]attributereadonly)Font struct: Converted to
readonly structstructtoreadonly struct{ get; private set; }to{ get; }private setwas only used in the constructor, so this is not a breaking changeWhy
readonly structAPI Changes
None - these are non-breaking enhancements that formalize existing immutability guarantees.