[vlc-devel] [PATCH] Direct3D shader support (HLSL)

[Sasha Koruga]

Hey JB,

>> +#include "hlsl.h"
> What is this hlsl ?

Sorry, that wasn't meant to be in there.

>> +    if(sys->hd3d9x_dll) {
>> +        HRESULT (WINAPI * OurD3DXCreateEffectFromFileA)(
>> +            LPDIRECT3DDEVICE9               pDevice,
>> +            LPCSTR                          pSrcFile,
>> +            CONST D3DXMACRO*                pDefines,
>> +            LPD3DXINCLUDE                   pInclude,
>> +            DWORD                           Flags,
>> +            LPD3DXEFFECTPOOL                pPool,
>> +            LPD3DXEFFECT*                   ppEffect,
>> +            LPD3DXBUFFER*                   ppCompilationErrors);
>> +
>> +        OurD3DXCreateEffectFromFileA =
>> (void*)GetProcAddress(sys->hd3d9x_dll,
>> TEXT("D3DXCreateEffectFromFileA"));
> Why A and not W?

Before W was not needed since "pixelShader.fx" was all ascii, but
you're right; now that path is involved, it should be W. (will change)

>> +            char fileLocation[512];
> MAX_PATH or something?

will fix (is MAX_PATH already declared?)

>
>> +            if(FAILED(hr)) {
>> +               msg_Warn(vd, "D3DXCreateEffectFromFileA Error
>> (hr=0x%lX) -- pixel shading will be disabled (pixelShader.fx possibly
>> missing).", hr);
>> +               if(buffer)
>> +                   msg_Warn(vd, "HLSL Compilation Error: %s",
>> (char*)buffer->lpVtbl->GetBufferPointer(buffer));
>> +               sys->d3dxShader = NULL;
> indentation here seems to be of 3 instead of 4

will fix

>
>> +        const char * shaderStrings[SHADEROPTIONSCOUNT] =
>> {"convertbt", "widen", "detect", "gamma18", "gamma22", "gammabt",
>> "invert", "grayscale"};
> What happens if there are other shaders in the .fx file?

I was thinking of having something like "custom1" "custom2" "custom3",
and then people could fill that in the shader file. If they think
their particular shader code is useful to more people, then they can
submit it.
Sounds good?
(so I'd change it to const char * shaderStrings[SHADEROPTIONSCOUNT] =
{"convertbt", "widen", "detect", "gamma18", "gamma22", "gammabt",
"invert", "grayscale", "custom1","custom2", "custom3"};

>
>> ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->SetBool(((ID3DXEffect*)(sys->d3dxShader)),
>> shaderStrings[i], sys->shaderFXOption[i]);
> Some Macros may make (ID3DXEffect*)(sys->d3dxShader) a bit more readable?

Is it ok to just declare something like [ID3DXEffect* effect =
sys->d3dxShader;] at the beginning of the function? (seems that's what
most ppl do throughout vlc)

Best Regards,
Sasha