Re: [PATCH 1/5] d3dcompiler_43/tests: Added HLSL test suite
Am 26.09.2010 um 19:51 schrieb Travis Athougies:
+ data = compute_shader_fullscreen9(device, vshader_passthru, pshader, quad_geometry, + D3DFMT_A8R8G8B8, 1, 1); + + ok(data[0] == D3DCOLOR_ARGB(0, 0, 255, 255), + "swizzle_test: Got color %08x (should be 0x0000FFFF)\n", data[0]); You leak the returned data here.
On the big picture of this, I don't like the void * blob returned from compute_shader_fullscreen. As far as I can see the only position dependent aspect of HLSL is vpos(or rather, the HLSL equivalent). For all others it would be OK to return a ARGB color.
+ /* The Direct3D 9 docs state that we cannot lock a render target surface, + instead we must copy the render target onto this surface to lock it */ I think you can, if you create it with D3DUSAGE_DYNAMIC | D3DUSAGE_RENDERTARGET. If you want the backbuffer to be lockable you'll need some device creation flag whose name I forgot.
On Sun, Sep 26, 2010 at 12:41 PM, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Am 26.09.2010 um 19:51 schrieb Travis Athougies:
+ data = compute_shader_fullscreen9(device, vshader_passthru, pshader, quad_geometry, + D3DFMT_A8R8G8B8, 1, 1); + + ok(data[0] == D3DCOLOR_ARGB(0, 0, 255, 255), + "swizzle_test: Got color %08x (should be 0x0000FFFF)\n", data[0]); You leak the returned data here.
Ok woops. Can't believe I missed that
On the big picture of this, I don't like the void * blob returned from compute_shader_fullscreen. As far as I can see the only position dependent aspect of HLSL is vpos(or rather, the HLSL equivalent). For all others it would be OK to return a ARGB color.
The reason why I don't return just one value here is (a) because the data might either be in DWORD or float format and (b) some tests use more than just 1 pixel, so they need access to all their pixels.
+ /* The Direct3D 9 docs state that we cannot lock a render target surface, + instead we must copy the render target onto this surface to lock it */ I think you can, if you create it with D3DUSAGE_DYNAMIC | D3DUSAGE_RENDERTARGET. If you want the backbuffer to be lockable you'll need some device creation flag whose name I forgot.
There is the lockable boolean argument that I think might work. I will look into using this On Sun, Sep 26, 2010 at 12:41 PM, Stefan Dösinger <stefandoesinger(a)gmx.at> wrote:
Am 26.09.2010 um 19:51 schrieb Travis Athougies:
+ data = compute_shader_fullscreen9(device, vshader_passthru, pshader, quad_geometry, + D3DFMT_A8R8G8B8, 1, 1); + + ok(data[0] == D3DCOLOR_ARGB(0, 0, 255, 255), + "swizzle_test: Got color %08x (should be 0x0000FFFF)\n", data[0]); You leak the returned data here.
On the big picture of this, I don't like the void * blob returned from compute_shader_fullscreen. As far as I can see the only position dependent aspect of HLSL is vpos(or rather, the HLSL equivalent). For all others it would be OK to return a ARGB color.
+ /* The Direct3D 9 docs state that we cannot lock a render target surface, + instead we must copy the render target onto this surface to lock it */ I think you can, if you create it with D3DUSAGE_DYNAMIC | D3DUSAGE_RENDERTARGET. If you want the backbuffer to be lockable you'll need some device creation flag whose name I forgot.
-- Travis Athougies
On 27 September 2010 02:22, Travis Athougies <iammisc(a)gmail.com> wrote:
+ /* The Direct3D 9 docs state that we cannot lock a render target surface, + instead we must copy the render target onto this surface to lock it */ I think you can, if you create it with D3DUSAGE_DYNAMIC | D3DUSAGE_RENDERTARGET. If you want the backbuffer to be lockable you'll need some device creation flag whose name I forgot.
There is the lockable boolean argument that I think might work. I will look into using this
Please don't. I think GetRenderTargetData() is the correct way to do this. Generally you don't want lockable render targets at all, but the case where they're "useful" is for writing data to the render target, not reading. The problem with your current setup is that you do the readback after doing a Present(). Present() invalidates the contents of the backbuffer.
+ ok(data[0] == D3DCOLOR_ARGB(0, 0, 255, 255), + "swizzle_test: Got color %08x (should be 0x0000FFFF)\n", data[0]); While I doubt you're going to care a lot for this specific test, I think it makes sense to integrate something like color_match() from the d3d9 tests right from the start. In fact, I think the way you want to do this is to keep the actual testing inside compile_pixel_shader9(). E.g., you could pass it an array with things like probe locations, expected values, allowed deviations and failure messages.
+ if (caps.PixelShaderVersion >= 0xffff0200) You can just use D3DPS_VERSION(2, 0) here.
Am 27.09.2010 um 12:57 schrieb Henri Verbeet:
On 27 September 2010 02:22, Travis Athougies <iammisc(a)gmail.com> wrote:
+ /* The Direct3D 9 docs state that we cannot lock a render target surface, + instead we must copy the render target onto this surface to lock it */ I think you can, if you create it with D3DUSAGE_DYNAMIC | D3DUSAGE_RENDERTARGET. If you want the backbuffer to be lockable you'll need some device creation flag whose name I forgot.
There is the lockable boolean argument that I think might work. I will look into using this
Please don't. I think GetRenderTargetData() is the correct way to do this. I remember running into problems with this on some windows drivers(*) with surfaces from swapchains, which is why getPixelColor in the d3d9 tests uses StretchRect to a lockable offscreen render target to read the backbuffer. This may however be outdated and apparently doesn't apply to offscreen RTs - the d3d9 tests use GetRenderTargetData on offscreen RTs and I haven't seen an issue.
(*) I think it was my radeon 9000 mobility driver on the old acer travelmate 800.
On Mon, Sep 27, 2010 at 3:57 AM, Henri Verbeet <hverbeet(a)gmail.com> wrote:
On 27 September 2010 02:22, Travis Athougies <iammisc(a)gmail.com> wrote:
+ /* The Direct3D 9 docs state that we cannot lock a render target surface, + instead we must copy the render target onto this surface to lock it */ I think you can, if you create it with D3DUSAGE_DYNAMIC | D3DUSAGE_RENDERTARGET. If you want the backbuffer to be lockable you'll need some device creation flag whose name I forgot.
There is the lockable boolean argument that I think might work. I will look into using this
Please don't. I think GetRenderTargetData() is the correct way to do this. Generally you don't want lockable render targets at all, but the case where they're "useful" is for writing data to the render target, not reading.
This is what I read somewhere, some time ago, so I'll just keep it the same.
The problem with your current setup is that you do the readback after doing a Present(). Present() invalidates the contents of the backbuffer.
+ ok(data[0] == D3DCOLOR_ARGB(0, 0, 255, 255), + "swizzle_test: Got color %08x (should be 0x0000FFFF)\n", data[0]); While I doubt you're going to care a lot for this specific test, I think it makes sense to integrate something like color_match() from the d3d9 tests right from the start. In fact, I think the way you want to do this is to keep the actual testing inside compile_pixel_shader9(). E.g., you could pass it an array with things like probe locations, expected values, allowed deviations and failure messages.
I'm wondering... Inside compile_pixel_shader9? or compute_pixel_shader9? I'm thinking that this might make the argument list of compile_pixel_shader9 really really ugly. I'm unsure of exactly what you're telling me here? Could you be more explicit?
+ if (caps.PixelShaderVersion >= 0xffff0200) You can just use D3DPS_VERSION(2, 0) here.
Uhmm.... Didn't know that.. thanks! -- Travis Athougies
On 29 September 2010 08:21, Travis Athougies <iammisc(a)gmail.com> wrote:
The problem with your current setup is that you do the readback after doing a Present(). Present() invalidates the contents of the backbuffer.
+ ok(data[0] == D3DCOLOR_ARGB(0, 0, 255, 255), + "swizzle_test: Got color %08x (should be 0x0000FFFF)\n", data[0]); While I doubt you're going to care a lot for this specific test, I think it makes sense to integrate something like color_match() from the d3d9 tests right from the start. In fact, I think the way you want to do this is to keep the actual testing inside compile_pixel_shader9(). E.g., you could pass it an array with things like probe locations, expected values, allowed deviations and failure messages.
I'm wondering... Inside compile_pixel_shader9? or compute_pixel_shader9? I'm thinking that this might make the argument list of compile_pixel_shader9 really really ugly. I'm unsure of exactly what you're telling me here? Could you be more explicit?
Sorry, in compute_pixel_shader9(). You'd do something like this: struct test_data { UINT x, y; D3DCOLOR color; UINT max_dev; const char *message; }; struct test_data test_data[] = { {320, 240, D3DCOLOR_ARGB(0x00, 0xff, 0xff, 0x00), 1, "Some failure message"}, ... }; Then pass the test data array to compute_pixel_shader9() which would loop through it and do the actual tests. You'd probably also want to pass __LINE__. Note that this specific structure would be a problem for floating point data though; you'll probably want to use floating point values for your expected data everywhere, and just convert D3DCOLOR result data to floating point inside compute_pixel_shader9().
participants (3)
-
Henri Verbeet -
Stefan Dösinger -
Travis Athougies