From 8124f5e14aa1d9dd2e76761a838a44edb97bc4b3 Mon Sep 17 00:00:00 2001 From: Henri Verbeet Date: Thu, 11 Dec 2008 11:52:37 +0100 Subject: [PATCH] wined3d: shader_trace_init() shouldn't have side effects. This moves setting baseShader.hex_version and baseShader.functionLength to shader_get_registers_used(), where it's more appropriate. --- dlls/wined3d/baseshader.c | 97 ++++++++++++++++++---------------- dlls/wined3d/pixelshader.c | 94 ++++++++++++++++---------------- dlls/wined3d/vertexshader.c | 5 +- dlls/wined3d/wined3d_private.h | 5 +- 4 files changed, 101 insertions(+), 100 deletions(-) diff --git a/dlls/wined3d/baseshader.c b/dlls/wined3d/baseshader.c index 23dd8f1df68..65dcf2a8fe6 100644 --- a/dlls/wined3d/baseshader.c +++ b/dlls/wined3d/baseshader.c @@ -206,27 +206,28 @@ static void shader_delete_constant_list(struct list* clist) { /* Note that this does not count the loop register * as an address register. */ -HRESULT shader_get_registers_used( - IWineD3DBaseShader *iface, - shader_reg_maps* reg_maps, - semantic* semantics_in, - semantic* semantics_out, - CONST DWORD* pToken, - IWineD3DStateBlockImpl *stateBlock) { - +HRESULT shader_get_registers_used(IWineD3DBaseShader *iface, struct shader_reg_maps *reg_maps, + struct semantic *semantics_in, struct semantic *semantics_out, const DWORD *byte_code, + IWineD3DStateBlockImpl *stateBlock) +{ IWineD3DBaseShaderImpl* This = (IWineD3DBaseShaderImpl*) iface; const SHADER_OPCODE *shader_ins = This->baseShader.shader_ins; DWORD shader_version = This->baseShader.hex_version; unsigned int cur_loop_depth = 0, max_loop_depth = 0; + const DWORD* pToken = byte_code; + char pshader; /* There are some minor differences between pixel and vertex shaders */ - char pshader = shader_is_pshader_version(This->baseShader.hex_version); memset(reg_maps->bumpmat, 0, sizeof(reg_maps->bumpmat)); memset(reg_maps->luminanceparams, 0, sizeof(reg_maps->luminanceparams)); - if (pToken == NULL) + if (!pToken) + { + WARN("Got a NULL pFunction, returning.\n"); + This->baseShader.functionLength = 0; return WINED3D_OK; + } /* get_registers_used is called on every compile on some 1.x shaders, which can result * in stacking up a collection of local constants. Delete the old constants if existing @@ -235,17 +236,23 @@ HRESULT shader_get_registers_used( shader_delete_constant_list(&This->baseShader.constantsB); shader_delete_constant_list(&This->baseShader.constantsI); + /* The version token is supposed to be the first token */ + if (!shader_is_version_token(*pToken)) + { + FIXME("First token is not a version token, invalid shader.\n"); + return WINED3DERR_INVALIDCALL; + } + shader_version = *pToken++; + This->baseShader.hex_version = shader_version; + pshader = shader_is_pshader_version(shader_version); + while (WINED3DVS_END() != *pToken) { CONST SHADER_OPCODE* curOpcode; DWORD opcode_token; - /* Skip version */ - if (shader_is_version_token(*pToken)) { - ++pToken; - continue; - /* Skip comments */ - } else if (shader_is_comment(*pToken)) { + if (shader_is_comment(*pToken)) + { DWORD comment_len = (*pToken & WINED3DSI_COMMENTSIZE_MASK) >> WINED3DSI_COMMENTSIZE_SHIFT; ++pToken; pToken += comment_len; @@ -499,8 +506,11 @@ HRESULT shader_get_registers_used( } } } + ++pToken; reg_maps->loop_depth = max_loop_depth; + This->baseShader.functionLength = ((char *)pToken - (char *)byte_code); + return WINED3D_OK; } @@ -980,38 +990,34 @@ static void shader_dump_ins_modifiers(const DWORD output) FIXME("_unrecognized_modifier(%#x)", mmask >> WINED3DSP_DSTMOD_SHIFT); } -/* First pass: trace shader, initialize length and version */ -void shader_trace_init( - IWineD3DBaseShader *iface, - const DWORD* pFunction) { - - IWineD3DBaseShaderImpl *This =(IWineD3DBaseShaderImpl *)iface; - +void shader_trace_init(const DWORD *pFunction, const SHADER_OPCODE *opcode_table) +{ const DWORD* pToken = pFunction; const SHADER_OPCODE* curOpcode = NULL; + DWORD shader_version; DWORD opcode_token; DWORD i; - TRACE("(%p) : Parsing program\n", This); + TRACE("Parsing %p\n", pFunction); if (!pFunction) { WARN("Got a NULL pFunction, returning.\n"); - This->baseShader.functionLength = 0; /* no Function defined use fixed function vertex processing */ return; } + /* The version token is supposed to be the first token */ + if (!shader_is_version_token(*pToken)) + { + FIXME("First token is not a version token, invalid shader.\n"); + return; + } + shader_version = *pToken++; + TRACE("%s_%u_%u\n", shader_is_pshader_version(shader_version) ? "ps": "vs", + WINED3DSHADER_VERSION_MAJOR(shader_version), WINED3DSHADER_VERSION_MINOR(shader_version)); + while (WINED3DVS_END() != *pToken) { - if (shader_is_version_token(*pToken)) /* version */ - { - This->baseShader.hex_version = *pToken; - TRACE("%s_%u_%u\n", shader_is_pshader_version(This->baseShader.hex_version)? "ps": "vs", - WINED3DSHADER_VERSION_MAJOR(This->baseShader.hex_version), - WINED3DSHADER_VERSION_MINOR(This->baseShader.hex_version)); - ++pToken; - continue; - } if (shader_is_comment(*pToken)) /* comment */ { DWORD comment_len = (*pToken & WINED3DSI_COMMENTSIZE_MASK) >> WINED3DSI_COMMENTSIZE_SHIFT; @@ -1021,13 +1027,13 @@ void shader_trace_init( continue; } opcode_token = *pToken++; - curOpcode = shader_get_opcode(This->baseShader.shader_ins, This->baseShader.hex_version, opcode_token); + curOpcode = shader_get_opcode(opcode_table, shader_version, opcode_token); if (!curOpcode) { int tokens_read; FIXME("Unrecognized opcode: token=0x%08x\n", opcode_token); - tokens_read = shader_skip_unrecognized(pToken, This->baseShader.hex_version); + tokens_read = shader_skip_unrecognized(pToken, shader_version); pToken += tokens_read; } else @@ -1037,10 +1043,10 @@ void shader_trace_init( DWORD usage = *pToken; DWORD param = *(pToken + 1); - shader_dump_decl_usage(usage, param, This->baseShader.hex_version); + shader_dump_decl_usage(usage, param, shader_version); shader_dump_ins_modifiers(param); TRACE(" "); - shader_dump_param(param, 0, 0, This->baseShader.hex_version); + shader_dump_param(param, 0, 0, shader_version); pToken += 2; } else if (curOpcode->opcode == WINED3DSIO_DEF) @@ -1079,7 +1085,7 @@ void shader_trace_init( if (opcode_token & WINED3DSHADER_INSTRUCTION_PREDICATED) { TRACE("("); - shader_dump_param(*(pToken + 2), 0, 1, This->baseShader.hex_version); + shader_dump_param(*(pToken + 2), 0, 1, shader_version); TRACE(") "); } if (opcode_token & WINED3DSI_COISSUE) @@ -1107,7 +1113,7 @@ void shader_trace_init( } } else if (curOpcode->opcode == WINED3DSIO_TEX - && This->baseShader.hex_version >= WINED3DPS_VERSION(2,0) + && shader_version >= WINED3DPS_VERSION(2,0) && (opcode_token & WINED3DSI_TEXLD_PROJECT)) { TRACE("p"); @@ -1116,12 +1122,12 @@ void shader_trace_init( /* Destination token */ if (curOpcode->dst_token) { - tokens_read = shader_get_param(pToken, This->baseShader.hex_version, ¶m, &addr_token); + tokens_read = shader_get_param(pToken, shader_version, ¶m, &addr_token); pToken += tokens_read; shader_dump_ins_modifiers(param); TRACE(" "); - shader_dump_param(param, addr_token, 0, This->baseShader.hex_version); + shader_dump_param(param, addr_token, 0, shader_version); } /* Predication token - already printed out, just skip it */ @@ -1133,19 +1139,16 @@ void shader_trace_init( /* Other source tokens */ for (i = curOpcode->dst_token; i < curOpcode->num_params; ++i) { - tokens_read = shader_get_param(pToken, This->baseShader.hex_version, ¶m, &addr_token); + tokens_read = shader_get_param(pToken, shader_version, ¶m, &addr_token); pToken += tokens_read; TRACE((i == 0)? " " : ", "); - shader_dump_param(param, addr_token, 1, This->baseShader.hex_version); + shader_dump_param(param, addr_token, 1, shader_version); } } TRACE("\n"); } } - ++pToken; - - This->baseShader.functionLength = ((char *)pToken - (char *)pFunction); } void shader_cleanup(IWineD3DBaseShader *iface) diff --git a/dlls/wined3d/pixelshader.c b/dlls/wined3d/pixelshader.c index 969d25a0fed..659a9b40760 100644 --- a/dlls/wined3d/pixelshader.c +++ b/dlls/wined3d/pixelshader.c @@ -316,67 +316,67 @@ static HRESULT WINAPI IWineD3DPixelShaderImpl_SetFunction(IWineD3DPixelShader *i IWineD3DPixelShaderImpl *This =(IWineD3DPixelShaderImpl *)iface; IWineD3DDeviceImpl *deviceImpl = (IWineD3DDeviceImpl *) This->baseShader.device; + unsigned int i, highest_reg_used = 0, num_regs_used = 0; + shader_reg_maps *reg_maps = &This->baseShader.reg_maps; + HRESULT hr; TRACE("(%p) : pFunction %p\n", iface, pFunction); /* First pass: trace shader */ - shader_trace_init((IWineD3DBaseShader*) This, pFunction); - pshader_set_limits(This); + shader_trace_init(pFunction, This->baseShader.shader_ins); /* Initialize immediate constant lists */ list_init(&This->baseShader.constantsF); list_init(&This->baseShader.constantsB); list_init(&This->baseShader.constantsI); - if (WINED3DSHADER_VERSION_MAJOR(This->baseShader.hex_version) > 1) { - shader_reg_maps *reg_maps = &This->baseShader.reg_maps; - HRESULT hr; - unsigned int i, j, highest_reg_used = 0, num_regs_used = 0; + /* Second pass: figure out which registers are used, what the semantics are, etc.. */ + memset(reg_maps, 0, sizeof(shader_reg_maps)); + hr = shader_get_registers_used((IWineD3DBaseShader *)This, reg_maps, + This->semantics_in, NULL, pFunction, deviceImpl->stateBlock); + if (FAILED(hr)) return hr; - /* Second pass: figure out which registers are used, what the semantics are, etc.. */ - memset(reg_maps, 0, sizeof(shader_reg_maps)); - hr = shader_get_registers_used((IWineD3DBaseShader*) This, reg_maps, - This->semantics_in, NULL, pFunction, NULL); - if (FAILED(hr)) return hr; - /* FIXME: validate reg_maps against OpenGL */ + pshader_set_limits(This); - for(i = 0; i < MAX_REG_INPUT; i++) { - if(This->input_reg_used[i]) { - num_regs_used++; - highest_reg_used = i; - } - } - - /* Don't do any register mapping magic if it is not needed, or if we can't - * achieve anything anyway - */ - if(highest_reg_used < (GL_LIMITS(glsl_varyings) / 4) || - num_regs_used > (GL_LIMITS(glsl_varyings) / 4) ) { - if(num_regs_used > (GL_LIMITS(glsl_varyings) / 4)) { - /* This happens with relative addressing. The input mapper function - * warns about this if the higher registers are declared too, so - * don't write a FIXME here - */ - WARN("More varying registers used than supported\n"); - } - - for(i = 0; i < MAX_REG_INPUT; i++) { - This->input_reg_map[i] = i; - } - This->declared_in_count = highest_reg_used + 1; - } else { - j = 0; - for(i = 0; i < MAX_REG_INPUT; i++) { - if(This->input_reg_used[i]) { - This->input_reg_map[i] = j; - j++; - } else { - This->input_reg_map[i] = -1; - } - } - This->declared_in_count = j; + for (i = 0; i < MAX_REG_INPUT; ++i) + { + if (This->input_reg_used[i]) + { + ++num_regs_used; + highest_reg_used = i; } } + + /* Don't do any register mapping magic if it is not needed, or if we can't + * achieve anything anyway */ + if (highest_reg_used < (GL_LIMITS(glsl_varyings) / 4) + || num_regs_used > (GL_LIMITS(glsl_varyings) / 4)) + { + if (num_regs_used > (GL_LIMITS(glsl_varyings) / 4)) + { + /* This happens with relative addressing. The input mapper function + * warns about this if the higher registers are declared too, so + * don't write a FIXME here */ + WARN("More varying registers used than supported\n"); + } + + for (i = 0; i < MAX_REG_INPUT; ++i) + { + This->input_reg_map[i] = i; + } + + This->declared_in_count = highest_reg_used + 1; + } + else + { + This->declared_in_count = 0; + for (i = 0; i < MAX_REG_INPUT; ++i) + { + if (This->input_reg_used[i]) This->input_reg_map[i] = This->declared_in_count++; + else This->input_reg_map[i] = -1; + } + } + This->baseShader.load_local_constsF = FALSE; This->baseShader.shader_mode = deviceImpl->ps_selected_mode; diff --git a/dlls/wined3d/vertexshader.c b/dlls/wined3d/vertexshader.c index 4f872416ee6..434fa056c6f 100644 --- a/dlls/wined3d/vertexshader.c +++ b/dlls/wined3d/vertexshader.c @@ -435,8 +435,7 @@ static HRESULT WINAPI IWineD3DVertexShaderImpl_SetFunction(IWineD3DVertexShader TRACE("(%p) : pFunction %p\n", iface, pFunction); /* First pass: trace shader */ - shader_trace_init((IWineD3DBaseShader*) This, pFunction); - vshader_set_limits(This); + shader_trace_init(pFunction, This->baseShader.shader_ins); /* Initialize immediate constant lists */ list_init(&This->baseShader.constantsF); @@ -451,6 +450,8 @@ static HRESULT WINAPI IWineD3DVertexShaderImpl_SetFunction(IWineD3DVertexShader This->semantics_in, This->semantics_out, pFunction, NULL); if (hr != WINED3D_OK) return hr; + vshader_set_limits(This); + This->baseShader.shader_mode = deviceImpl->vs_selected_mode; if(deviceImpl->vs_selected_mode == SHADER_ARB && diff --git a/dlls/wined3d/wined3d_private.h b/dlls/wined3d/wined3d_private.h index f8e2a8c09bf..5ac771fa7b7 100644 --- a/dlls/wined3d/wined3d_private.h +++ b/dlls/wined3d/wined3d_private.h @@ -2229,6 +2229,7 @@ typedef struct IWineD3DBaseShaderImpl { void shader_buffer_init(struct SHADER_BUFFER *buffer); void shader_buffer_free(struct SHADER_BUFFER *buffer); void shader_cleanup(IWineD3DBaseShader *iface); +void shader_trace_init(const DWORD *byte_code, const SHADER_OPCODE *opcode_table); extern HRESULT shader_get_registers_used( IWineD3DBaseShader *iface, @@ -2241,10 +2242,6 @@ extern HRESULT shader_get_registers_used( extern void shader_generate_main(IWineD3DBaseShader *iface, SHADER_BUFFER *buffer, const shader_reg_maps *reg_maps, const DWORD *pFunction); -extern void shader_trace_init( - IWineD3DBaseShader *iface, - const DWORD* pFunction); - static inline int shader_get_regtype(const DWORD param) { return (((param & WINED3DSP_REGTYPE_MASK) >> WINED3DSP_REGTYPE_SHIFT) | ((param & WINED3DSP_REGTYPE_MASK2) >> WINED3DSP_REGTYPE_SHIFT2));