More Span<T> and ReadOnlySpan<T> #2669
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…oads are still useful. Some null checks have been removed as they are immediately followed by minimum length checks, which effectively accomplish the same goal.
The length checks immediately following the null checks accomplish the same goal.
…nlySpan<char> instead of strings.
|
I'll make some tests later. Feedback on the null-terminated LPArray/LPStr situation would be appreciated! |
…apedText to use ReadOnlySpan<char>
ScrubN
commented
Nov 14, 2023
There was a problem hiding this comment.
Is there a specific reason public Result Shape(string text, float xOffset, float yOffset, SKFont font) uses buffer.AddUtf8 instead of buffer.AddUtf16?
|
Since the CI didn't run, here's a screenshot of the test results from my IDE. Was a bit of a pain to get it working since its setup for azure pipelines. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
I have migrated many methods that previously required Arrays to use Spans instead. Additionally, a handful of methods that were previously string-only and already had the potential to be substituted by ReadOnlySpan were also converted to spans.
Some methods that previously did not use spans warranted keeping non-span overloads due to specific advantages or assumptions, such as the lack of needing to make defensive copies or the fact that strings are so versatile.
Some methods could not be migrated from array/string to span because the C++ requires that they be null-terminated. This could be solved by copying the contents of the span into a rented array and filling the rest of the array with
null, however I wanted to discuss this change before making it.The next changes aren't strictly related to converting to spans, but I figured they were related enough to include them here. If not, I can move them to a new PR.
I changed a few
return new T[0]toreturn Array.Empty<T> ()to reduce pointless allocations. As far as I can tell, this does not change the behaviour of dereferencing a pointer to the array.I also changed the algorithm of
implicit operator SKRuntimeEffectUniform (float[][] value)to make allocations linearly associated with the length ofvaluerather than using a variable length list.Lastly, I believe I fixed 4 memory leaks in
SKCanvas.DrawVertices, swapped out 3SKTypeFace.ToFontcalls toSKTypeFace.GetFontinSKTypeFace.GetGlyphs, and fixed a potential(?) nullptr dereference inSKPath.GetPoints. If any changed behaviour occurs in those methods as a result, that might have been my fault.Bugs Fixed
Span<T>orReadOnlySpan<T>#2616API Changes
new T[0]forArray.Empty<T> ()Behavioral Changes
None.
Required skia PR
None.
PR Checklist