[vlc-devel] [PATCH] Direct3D shader support (HLSL)
Laurent Aimar
fenrir at elivagar.org
Mon Aug 16 20:35:08 CEST 2010
Hi,
On Sun, Aug 15, 2010 at 09:07:58PM -0700, Sasha Koruga wrote:
> diff --git a/modules/video_output/msw/common.h
> b/modules/video_output/msw/common.h
> index 3a67c7d..e3d2fa0 100644
> --- a/modules/video_output/msw/common.h
> +++ b/modules/video_output/msw/common.h
> @@ -1,7 +1,7 @@
> /*****************************************************************************
> * common.h: Windows video output header file
> *****************************************************************************
> - * Copyright (C) 2001-2009 the VideoLAN team
> + * Copyright (C) 2001-2010 the VideoLAN team
> * $Id$
> *
> * Authors: Gildas Bazin <gbazin at videolan.org>
> @@ -168,6 +168,7 @@ struct vout_display_sys_t
> bool allow_hw_yuv; /* Should we use hardware YUV->RGB conversions */
> /* show video on desktop window ? */
> bool use_desktop;
> + bool *shaderFXOption;
I don't see why shaderFXOption is present. Is it used by another patch?
> struct {
> bool is_fullscreen;
> bool is_on_top;
> @@ -177,6 +178,8 @@ struct vout_display_sys_t
>
> // core objects
> HINSTANCE hd3d9_dll; /* handle of the opened
> d3d9 dll */
> + HINSTANCE hd3d9x_dll; /* handle of the opened
> d3d9x dll */
> + void* d3dxShader;
ID3DXEffect *d3dxShader
would make the code easier to read.
Btw, please do not use camel case variable name (shader_option/d3dx_shader/d3dxshader
or something like that for example, same for following code).
> LPDIRECT3D9 d3dobj;
> LPDIRECT3DDEVICE9 d3ddev;
> D3DPRESENT_PARAMETERS d3dpp;
> diff --git a/modules/video_output/msw/direct3d.c
> b/modules/video_output/msw/direct3d.c
> index 9237c30..4ce7327 100644
> --- a/modules/video_output/msw/direct3d.c
> +++ b/modules/video_output/msw/direct3d.c
> @@ -1,10 +1,11 @@
> /*****************************************************************************
> * direct3d.c: Windows Direct3D video output module
> *****************************************************************************
> - * Copyright (C) 2006-2009 the VideoLAN team
> + * Copyright (C) 2006-2010 the VideoLAN team
> *$Id$
> *
> - * Authors: Damien Fouilleul <damienf at videolan.org>
> + * Authors: Damien Fouilleul <damienf at videolan.org>,
> + * Sasha Koruga <skoruga at gmail.com>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -44,8 +45,16 @@
>
> #include <windows.h>
> #include <d3d9.h>
> +#include <d3dx9.h>
>
> #include "common.h"
> +#include "hlsl.h"
> +
> +#ifndef D3DXFX_NOT_CLONEABLE
> +#define D3DXFX_NOT_CLONEABLE (1 << 11)
> +#endif
Why is it defined here and not in a relevent header?
> +#define SHADEROPTIONSCOUNT 8
>
> /*****************************************************************************
> * Module descriptor
> @@ -456,6 +465,31 @@ static int Direct3DCreate(vout_display_t *vd)
> }
> sys->d3dobj = d3dobj;
>
> + //load shader support
> + for(int i = 43; i > 23; --i) {
> + char buffer[3], filename[14];
> + const char pre[] = "D3dx9_", post[] = ".dll";
> + strncpy(filename, pre, sizeof(pre));
> + strncat(filename, itoa(i,buffer,10), 2);
> + strncat(filename, post, sizeof(post));
> + sys->hd3d9x_dll = LoadLibrary(filename);
char *filename;
if (asprintf(&filename, "D3dx9_%d.dll", i) == -1) {
continue;
}
free(filename);
would be simpler and less error prone.
> + msg_Dbg(vd, "Attempting to load: %s", filename);
> + if(sys->hd3d9x_dll) {
> + msg_Dbg(vd, "loaded: %s", filename);
> + break;
> + }
> + }
> + if(!sys->hd3d9x_dll) {
> + msg_Warn(vd, "cannot load D3dx9d_[24-43].dll; HLSL pixel
> shading will be disabled. (Please install DirectX end-user runtime)");
> + sys->d3dxShader = NULL;
> + sys->shaderFXOption = NULL;
> + }
> + else {
> + sys->shaderFXOption = malloc(sizeof(bool) * SHADEROPTIONSCOUNT);
calloc(SHADEROPTIONSCOUNT, sizeof(*sys->shaderFXOption ));
> + for(unsigned int i = 0; i < SHADEROPTIONSCOUNT; ++i)
unsigned i
is enough (and more consistant, same for following code).
> + sys->shaderFXOption[i] = false;
> + }
> +
Also, declaring bool shaderFXOption[SHADEROPTIONSCOUNT] directly would be simpler.
But I am not sure why/how shaderFXOption is used, so it may depends on another of
your patch.
> /*
> ** Get device capabilities
> */
> @@ -478,13 +512,23 @@ static void Direct3DDestroy(vout_display_t *vd)
> {
> vout_display_sys_t *sys = vd->sys;
>
> - if (sys->d3dobj)
> - IDirect3D9_Release(sys->d3dobj);
> - if (sys->hd3d9_dll)
> + if (sys->d3dobj) {
> + IDirect3D9_Release(sys->d3dobj);
> + sys->d3dobj = NULL;
> + }
> + if (sys->hd3d9_dll) {
> FreeLibrary(sys->hd3d9_dll);
> + sys->hd3d9_dll = NULL;
> + }
> + if (sys->hd3d9x_dll) {
> + FreeLibrary(sys->hd3d9x_dll);
> + sys->hd3d9x_dll = NULL;
> + }
> + if (sys->shaderFXOption) {
> + free(sys->shaderFXOption);
> + sys->shaderFXOption = NULL;
> + }
>
> - sys->d3dobj = NULL;
> - sys->hd3d9_dll = NULL;
I would prefer to let all the = NULL at the end (simpler to read, less lines).
(Same for all occurence if the same reasons applies).
> @@ -589,6 +633,40 @@ static int Direct3DOpen(vout_display_t *vd,
> video_format_t *fmt)
> return VLC_EGENERIC;
> }
>
> + if(sys->hd3d9x_dll) {
> + HRESULT (WINAPI * OurD3DXCreateEffectFromFileA)(
> + LPDIRECT3DDEVICE9 pDevice,
> + LPCSTR pSrcFile,
> + CONST D3DXMACRO* pDefines,
> + LPD3DXINCLUDE pInclude,
> + DWORD Flags,
> + LPD3DXEFFECTPOOL pPool,
> + LPD3DXEFFECT* ppEffect,
> + LPD3DXBUFFER* ppCompilationErrors);
Please, correctly align. No need for the terminal A.
> +
> + OurD3DXCreateEffectFromFileA =
> (void*)GetProcAddress(sys->hd3d9x_dll,
> TEXT("D3DXCreateEffectFromFileA"));
> + if (!OurD3DXCreateEffectFromFileA) {
> + msg_Warn(vd, "Cannot locate reference to
> D3DXCreateEffectFromFileA ABI in DLL; pixel shading will be
> disabled");
> + sys->d3dxShader = NULL;
> + }
> + else {
> + LPD3DXBUFFER buffer = NULL;
> + DWORD dwShaderFlags = D3DXFX_NOT_CLONEABLE;
> + char fileLocation[512];
> + strncpy(fileLocation, config_GetUserDir( VLC_DATA_DIR ), 512);
> + strncat(fileLocation, "\\pixelShader.fx", 512 -
> strlen(fileLocation));
asprintf may also help here.
> + HRESULT hr = OurD3DXCreateEffectFromFileA(sys->d3ddev,
> fileLocation, NULL, NULL, dwShaderFlags, NULL,
> ((ID3DXEffect**)(&sys->d3dxShader)), &buffer);
> + if(FAILED(hr)) //try loading it from current directory as
> oppose to VLC_DATA_DIR
> + hr = OurD3DXCreateEffectFromFileA(sys->d3ddev,
> "pixelShader.fx", NULL, NULL, dwShaderFlags, NULL,
> ((ID3DXEffect**)(&sys->d3dxShader)), &buffer);
I am not sure we want that one.
> + 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;
> + }
> + }
> + }
> +
> /* Change the window title bar text */
> EventThreadUpdateTitle(sys->event, VOUT_TITLE " (Direct3D output)");
>
> @@ -605,10 +683,14 @@ static void Direct3DClose(vout_display_t *vd)
>
> Direct3DDestroyResources(vd);
>
> - if (sys->d3ddev)
> + if(sys->d3dxShader) {
> + ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->Release(sys->d3dxShader);
> + sys->d3dxShader = NULL;
> + }
> + if (sys->d3ddev) {
> IDirect3DDevice9_Release(sys->d3ddev);
> -
> - sys->d3ddev = NULL;
> + sys->d3ddev = NULL;
> + }
> }
>
> /**
> @@ -1035,6 +1117,7 @@ static void Direct3DDestroyScene(vout_display_t *vd)
> */
> static void Direct3DRenderScene(vout_display_t *vd, LPDIRECT3DSURFACE9 surface)
> {
> + unsigned int shaderPasses;
> vout_display_sys_t *sys = vd->sys;
> LPDIRECT3DDEVICE9 d3ddev = sys->d3ddev;
> HRESULT hr;
> @@ -1145,39 +1228,88 @@ static void Direct3DRenderScene(vout_display_t
> *vd, LPDIRECT3DSURFACE9 surface)
> return;
> }
>
> - // Setup our texture. Using textures introduces the texture stage states,
> - // which govern how textures get blended together (in the case of multiple
> - // textures) and lighting information. In this case, we are modulating
> - // (blending) our texture with the diffuse color of the vertices.
> - hr = IDirect3DDevice9_SetTexture(d3ddev, 0,
> (LPDIRECT3DBASETEXTURE9)d3dtex);
> - if (FAILED(hr)) {
> - msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> - IDirect3DDevice9_EndScene(d3ddev);
> - return;
> - }
> + if(((ID3DXEffect*)(sys->d3dxShader)))
if (sys->d3dxShader)
is enougth.
> + {
> + hr = ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->SetTechnique(((ID3DXEffect*)(sys->d3dxShader)),
Defining all the ID3DXEffect_* wrapper (as done for other D3D object like
IDirect3DSurface9) will make the code more readable.
(Same for following code).
> "Shade");
> + if (FAILED(hr))
> + {
> + msg_Err(vd, "SetTechnique failed (hr=0x%lX)", hr);
> + }
>
> - // Render the vertex buffer contents
> - hr = IDirect3DDevice9_SetStreamSource(d3ddev, 0, d3dvtc, 0,
> sizeof(CUSTOMVERTEX));
> - if (FAILED(hr)) {
> - msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> - IDirect3DDevice9_EndScene(d3ddev);
> - return;
> + const char * shaderStrings[SHADEROPTIONSCOUNT] =
> {"convertbt", "widen", "detect", "gamma18", "gamma22", "gammabt",
> "invert", "grayscale"};
> + for(unsigned int i = 0; i < SHADEROPTIONSCOUNT; ++i)
> + {
> + hr =
> ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->SetBool(((ID3DXEffect*)(sys->d3dxShader)),
> shaderStrings[i], sys->shaderFXOption[i]);
> + if (FAILED(hr))
> + {
> + msg_Err(vd, "SetBool #%d failed (hr=0x%lX)", i, hr);
> + }
> + }
> +
> + hr = ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->Begin(((ID3DXEffect*)(sys->d3dxShader)),
> &shaderPasses, 0);
> + if (FAILED(hr))
> + {
> + msg_Err(vd, "Begin failed (hr=0x%lX)", hr);
> + }
> }
> + else
> + shaderPasses = 1;
> +
> + for( UINT uPass = 0; uPass < shaderPasses; ++uPass ) {
> + if(((ID3DXEffect*)(sys->d3dxShader))) {
> + hr =
> ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->BeginPass(((ID3DXEffect*)(sys->d3dxShader)),
> uPass);
> + if (FAILED(hr))
> + msg_Err(vd, "BeginPass failed (hr=0x%lX)", hr);
> + }
> +
> + // Setup our texture. Using textures introduces the texture
> stage states,
> + // which govern how textures get blended together (in the
> case of multiple
> + // textures) and lighting information. In this case, we are modulating
> + // (blending) our texture with the diffuse color of the vertices.
> + hr = IDirect3DDevice9_SetTexture(d3ddev, 0,
> (LPDIRECT3DBASETEXTURE9)d3dtex);
> + if (FAILED(hr)) {
> + msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> + IDirect3DDevice9_EndScene(d3ddev);
> + return;
> + }
> +
> + // Render the vertex buffer contents
> + hr = IDirect3DDevice9_SetStreamSource(d3ddev, 0, d3dvtc, 0,
> sizeof(CUSTOMVERTEX));
> + if (FAILED(hr)) {
> + msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> + IDirect3DDevice9_EndScene(d3ddev);
> + return;
> + }
> +
> + // we use FVF instead of vertex shader
> + hr = IDirect3DDevice9_SetFVF(d3ddev, D3DFVF_CUSTOMVERTEX);
> + if (FAILED(hr)) {
> + msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> + IDirect3DDevice9_EndScene(d3ddev);
> + return;
> + }
> +
> + // draw rectangle
> + hr = IDirect3DDevice9_DrawPrimitive(d3ddev, D3DPT_TRIANGLEFAN, 0, 2);
> + if (FAILED(hr)) {
> + msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> + IDirect3DDevice9_EndScene(d3ddev);
> + return;
> + }
> +
> +
> + if(((ID3DXEffect*)(sys->d3dxShader))) {
> + hr =
> ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->EndPass(((ID3DXEffect*)(sys->d3dxShader)));
> + if (FAILED(hr))
> + msg_Err(vd, "EndPass failed (hr=0x%lX)", hr);
> + }
I don't know how shader works so I can be wrong, but is it really needed to do
all that for each shader?
>
> - // we use FVF instead of vertex shader
> - hr = IDirect3DDevice9_SetFVF(d3ddev, D3DFVF_CUSTOMVERTEX);
> - if (FAILED(hr)) {
> - msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> - IDirect3DDevice9_EndScene(d3ddev);
> - return;
> }
>
> - // draw rectangle
> - hr = IDirect3DDevice9_DrawPrimitive(d3ddev, D3DPT_TRIANGLEFAN, 0, 2);
> - if (FAILED(hr)) {
> - msg_Dbg(vd, "%s:%d (hr=0x%0lX)", __FUNCTION__, __LINE__, hr);
> - IDirect3DDevice9_EndScene(d3ddev);
> - return;
> + if(((ID3DXEffect*)(sys->d3dxShader))) {
> + hr = ((ID3DXEffect*)(sys->d3dxShader))->lpVtbl->End(((ID3DXEffect*)(sys->d3dxShader)));
> + if (FAILED(hr))
> + msg_Err(vd, "End failed (hr=0x%lX)", hr);
> }
The code also needs to be checked for K&R style (consistant with the file you are editing).
I am a bit sceptical about the way shaders are activated/disabled. It seems a bit too
much hardcoded, but that could be changed later if needed.
Regards,
--
fenrir
More information about the vlc-devel
mailing list