Dev Feedback on OVR SDK 0.3.1 Preview
Hey guys, it looks like you're cramming a whole lot of features in to this release! Here's my first impressions with the new Distortion Renderer code in the new SDK:
(Disclaimer: I mostly looked at the D3D9 part of the Distortion Renderer code)
I think that the D3D9 version of WaitUntilGpuIdle() should look like this:
(Disclaimer: I mostly looked at the D3D9 part of the Distortion Renderer code)
- I love the changes in the Distortion Renderer to drastically simplify the pixel shaders by rendering a large mesh. These fragment shaders are *way* faster than the previous ones which tried to compute chromatic correction in the pixel shader.
- It looks to me like the CAPI D3D9 Distortion Renderer code is reeeeeeeeally barebones.
- The WaitUntilGpuIdle and WaitTillTimeAndFlushGpu functions are #ifdef'd out so that they don't do anything.
- The distortion mesh's vertex declaration may have compatibility issues on older video cards. I'd suggest instead of using three D3DDECLUSAGE_POSITION elements, to instead place the TimeWarpFactor and VignetteFactor elements of the ovrDistortionVertex struct into the decl as TEXCOORD elements rather than POSITION.
- I like that Oculus is trying to be like a good middleware citizen by recording and restoring all state before rendering (and then reverting state after rendering). One issue with DistortionRenderer::RecordAndSetState is that IDirect3DDevice9::Get* calls are not supported on D3DCREATE_PURE devices. Since many graphics engines cache render-state at the engine level, I think that a better alternative method to use here would be to add two callback functions that the renderer can implement to get render-states from the state cache (two functions because you may need one for D3DSAMPLERSTATETYPE values and one for D3DRENDERSTATETYPE values).
- Many renderer changes are not being saved and restored at the end. These include Viewport, StreamSource, Vertex Declaration, Index Buffer, Vertex and Pixel Shaders and Shader Constants, and Textures.
- Since you guys are rendering a 2D grid for the Distortion Mesh, I'm almost entirely certain that you could be using a TRIANGLESTRIP (8194 indices required, not counting strip/row restarts) rather than a TRIANGLELIST (24576 indices required). This saves a massive amount of index buffer space and is likely faster to render the distortion mesh.
- Using D3DPOOL_DEFAULT and D3DUSAGE_WRITEONLY on the distortion mesh vertex buffer and index buffer may result in a performance boost in D3D9.
- How did you guys arrive at the conclusion that 8192 triangles per eye was an optimal number for the distortion renderer? Wouldn't larger numbers of triangles result in lower linear interpolation stretch error? Did it just hit the point of diminishing returns as 8192 "looks good enough"?
I think that the D3D9 version of WaitUntilGpuIdle() should look like this:
// Note this is untested code, also I've gotta check the documentation to be sure I'm using the right flags...
LPDIRECT3DQUERY9 waitQuery = NULL;
if (device->CreateQuery(D3DQUERYTYPE_EVENT, &waitQuery) == S_OK && waitQuery != NULL)
{
BOOL done = FALSE;
waitQuery->Issue(D3DISSUE_END);
// GetData will returns S_OK for both done == TRUE or FALSE.
// Exit on failure to avoid infinite loop.
do { }
while(!done &&
!FAILED(query->GetData(&done, sizeof(BOOL), 0)
);
waitQuery->Release();
}