mono / SkiaSharp

SkiaSharp is a cross-platform 2D graphics API for .NET platforms based on Google's Skia Graphics Library. It provides a comprehensive 2D API that can be used across mobile, server and desktop models to render images.
MIT License
4.52k stars 540 forks source link

[FEATURE] Replace `ref` parameters by `in` parameters where applicable #2314

Open OJacot-Descombes opened 2 years ago

OJacot-Descombes commented 2 years ago

Problem

As an example, SKCanvas.Concat is declared as public void Concat (ref SKMatrix m). This does not let me write

canvas.Concat(ref obj.Matrix); // Where Matrix is a readonly field.

I get

CS0192 A readonly field cannot be used as a ref or out value (except in a constructor)

I have to create a local copy of the SKMatrix struct first. But copying the struct is precisely what the ref keyword is supposed to avoid and is not good for performance.

Proposed solution

Replace the ref keyword with the in keyword (in all methods where this applies).

public void Concat (in SKMatrix m)

Additional context

The documentation for in parameter modifier (C# Reference) says

The in keyword causes arguments to be passed by reference but ensures the argument is not modified.

AnarchyMob commented 1 year ago

SKMatrix is not a read-only structure, passing through with "in" modifier will create a shadow copy of the structure...

OJacot-Descombes commented 1 year ago

@AnarchyMob, this is not how the in-keyword works in C#. It passes a reference to the original struct but disallows assignments to it:

image

See also the link in the "Additional context" section of my post. The Microsoft's doc clearly says

The in keyword causes arguments to be passed by reference but ensures the argument is not modified.

ojb500 commented 1 year ago

@OJacot-Descombes

The in keyword causes arguments to be passed by reference but ensures the argument is not modified.

True, but the compiler is often forced to emit a defensive copy to ensure this.

Without the readonly modifier the compiler cannot know that calling a method of m (or even a get accessor) would not mutate the struct in some way. Therefore it would be unsafe to pass a reference to the original struct.

https://learn.microsoft.com/en-us/dotnet/csharp/write-safe-efficient-code#avoid-defensive-copies

OJacot-Descombes commented 1 year ago

OK, I learned something. But luckily there are Readonly Instance Members. You can apply readonly to property accessors and to methods.

private int _x;
public int X { readonly get { return _x; } set { _x = value; } }
cyraid commented 1 year ago

I wouldn't mind at least being able to pass a ref to skia as that's a lot of structure copying as I'm saving and restore the matrix a lot.