Add readonly to members in Rect

[JeremyKuhne]

Marking members as readonly allows the compiler to generate more efficient code.

https://learn.microsoft.com/dotnet/csharp/language-reference/builtin-types/struct#readonly-instance-members

This is in two commits. The first collapses code into a single file and the second adds the readonly. Suggest looking at the second commit for the diff and do not squash the commits.

Microsoft Reviewers: Open in CodeFlow

[JeremyKuhne]

Point, Size, Int32Rect, and Vector would benefit from the same treatment. All structs would, but commonly used ones will have more impact. I'll create these when I get the time, or if someone else feels compelled, ping me. :)

[miloush]

One would think that compiler could figure out that double X => _x is readonly...

[JeremyKuhne]

One would think that compiler could figure out that double X => _x is readonly...

It might. I haven't looked at this particular case. One could get a reasonable indication by looking at sharplab.io. In any case, better to be explicit. :)

[ThomasGoulet73]

What's the reasoning behind removing the generated Rect, is the plan to remove more generated code ? WpfGfx currently has knowledge of Rect and is the source of truth regarding it's definition, I'm not sure if moving the source of truth to PresentationCore and separate from WpfGfx is a good idea.

[miloush]

I was going to ask about that too, and the process that generates them, does it get regenerated?

[ThomasGoulet73]

It gets generated by MilCodeGen which needs to be ran manually (Though right now we can't run it since it's been broken since WPF was open sourced but it should be fixed by #6135).

I believe the entry that generates this file is this:

wpf/src/Microsoft.DotNet.Wpf/src/WpfGfx/codegen/mcg/xml/Resource.xml

Line 4005 in 4204efe

<Generate Name="Rect" ManagedClass="true" ManagedValueMethods="true" ManagedTypeConverter="true" NativeRetriever="true" AnimationResource="true"/>

[JeremyKuhne]

The odds of actually breaking it are pretty slim. Is it worth leaving this easy perf on the table for this? I don't think so. Add whatever comments one needs to, but adding / changing field order is going to fail spectacularly anyway.

[ThomasGoulet73]

My PRs for fixing MilCodeGen are now merged in main so MilCodeGen is now runnable.

Merging the PR as-is would mean that the next person to run MilCodeGen would recreate the generated Rect.cs. That leaves 3 option:

1. Modify MilCodeGen to stop generating Rect.
2. Modify this PR to keep the generated Rect and only add readonly to the non-generated members of Rect.
3. Modify MilCodeGen to add readonly to the generated methods.

Personally, I'm not a fan of option 1. There's tons of files generated from MilCodeGen, I just don't see why Rect should be "special" (Even if it's easy perf).

IMO the best option is option 3 but if you don't want to bother with modifying MilCodeGen, you could do option 2 and I can do option 3 in a separate PR (We should probably add readonly to the 13 structs using the same MilCodeGen template). Even just option 2 is good improvement because a lot of the commonly used members are not defined in the generated Rect (X, Y, Size, etc).

[ThomasGoulet73]

Also, I should also point out that MilCodeGen does not just generate the struct. It generates the struct, its type converter and its animation. We can stop generating only the struct but since the type converter and the animation uses members from the generated struct, I guess it makes it a bit less clean in my opinion.

[miloush]

I am up for updating MilCodeGen to generate readonly fields.

[h3xds1nz]

Yeah option 3 was on my radar as with other modifications.