SKShader.CreateLocalMatrix may or may not return the same object?

[andersforsgren]

I'm trying to wrap my head around how the managed wrappers for objects like shaders work, when there are functions on the SKShader type that might return the SAME object. Example: the return value of SKShader.CreateLocalMatrix may or may not be the argument object. If the matrix is an identity then the same object is returned.

var shader1 = SKShader.CreateColor(new SKColor());
var shader2 = SKShader.CreateColor(new SKColor());
var shader3 = SKShader.CreateLocalMatrix(shader1, SKMatrix.CreateIdentity());
var shader4 = SKShader.CreateLocalMatrix(shader2, SKMatrix.CreateRotationDegrees(45));

shader3.Dispose();  // disposes shader3 and shader1 (since shader1 == shader3)
shader4.Dispose();  // disposes shader4 but not shader2 (since shader2 != shader4)

But this behavior makes it really hard to work in the case of a general matrix. Say you have an image in a shader that you want to retain (not dispose) and you wrap it in an outer shader with a matrix. What then have you created so that you need to dispose it? "It depends"?

It feels like the API becomes kind of clumsy for a C# IDisposable when a method can sometimes return its argument and some times not? Have I misunderstood something?

Is this optimization to make the SKShader.Create..(...) methods potentially return new OR the same object new behavior in 2.X?

[mattleibow]

This is very interesting... Sometimes they reuse objects, but I did not think they did for SKShader...

The docs on the member don't say anything:

/**
 *  Return a shader that will apply the specified localMatrix to this shader.
 *  The specified matrix will be applied before any matrix associated with this shader.
 */

But I see the code does:

sk_sp<SkShader> SkShader::makeWithLocalMatrix(const SkMatrix& localMatrix) const {
    if (localMatrix.isIdentity()) {
        return sk_ref_sp(const_cast<SkShader*>(this));
    }
    // ...
}

I can modify the C++ code to always make a copy?

[andersforsgren]

I think both the C# api wrapper and the C++ code should return the same object, but that object should have an increased ref count?

 if (localMatrix.isIdentity()) {
        return sk_ref_sp(const_cast<SkShader*>(this));
 }

Shuldn't this increase the ref count to 'this' by one before it returns the same ref counted pointer? I can't see where it's going wrong then. But it definitely changed behavior since 1.X

Here is the behavior in e.g. v1.60.1 (I Didn't actually look at the ref counts in this version because I didn't find SkiaApi.sk_refcnt_get_ref_count but the object temp is the same as the object shader1 here, in when mat is identity. So that behavior of returning the same shader reference has not changed. But the ref counting seems to have stopped working?)

using (SKShader shader1 = SKShader.CreateColor(new SKColor()))    // shader1 has ref count=1
{
   using (SKShader temp = SKShader.CreateLocalMatrix(shader1, mat)) // Temp has the same Handle but with ref count 2
   {
       // use temp wrapper here
   }  // temp is disposed which decrements the ref count on its handle to 1, so the handle is still not IntPtr.Zero
   
   // keep using shader1 here.  This works.
   
} // shader1 is disposed which decrements the ref count to 0  and the Handle becomes IntPtr.Zero here.

Behavior in v2.88:

using (SKShader shader1 = SKShader.CreateColor(new SKColor()))    // shader1 has ref count=1
{
   using (SKShader temp = SKShader.CreateLocalMatrix(shader1, mat)) // Temp has the same Handle, and ref count=1 (!)
   {
       // use temp wrapper here
   }  // temp is disposed, ref count becomes 0, so the handle becomes IntPtr.Zero for both temp and shader1
   
   // keep using shader1 here. This will fail.
} 

In 2.88 I could actually check the ref counts in the above example simply by adding a watch statement for SkiaApi.sk_refcnt_get_ref_count to confirm that the ref count is never 2.

Update: Stepping through it I can see that the createLocalMatrix does bump the ref count to 2, but that the managed wrapper does an unref() before returning. That happens in GetOrAddObject() which is called with unrefExisting=true

SkiaSharp.dll!SkiaSharp.SKObjectExtensions.SafeUnRef(SkiaSharp.ISKReferenceCounted obj) Line 341
SkiaSharp.dll!SkiaSharp.HandleDictionary.GetOrAddObject<SkiaSharp.SKShader>(nint handle, bool owns, bool unrefExisting, System.Func<nint, bool, SkiaSharp.SKShader> objectFactory) Line 86
SkiaSharp.dll!SkiaSharp.SKObject.GetOrAddObject<SkiaSharp.SKShader>(nint handle, System.Func<nint, bool, SkiaSharp.SKShader> objectFactory) Line 104
SkiaSharp.dll!SkiaSharp.SKShader.GetObject(nint handle) Line 434
SkiaSharp.dll!SkiaSharp.SKShader.WithLocalMatrix(SkiaSharp.SKMatrix localMatrix) Line 31

I used this test case

[Fact]
public void TestCreateLocalMatrixRefCount()
{
	SKShader s1 = SKShader.CreateColor(new SKColor(255, 0, 0, 128));
	Assert.Equal(1, SkiaApi.sk_refcnt_get_ref_count(s1.Handle));
	SKShader s2 = s1.WithLocalMatrix(SKMatrix.CreateIdentity());
	Assert.True(ReferenceEquals(s1, s2));
	Assert.Equal(2, SkiaApi.sk_refcnt_get_ref_count(s1.Handle));
	s2.Dispose();
	Assert.Equal(1, SkiaApi.sk_refcnt_get_ref_count(s1.Handle));
	Assert.NotEqual(IntPtr.Zero, s1.Handle);
	s1.Dispose();
	Assert.Equal(IntPtr.Zero, s1.Handle);
}

[andersforsgren]

Do you think this should be converted into an issue?

[mattleibow]

Please do create an issue!

[mattleibow]

Is this just for identity? Maybe I can just cache that one and prevent dispose from doing anything... I have cases like this with empty SKData and default SKTypeface. Then it will always use the right instance and prevent it from ever being collected

[mattleibow]

Thanks for all the investigation on this one, I think I know what I have to do. I opened this issue: #2782