Draft: Add SKSLEffect. #17981

kkwpsv · 2025-01-16T04:22:23Z

What does the pull request do?

Add fragment shader effect support using SKSL.

What is the current behavior?

Not support now.

What is the updated/expected behavior with this PR?

Fragment shader effects and animations work as expected.

How was the solution implemented (if it's not obvious)?

Create an SKImageFilter using SKImageFilter.CreateRuntimeShader, and works like other effect.

Checklist

Added unit tests (if possible)?
Added XML documentation to any related classes?
Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

A SkiaSharp change is required, which is base on SkiaSharp3.

Obsoletions / Deprecations

None.

Fixed issues

#17980

kkwpsv · 2025-01-16T04:24:30Z

The required SkiaSharp nuget package is here. Currently only Windows version is available.

kekekeks · 2025-01-16T09:32:36Z

src/Avalonia.Base/Media/Effects/IShaderEffect.cs

+
+namespace Avalonia.Media
+{
+    public interface IShaderEffect : IEffect


Custom effects need to report their expected padding, i. e. the area outside of the affected drawing bounds they will alter (e. g. drop-shadow(5px 5px) would add (0,0,5,5) padding for renderer invalidation).

Note that this can depend on uniform values.

I think shader effect shouldn't has padding?
The coord is sksl input, and the color is output. Coord cannot be changed.

SKImageFilter can have padding, that's how our blur and drop shadow effects work. They report padding to the compositor and it extends the intermediate texture bounds.

Actually, right now the intermediate texture size always matches the render target size. That's something we've planned to change but never actually implemented.

SKImageFilter can have padding, that's how our blur and drop shadow effects work. They report padding to the compositor and it extends the intermediate texture bounds.

But runtimeshader seems to be not?
I'll read skia source code to confirm it.

The "padding" is just shader sampling the source at a different coordinate than it's currently producing output for. E. g. a simple implementation of drop-shadow(5,5) shader would just sample src at (-5,-5) from coord and use float4(1-alpha, 1-alpha, 1-alpha, alpha) for output.

The "padding" is just shader sampling the source at a different coordinate than it's currently producing output for. E. g. a simple implementation of drop-shadow(5,5) shader would just sample src at (-5,-5) from coord and use float4(1-alpha, 1-alpha, 1-alpha, alpha) for output.

I have read skia source and usage of GetEffectOutputPadding.

GetEffectOutputPadding affect dirty rect and bounds in Avalonia compositor.
The drop shadow implementation is here.
It has blur and MatrixTransform. And they can affect rendering bounds. So we need padding for compositor.
The implementation seems to be not same as you said?

And i think runtimeshader cannot affect. It's just a fragment shader and a subset of SKimageFilter.
Or can you give me a runtimeshader example where padding is needed?

In WPF it's a normal thing for ShaderEffect to extend the affected Visual's render bounds. We should do the same. If a custom SKImageFilter somehow doesn't support that, we need to manually manage the intermediate texture and use SKShader instead of SKImageFilter.

kekekeks · 2025-01-16T09:34:26Z

samples/ControlCatalog/ControlCatalog.csproj

@@ -4,6 +4,7 @@
    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
    <Nullable>enable</Nullable>
    <IncludeAvaloniaGenerators>true</IncludeAvaloniaGenerators>
+    <AvsIncludeSkiaSharp3>true</AvsIncludeSkiaSharp3>


Avalonia 11.x will stay compatible with SkiaSharp 2.88, so such change cant' be merged. We need to have code paths for both 2.88 and 3.x.

I suppose some of user-facing version-specific APIs can live in separate Avalonia.Skia.VersionSpecific.2.88 and Avalonia.Skia.VersionSpecific.3.0 packages.
@maxkatz6

How feasible would it be to avoid SkiaSharp types in the new public API? This way it would be easier for us to maintain and abstract different versions internally.

@maxkatz6 This pr is add the sksl effect, which means that it depends on sksl. Or do you want to design a new shader language, alsl (avalonia shader language) ?

No. I mean SKIA shaders, but not SkiaSharp API (not public API at least, internally we can bridge different versions/implementations of SkiaSharp).

How feasible would it be to avoid SkiaSharp types in the new public API? This way it would be easier for us to maintain and abstract different versions internally.

Only SkiaSharp with my pr can works now.
This requires RuntimeShader, and SkiaSharp hasn't exposed this api yet.
And i think it's hard to avoid SkiaSharp types. The SKImageFilter can be many types of things, like images, shaders which is not exists in avalonia.

Technically we don't need SKRuntimeFilter and can work with SKShader. The way it would work is by manually creating an intermediate texture and SKCanvas for subsequent drawing calls and using it as an input for SKShader.

That way it can be compatible with 2.88.
So the API could accept SKSL source + Avalonia.Media.Bitmap for inputs.

kekekeks · 2025-01-16T09:37:57Z

src/Skia/Avalonia.Skia/Effects/SKSLEffect.cs

+            ShaderBuilder = builder;
+        }
+
+        public void RegisterUniform(string name, AvaloniaProperty<int> property) => _uniformProperties.Add(name, property);


How uniform updates are supposed to be propagated to the composition thread?

I think we need to make effects to use the same infrastructure as brushes do (ICompositionRenderResource/ICompositorSerializable)

It will be stored in the copided SkRuntimeShaderBuilder in ToImuatable.

How would invalidation work after the initial layout/render pass?

I guess AffectsRender<T> is still public in Effect.cs. It's something we are planning to remove though

Yes, I'm using AffectsRender currently.

When calling RegisterUniform, we can call AffectsRender internally?

I would still prefer switching to the compositor-aware infra, so we could make SKSLShader disposable and implement lifetime management for inputs/builder as well.

kekekeks · 2025-01-16T09:43:48Z

src/Skia/Avalonia.Skia/Effects/SKSLEffect.cs

+
+        public string[] ChildShaderNames { get; }
+
+        public SKImageFilter?[] Inputs { get; }


The problem with having ShaderBuilder and Inputs as such properties is lifetime management. There is no clear ownership of ImmutableSKSLEffect once it's passed to the compositor, so we are not allowed to manually call Dispose on ShaderBuilder and SKImageFilters, yet they might hold a huge chunk of unmanaged memory by referencing bitmaps. So the API is prone to "soft" unmanaged memory leaks when GC isn't aware that it's supposed to collect collect gen2 to free a few GB of native bitmaps.

@kekekeks Do you means using GC.AddMemoryPressure to add the memory pressure to GC? Or the GC untimely collect the object to release the skia resource?

Ownership of allocated objects needs to be transferred to the compositor and only the compositor knows when to free those objects. By implementing ICompositionRenderResource etc. you make sure these objects are properly ref counted and will be cleaned up at the right time.

ICompositorSerializable allows you to sync mutable states with the compositor, which will then hold an immutable copy of that resource. Using that mechanism makes AffectsRender redundant.

which will then hold an immutable copy of that resource.

Creating a copy of the SKImageFilter seems to be not possible. It's immutable.
Users may want to reuse SKImageFilter. So the ownership shouldn't be transferred to the compositor?
But maybe we can use ICompositionRenderResource to let users know when the compositor is no longer using the object.

As long as objects are properly ref counted this is fine

Users may want to reuse SKImageFilter

Compositor shouldn't be calling Dispose directly either, it should be calling a user-specified callback as a means to report that it's not using the resource anymore.

e. g. void AddInput(SKImageFilter filter, Action? release = null);

kekekeks · 2025-01-16T09:53:19Z

We probably want to expose a similar API for SKColorFilter like we currently do with XPF, but it should be way more simple to implement

lindexi · 2025-01-16T10:09:38Z

@kekekeks What is the API similar XPF means? Can it use the sksl?

kekekeks · 2025-01-16T10:21:07Z

What is the API similar XPF means? Can it use the sksl?

With XPF we have an extension API that allows setting SKColorFilter for a visual. That was required for some apps that utilized WPF's ShaderEffect to implement features like colormap. For Avalonia I'd like to have that as a proper IEffect.

kkwpsv · 2025-01-16T10:44:34Z

The RegisterUniform may be not good enough.
There's too many type of uniforms. We need many overloads. And some type is missing in Avalonia now like vector3 vector4.

dme-compunet · 2025-01-17T07:56:19Z

According to C# naming convention, the SKSLEffect** should be renamed to SkslEffect**

Add SKSLEffect.

2bea9f7

kkwpsv mentioned this pull request Jan 16, 2025

Add fragment shader effect support. #17980

Open

kkwpsv marked this pull request as draft January 16, 2025 04:25

maxkatz6 requested a review from kekekeks January 16, 2025 04:34

maxkatz6 added needs-api-review feature area-skia blocked labels Jan 16, 2025

kekekeks requested changes Jan 16, 2025

View reviewed changes

Fix effect name typo.

464cf0f


		public string[] ChildShaderNames { get; }

		public SKImageFilter?[] Inputs { get; }

Draft: Add SKSLEffect. #17981

Are you sure you want to change the base?

Draft: Add SKSLEffect. #17981

Conversation

kkwpsv commented Jan 16, 2025

What does the pull request do?

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

kkwpsv commented Jan 16, 2025

kekekeks Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

kkwpsv Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maxkatz6 Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kkwpsv Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Gillibald Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kekekeks commented Jan 16, 2025 • edited Loading

lindexi commented Jan 16, 2025

kekekeks commented Jan 16, 2025

kkwpsv commented Jan 16, 2025

dme-compunet commented Jan 17, 2025 • edited Loading

kekekeks Jan 16, 2025 •

edited

Loading

kkwpsv Jan 16, 2025 •

edited

Loading

kekekeks Jan 16, 2025 •

edited

Loading

maxkatz6 Jan 16, 2025 •

edited

Loading

kkwpsv Jan 16, 2025 •

edited

Loading

kekekeks Jan 16, 2025 •

edited

Loading

Gillibald Jan 16, 2025 •

edited

Loading

kekekeks commented Jan 16, 2025 •

edited

Loading

dme-compunet commented Jan 17, 2025 •

edited

Loading