-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Nvidia atw updates #355
base: master
Are you sure you want to change the base?
Nvidia atw updates #355
Conversation
JeroMiya
commented
Nov 20, 2017
- default thread sleep for the ATW Double buffered sample set to 0 instead of 500.
- added log lines to indicate thread ids of the render thread and the ATW thread to help debugging.
- D3DCAPI example now uses double buffering and single render target for both eyes.
- added client context timeout code for D3DCAPI example.
- Added m_vsyncBlockAfterFrameBufferPresent field to D3D base class to control vsync blocking behavior. Used by ATW to block after the framebuffer present of the wrapped RenderManager instance but not before (which is handled by the ATW thread).
- modified constructor params of the wrapped D3D11 RenderManager instance before constructing it and passing it to the D3D11 ATW backend.
- Added a std::condition_variable to the D3D11 ATW backend and use it to block client Present until that frame is presented.
…W backend and some commented out code in the D3D11 ATW double buffer example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are larger blocks of change here than I can do an easy review on. I commented on one change that seems to have made the code less general.
HRESULT hr; | ||
for (size_t i = 0; i < numRenderInfo; i++) { | ||
for (size_t i = 0; i < 2; i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why make this less general? A monocular view onscreen is possible when we use numRenderInfo and not when we use 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 2 here has a different meaning than the numRenderInfo it replaces. It's no longer the number of render info but the number of buffers in the client-side swap chain, which the client controls and which we only support 1 or 2 at the moment.
For reference, these are the only configurations we support at the moment (i.e. for RegisterRenderBuffers):
1 buffer, false for clientWillNotOverwrite: both eyes in one buffer, single buffered (texture copy)
2 buffers, false for clientWillNotOverwrite: one buffer per eye, single buffered (texture copy)
2 buffers, true for clientWillNotOverwrite: both eyes in one buffer, double buffered (NO texture copy)
4 buffers, true for clientWillNotOverwrite: one buffer per eye, double buffered (NO texture copy)
RenderManager currently has no generic way to support all scenarios and so the client has to make assumptions in order to fit into one of the four supported configurations above - e.g. that there are going to be two render infos, each corresponding to an eye. Ideally, the client would request the suggested viewports and buffer sizes and number for a given client preference (one buffer per eye or all eyes in one buffer X double buffered or not double buffered) and then RenderManager would give the client 1) how many buffers to create, 2) what sizes for each buffer, 3) which RenderInfos to render for which buffers and 4) what viewports on THOSE buffers to use (as opposed to the viewports RenderInfo has, which assumes one buffer per eye).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the new code on a DK2 with a GeForce 1080 and it seemed to work the same as the master code, both on the ATW test program that idles between presents and on the HighPoly code that pushes hard on the graphics card with resulting low rendering rate.
If we can separate the changes to the CAPI demo app out, I'm glad to approve the rest of the changes. See my comments on the changes in that app.
static_cast<float>(renderInfo.viewport.width), | ||
static_cast<float>(renderInfo.viewport.height)); | ||
context->RSSetViewports(1, &viewport); | ||
CD3D11_VIEWPORT viewport(static_cast<float>(eye == 0 ? 0 : renderInfo.viewport.width), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be handled by RenderManager, not by the application. The viewport should have been set correctly by the osvrRenderManagerGetRenderInfoFromCollectionD3D11() call.
|
||
// Make a grey background | ||
FLOAT colorRgba[4] = { 0.3f, 0.3f, 0.3f, 1.0f }; | ||
context->ClearRenderTargetView(renderTargetView, colorRgba); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there are two windows for the two eyes, we're going to have to clear both of them here, not just one of them.
RenderView(i, renderInfo[i], renderTargetView, depthStencilView); | ||
} | ||
|
||
// Every other second, we show a black screen to test how |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code to do the black swap was removed, but this comment was not.