[vlc-devel] [PATCH] Direct3D: implement HLSL pixel shading in the rendering pipeline

Felix Abecassis felix.abecassis at gmail.com
Tue Jan 28 13:43:05 CET 2014


2014-01-28 Rafaël Carré <funman at videolan.org>:
> Do you want to add these to mingw-w64 ?
>
> It is very easy and I will gladly guide you through the process.
>
> In fact it might be as easy as pointing it out to jacek (caban) who's
> working
> on DirectX in wine (among other things).
Sure, that would be great.

>
> Can you reorganize the order of functions in the file to not need these?
I wanted to be consistent with the rest of the file but I agree it
adds unnecessary noise at the beginning of the file. I've reorganized
the functions definition as you suggested.

>> +    char *shader_file = Direct3DGetShaderFile();
>
> memleak
Indeed, thanks.

> msg_Err ?
Not using shaders is not a fatal error for the module so I think
msg_Warn should be used.

> nit picking: no need to set sys->d3dx_shader if it's already NULL
I think both way are fine, but still I prefer to do:
if (A)
    Destroy(A);
if (B)
    Destroy(B);
A = NULL;
B = NULL;

Instead of:
if (A) {
    Destroy(A);
    A = NULL;
}
if (B) {
...
Direct3DDestroy() uses the former.

> What's shaderPasses value here?
A technique can have several passes, right now we only have only one
pass in the provided shaders but we could have more. For instance the
grayscale shader:
technique Grayscale
{
    pass p1
    {
        PixelShader = compile ps_2_0 grayscale();
    }
}

>
> hd3d9x_dll is always non NULL
No, see first line :).

>> +static HINSTANCE Direct3DLoadShaderLibrary()
>
> (void)?
I think it's only needed when declaring a prototype.

Thanks for your feedback.

-- 
Félix Abecassis
http://felix.abecassis.me



More information about the vlc-devel mailing list