ComputeSharp icon indicating copy to clipboard operation
ComputeSharp copied to clipboard

Could use a `ID2D1PixelShader.ShaderID` property

Open rickbrew opened this issue 2 years ago • 5 comments

The heavy use of generics and structs is a boon for performance, but it can get in the way of clean code. In this case, I'm writing an ID2D1Transform that broadcasts a given channel to the output. That is, if R is the channel, then output = { input.R, input.R, input.R, input.R }.

I'm thinking this particular case would be easily solved by having a generated ShaderID property, or even a GetShaderID() extension method, on ID2D1PixelShader. Then I'd store a boxed instance of each shader and use the ShaderID property when calling LoadPixelShader() and SetPixelShader(). I could even cache these in static readonly fields so there's only 4 allocations ever.

I'm using the broadcast transform to extract the W (aka A) component of a sample map, whose output is { x, y, 0, w }. The x and y tell the SampleInputTransform where to read from the input, and the w is used as a coverage value (0 if the coordinate is out-of-bounds, else 1). The final output is the sum of each (SampleInputTransform * Coverage), then divided by Sum(Coverage).

The switches in the code below could be changed to just effectContext.LoadPixelShader(shaders[(int)channelSelector].ShaderID) and this.drawInfo.SetPixelShader(shaders[(int)channelSelector].ShaderID) with the addition of this property / extension method.

internal sealed partial class BroadcastChannelTransform
    : TrivialDrawTransform
{
    private readonly ChannelSelector channelSelector;

    public BroadcastChannelTransform(IDeviceEffectContext effectContext, ChannelSelector channelSelector)
        : base(effectContext, 1)
    {
        this.channelSelector = channelSelector;

        switch (channelSelector)
        {
            case ChannelSelector.R:
                effectContext.LoadPixelShader<RedShader>();
                break;

            case ChannelSelector.G:
                effectContext.LoadPixelShader<GreenShader>();
                break;

            case ChannelSelector.B:
                effectContext.LoadPixelShader<BlueShader>();
                break;

            case ChannelSelector.A:
                effectContext.LoadPixelShader<AlphaShader>();
                break;

            default:
                throw ExceptionUtil.InvalidEnumArgumentException(channelSelector, nameof(channelSelector));
        }
    }

    protected override void OnSetDrawInfo()
    {
        switch (this.channelSelector)
        {
            case ChannelSelector.R:
                this.DrawInfo.SetPixelShader<RedShader>(PixelOptions.TrivialSampling);
                break;

            case ChannelSelector.G:
                this.DrawInfo.SetPixelShader<GreenShader>(PixelOptions.TrivialSampling);
                break;

            case ChannelSelector.B:
                this.DrawInfo.SetPixelShader<BlueShader>(PixelOptions.TrivialSampling);
                break;

            case ChannelSelector.A:
                this.DrawInfo.SetPixelShader<AlphaShader>(PixelOptions.TrivialSampling);
                break;
        }

        this.DrawInfo.SetInputDescription(0, Filter.MinMagMipPoint);

        base.OnSetDrawInfo();
    }

    [D2DInputCount(0)]
    [D2DInputSimple(0)]
    [D2DEmbeddedBytecode(D2D1ShaderProfile.PixelShader40Level91)]
    private readonly partial struct RedShader
        : ID2D1PixelShader
    {
        public float4 Execute()
        {
            return D2D.GetInput(0).R;
        }
    }

    [D2DInputCount(0)]
    [D2DInputSimple(0)]
    [D2DEmbeddedBytecode(D2D1ShaderProfile.PixelShader40Level91)]
    private readonly partial struct GreenShader
        : ID2D1PixelShader
    {
        public float4 Execute()
        {
            return D2D.GetInput(0).G;
        }
    }

    [D2DInputCount(0)]
    [D2DInputSimple(0)]
    [D2DEmbeddedBytecode(D2D1ShaderProfile.PixelShader40Level91)]
    private readonly partial struct BlueShader
        : ID2D1PixelShader
    {
        public float4 Execute()
        {
            return D2D.GetInput(0).B;
        }
    }

    [D2DInputCount(0)]
    [D2DInputSimple(0)]
    [D2DEmbeddedBytecode(D2D1ShaderProfile.PixelShader40Level91)]
    private readonly partial struct AlphaShader
        : ID2D1PixelShader
    {
        public float4 Execute()
        {
            return D2D.GetInput(0).A;
        }
    }
}

rickbrew avatar Apr 23 '22 17:04 rickbrew

Actually I think I'd need access to non-generic methods for getting the shader bytecode, and setting the const buffer, as well.

It's possible for me to do this with my own methods and some reflection/Activator stuff, but PDN plugin authors may want to do this as well, so it'd be nice to have it in CS.D2D1 proper.

rickbrew avatar Apr 23 '22 17:04 rickbrew

Basically I need polymorphism for ID2DPixelShader :)

rickbrew avatar Apr 24 '22 00:04 rickbrew

I'm not sure I understand the issue here, eg. for getting the bytecode isn't this something you can easily build on top?

For instance:

public interface IPixelShader
{
    ReadOnlyMemory<byte> GetBytecode();

    public sealed class For<T>
        where T : unmanaged, ID2D1PixelShader
    {
        public ReadOnlyMemory<byte> GetBytecode()
        {
            return D2D1InteropServices.LoadShaderBytecode<T>();
        }
    }
}

And then you can just store a bunch of IPixelShader instances and use them to get the bytecode 🤷‍♂️

Same for any other additional members you'd nee polymorphism over, that depend on the shader type. I don't see how this has to be built-in into the D2D1 package, as it seems to me it can easily be created externally 🤔

Sergio0694 avatar Apr 24 '22 14:04 Sergio0694

Yes I can build it on top for my internal use within Paint.NET, and probably will.

But, plugins won't be able to do that, would be nice to have support built-in to CS / CS.D2D1

rickbrew avatar Apr 24 '22 16:04 rickbrew

Rather, they will of course be able to do it themselves. The solution doesn't scale well that way though.

rickbrew avatar Apr 24 '22 16:04 rickbrew

I don't think I'm going to add this to the base ComputeSharp.D2D1 project, as it feels like a more high level abstraction. I for sure wouldn't add this to the interface, and I don't like the idea of a higher level shader type wrapper in this project. I'll likely end up adding something similar in the Win2D-specific packages, since those would then be usable as polymorphic effects for Win2D/Composition interop. Plus, the issue being described here doesn't just apply to the effect id (which is not even something that's directly available from a shader anyway, as it's factory-specific when registering the effect), but to pretty much all exposed values from D2D1PixelShader.

Sergio0694 avatar Aug 20 '22 13:08 Sergio0694

Wait, are you generating a new GUID every time the PSE is registered? Even if it's the same PSE<TShader>, just a different factory?

rickbrew avatar Aug 21 '22 17:08 rickbrew

No, currently it's the same one (just typeof(T).GUID), but we said it's an implementation detail.

Sergio0694 avatar Aug 22 '22 00:08 Sergio0694

Okay. I will need the GUID to be stable, at least for the process lifetime.

The reason is that I have some future plans that may require the ability to "clone" an effect. Which means I'd need to CreateEffect(effectID) and then copy all of the property values over. Which means that the GUID for a particular effect needs to be the same across factories.

One of those future plans is that I'm hoping to add multi-GPU rendering support. This would require recording all of the drawing commands, which I can do with ID2D1CommandList which can stream out to a ID2D1CommandSink. Then I can stream those commands over to another ID2D1DeviceContext, but that also means I need to recreate all of the device resources, including effects, for the 2nd device context. (I'd also have to record all of the calls into the factory, like RegisterEffect)

rickbrew avatar Aug 22 '22 15:08 rickbrew

"Okay. I will need the GUID to be stable, at least for the process lifetime."

Sure, yeah that seems reasonable 🙂 Feel free to open an issue to track that, I can just add documentation for that to clarify that bit.

Sergio0694 avatar Aug 22 '22 16:08 Sergio0694