Skip to content

Conversation

@kkwpsv
Copy link
Contributor

@kkwpsv kkwpsv commented Jan 15, 2025

Description of Change

Add RuntimeShader support.
See #3137.

Bugs Fixed

None.

API Changes

Added:

  • SKImageFilter SKImageFilter.CreateRuntimeShader(SKRuntimeShaderBuilder builder, string childShaderName, SKImageFilter? input)
  • SKImageFilter SKImageFilter.CreateRuntimeShader (SKRuntimeShaderBuilder builder, float maxSampleRadius, string childShaderName, SKImageFilter? input)
  • SKImageFilter SKImageFilter.CreateRuntimeShader (SKRuntimeShaderBuilder builder, float maxSampleRadius, string[] childShaderNames, SKImageFilter?[] inputs)
  • SKRuntimeEffectUniforms (SKRuntimeEffectUniforms effectUniforms)
  • SKRuntimeEffectChildren (SKRuntimeEffectChildren effectChildren)
  • SKRuntimeEffectBuilder (SKRuntimeEffectBuilder builder)
  • SKRuntimeShaderBuilder (SKRuntimeShaderBuilder builder)

Behavioral Changes

None.

Required skia PR

mono/skia#145

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of main at time of PR
  • Merged related skia PRs
  • Changes adhere to coding standard
  • Updated documentation

@kkwpsv
Copy link
Contributor Author

kkwpsv commented Jan 15, 2025

@dotnet-policy-service agree

@kkwpsv
Copy link
Contributor Author

kkwpsv commented Jan 16, 2025

Here is a demo.
https://github.com/kkwpsv/AvaloniaSkiaRuntimeShaderDemo

Copy link
Contributor

@mattleibow mattleibow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this PR. This looks like a good feature to get in. Sorry about the long wait for a review @kkwpsv. 😞

I know the C++ API has this different way to make the "shader", but it always felt a bit odd to have this in sort of separate locations when the Shader, Blender and ColorFilter were all in SKRuntimeEffect.

I wonder if instead of putting the code in SKImageFilter, we rather add a SKRuntimeImageFilterBuilder and in fact mirror most of what we do for SKShader but instead of using the word shader like in C++ we use the word ImageFilter. I would even go so far as to add a CreateImageFilter method that just calls into CreateShader. And then also make a BuildImageFilter that then calls that - which would mean it really is just a shader. The Build() method on this would then call the image filter code in the C API and return a new SKImageFilter

They dropped the different builders in C++, so we might avoid exposing this at all in the C API: https://github.com/google/skia/blob/main/include/effects/SkImageFilters.h#L458. I would think we could rather do what we did for all the others and just pass the values all in as args:

sk_shader_t* sk_runtimeeffect_make_shader(sk_runtimeeffect_t* effect, sk_data_t* uniforms, sk_flattenable_t** children, size_t childCount, const sk_matrix_t* localMatrix)

They have this PR where they removed the builders and just have the base one: google/skia@0b776ce. I don't think we should do that as the separate builders are OK and also allow for a more fluent style code. But, that is also up for discussion.

Let me know what you think about these ideas!

@github-project-automation github-project-automation bot moved this to Changes Requested in SkiaSharp Backlog Feb 27, 2025
@kkwpsv
Copy link
Contributor Author

kkwpsv commented Feb 28, 2025

I wonder if instead of putting the code in SKImageFilter, we rather add a SKRuntimeImageFilterBuilder and in fact mirror most of what we do for SKShader but instead of using the word shader like in C++ we use the word ImageFilter. I would even go so far as to add a CreateImageFilter method that just calls into CreateShader. And then also make a BuildImageFilter that then calls that - which would mean it really is just a shader. The Build() method on this would then call the image filter code in the C API and return a new SKImageFilter

I think there is no problem with your idea.
What I am doing now is just to make the API close to C++.

@mattleibow
Copy link
Contributor

@kkwpsv are you able to make the changes?

@taublast
Copy link
Contributor

Need any help? This all sounds very exciting!

@OptimusPi
Copy link

Any updates? I tried the example repo, curious if it could handle the Balatro background effect shader.

kkwpsv/AvaloniaSkiaRuntimeShaderDemo#1

Works super well :)

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

Projects

Status: Changes Requested

Development

Successfully merging this pull request may close these issues.

4 participants