Skip to content

Conversation

@h3xds1nz
Copy link
Member

@h3xds1nz h3xds1nz commented Aug 6, 2024

Description

Removes CheckedPointer which was used in quite some places back in early WPF implementations in NetFX but now the only thing using it is TrueTypeFontDriver and replaces it with ReadOnlySpan<T>.

  • The Probe checks are the same as implemented by ReadOnlySpan<T> when slicing.
  • Removes now useless Marshal.Copy with simple .ToArray() call from ReadOnlySpan<T> in ComputeFontSubset.
  • I've used Slice and non-Try methods from BinaryPrimitives to be sure the exception behavior stays the same.
  • Offers quite nice performance improvement over the original version.

SetFace method

Method Mean [ns] Error [ns] StdDev [ns] Gen0 Code Size [B] Allocated [B]
Original 94.60 ns 1.523 ns 1.564 ns 0.0358 1,325 B 600 B
PR__EDIT 53.56 ns 1.039 ns 0.921 ns 0.0358 512 B 600 B

Customer Impact

Smaller code-size of PresentationCore, improved performance in this particular scenario.
For developers it introduces more familiar types in internal classes than custom implementations for a simple thing.

Regression

No.

Testing

Tested base font file parsing and class (TrueTypeFontDriver) functionality after CheckedPointer removal.

Risk

Low.

Microsoft Reviewers: Open in CodeFlow

@h3xds1nz h3xds1nz requested review from a team as code owners August 6, 2024 19:45
@dotnet-policy-service dotnet-policy-service bot added PR metadata: Label to tag PRs, to facilitate with triage Community Contribution A label for all community Contributions labels Aug 6, 2024
@h3xds1nz h3xds1nz force-pushed the remove-checkedpointer branch from 32ee796 to c5ee887 Compare October 7, 2024 16:35
@h3xds1nz
Copy link
Member Author

h3xds1nz commented Oct 7, 2024

Resolved merge conflicts 10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Community Contribution A label for all community Contributions Included in test pass PR metadata: Label to tag PRs, to facilitate with triage Status:Completed Status:Proposed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants