Stefan Dösinger wrote:
Pre 2.0 pixel shaders do not contain a tag for the texture type to sample from. Wine had 2D textures hardcoded, this patch checks the bound texture type to decide from which texture to sample from. For >= 2.0 shaders the shader tag is still used.
Why can't this be done in a much cleaner way where the sampler is initialized (2nd pass, baseshader, get_registers_used)? Then you don't have to copy and paste the same thing in many places - there's 10 texture instructions or something like that for pixel shaders alone.
Also, maybe the stateblock should be passed as an argument to all functions that use its states. I don't like walking up the tree to find what you need (this->wineD3Ddevice) - it's a hidden dependency on device state, which isn't even all the time - that's why you had to move the point where the shader is compiled [ this needs better documentation ].
Similarly, I think the code would be cleaner if projected sampling vs regular was retrieved from the stateblock, and stored somewhere else, at the point when the sampler is initialized.
Also, maybe the stateblock should be passed as an argument to all functions that use its states. I don't like walking up the tree to find what you need (this->wineD3Ddevice) - it's a hidden dependency on device state, which isn't even all the time - that's why you had to move the point where the shader is compiled [ this needs better documentation ].
isn't even < valid > all the time Texture dimensions are set in drawprim's upload function.
Am Sonntag 27 August 2006 04:31 schrieb Ivan Gyurdiev:
Also, maybe the stateblock should be passed as an argument to all functions that use its states. I don't like walking up the tree to find what you need (this->wineD3Ddevice) - it's a hidden dependency on device state, which isn't even all the time - that's why you had to move the point where the shader is compiled [ this needs better documentation ].
isn't even < valid > all the time Texture dimensions are set in drawprim's upload function.
Our shaders depend on the values set in the stateblock, which can be different every draw theoretically. I think we should add a fast verficication of the parameters in CompileShader(any better place?) which compares the relevant settings in the stateblock to the values that were used to cross-compile the shader. That should be in a seperate patch though, not in the sampling patch, and it should be done for vertex shaders too(can the declaration change?)
On 27/08/06, Stefan Dösinger stefan@codeweavers.com wrote:
and it should be done for vertex shaders too(can the declaration change?)
Yes, although it is somewhat unlikely.
Am Sonntag 27 August 2006 04:29 schrieb Ivan Gyurdiev:
Stefan Dösinger wrote:
Pre 2.0 pixel shaders do not contain a tag for the texture type to sample from. Wine had 2D textures hardcoded, this patch checks the bound texture type to decide from which texture to sample from. For >= 2.0 shaders the shader tag is still used.
Why can't this be done in a much cleaner way where the sampler is initialized (2nd pass, baseshader, get_registers_used)? Then you don't have to copy and paste the same thing in many places - there's 10 texture instructions or something like that for pixel shaders alone.
Well, I'm pretty new to the shader code, seems like I didn't find the right place for this :-)
If I understand you correctly I should check the texture type from the stateblock around line 300 in baseshader.c:
* Declare 1.X samplers implicitly, based on the destination reg. number */ if (D3DSHADER_VERSION_MAJOR(This->baseShader.hex_version) == 1 && (D3DSIO_TEX == curOpcode->opcode || D3DSIO_TEXBEM == curOpcode->opcode || D3DSIO_TEXM3x2TEX == curOpcode->opcode || D3DSIO_TEXM3x3TEX == curOpcode->opcode)) {
/* Fake sampler usage, only set reserved bit and ttype */ DWORD sampler_code = *pToken & D3DSP_REGNUM_MASK; reg_maps->samplers[sampler_code] = (0x1 << 31) | WINED3DSTT_2D; <--- }
Also, maybe the stateblock should be passed as an argument to all functions that use its states. I don't like walking up the tree to find what you need (this->wineD3Ddevice) - it's a hidden dependency on device state, which isn't even all the time - that's why you had to move the point where the shader is compiled [ this needs better documentation ].
Do you mean adding a pointer to the stateblock in baseShader or copying the entire stateblock? I do not really see any advantage in that, except avoiding the This->wineD3DDevice->stateBlock(would be This->stateBlock instead). I think that is confusing, we should have only one place to access the stateblock(and only one copy of the primary state block)
Similarly, I think the code would be cleaner if projected sampling vs regular was retrieved from the stateblock, and stored somewhere else, at the point when the sampler is initialized.
Do you mean an extra stateblock member for every sampler which stores if projected sampling is used? I do not see any advantage in that, we'd create another redundant copy of the data. a
Stefan Dösinger wrote:
Am Sonntag 27 August 2006 04:29 schrieb Ivan Gyurdiev:
Stefan Dösinger wrote:
Pre 2.0 pixel shaders do not contain a tag for the texture type to sample from. Wine had 2D textures hardcoded, this patch checks the bound texture type to decide from which texture to sample from. For >= 2.0 shaders the shader tag is still used.
Why can't this be done in a much cleaner way where the sampler is initialized (2nd pass, baseshader, get_registers_used)? Then you don't have to copy and paste the same thing in many places - there's 10 texture instructions or something like that for pixel shaders alone.
Well, I'm pretty new to the shader code, seems like I didn't find the right place for this :-)
If I understand you correctly I should check the texture type from the stateblock around line 300 in baseshader.c:
Yes.
Also, maybe the stateblock should be passed as an argument to all functions that use its states. I don't like walking up the tree to find what you need (this->wineD3Ddevice) - it's a hidden dependency on device state, which isn't even all the time - that's why you had to move the point where the shader is compiled [ this needs better documentation ].
Do you mean adding a pointer to the stateblock in baseShader or copying the entire stateblock?
Well, clearly copying the entire stateblock is a bad idea, and unnecessary.
I guess what I was thinking is that the reg_maps structure, which currently stores a sampler DWORD, would instead store a sample structure, and the generation code wouldn't know or care where the data came from originally - wouldn't know anything about the stateblock. The stype would be filled out according to the texture, or the sampler code for post-2.0 shaders, and there would be a projected flag. Then the stateblock could be an argument to the initialization function to make it clear that we're reading data from the stateblock [ so the stateblock better be able to provide that data at that point ].
... on second thought it's probably fine the way you have it - but the stype field should be initialized in one place, and the projected too (should be a boolean flag).
-
H. Verbeet -
Ivan Gyurdiev -
Stefan Dösinger