Skip to content
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

overlay: in D3D9's doPresent(), use swapchain's backbuffer and dimensions if drawn via IDirect3DSwapChain9::present(). #2367

Conversation

@mkrautz
Copy link
Member

commented Jun 26, 2016

Fixes #1056

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jun 26, 2016

Note: this PR apparently causes the overlay to show up in a lot more windows application than it did previously.

For example, with this, the overlay now shows up in the Battle.net Launcher.

@hacst

This comment has been minimized.

Copy link
Member

commented Jul 1, 2016

Code could use some comments. I have a really hard time piecing together what this does and why it has to be exactly this way.

@mkrautz mkrautz self-assigned this Jul 2, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2016

Agreed. I will add comments.

@mkrautz mkrautz force-pushed the mkrautz:overlay-swapchain-present-use-swapchains-backbuffer branch from bdc8e35 to b342341 Jul 3, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2016

@hacst PTAL

@hacst

This comment has been minimized.

Copy link
Member

commented Jul 3, 2016

Now I get the viewport thing. I'm still not understanding the swap-chain concept well enough to actually review the patch though.

My crude understanding from the MSDN doc currently is:

  • There is always an implicit swap chain associated with a device. This seems to be what we are using in the device(ex) present codepaths.
  • For the code you added to retrieve the backbuffer from the swapchain to make sense we have to have additional swapchains with their own back-buffers we present which usually is a thing used in multi-window applications.
  • For fullscreen apps there can only ever be one swapchain associated with the device handling that fullscreen (though it seems we simultaneously could have multiple other ones not in fullscreen on the same device at the same time).

What I'm still wondering about is:

  • Did I even get the stuff above somewhat coherent? My understanding of the whole d3d pipeline/model is woefully incompletely and fragmented.
  • Do we actually encounter games with multiple swap-chains? What do they use them for and wouldn't we ultimately still want to draw to the back-buffer of the one associated with the device?
  • What does "associated" even mean in that case? Is it always the implicit one, does it change and is there even something like a "special" swap-chain for the device?
  • If we just draw ourselves to any swap-chain back-buffer we can get our hands on will we appear multiple times on the screen of a single application in some situations? (e.g. are additional swap-chains something you would use for "picture-in-picture" functionality)

If you found any more decent information on d3d swap-chains that go over these issues that would also be helpful.

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jul 3, 2016

Apologies for the brain-dumpy nature of my reply, but here goes:

Now I get the viewport thing. I'm still not understanding the swap-chain concept well enough to actually review the patch though.

My crude understanding from the MSDN doc currently is:

  • There is always an implicit swap chain associated with a device. This seems to be what we are using in the device(ex) present codepaths.

Agreed.

  • For the code you added to retrieve the backbuffer from the swapchain to make sense we have to have additional swapchains with their own back-buffers we present which usually is a thing used in multi-window applications.

Yes, this fix is for windowed-mode Final Fantasy XIV.

  • For fullscreen apps there can only ever be one swapchain associated with the device handling that fullscreen (though it seems we simultaneously could have multiple other ones not in fullscreen on the same device at the same time).

Agreed.

What I'm still wondering about is:

  • Did I even get the stuff above somewhat coherent? My understanding of the whole d3d pipeline/model is woefully incompletely and fragmented.

Same, but I think you're right on the money.

  • Do we actually encounter games with multiple swap-chains? What do they use them for and wouldn't we ultimately still want to draw to the back-buffer of the one associated with the device?

More than one extra swap chain? I don't know.

It seems to me that this particular game, Final Fantasy XIV, does this:

  • One implicit swap chain (full screen)
  • One extra swap chain (windowed)

When in windowed mode, only the windowed swap chain is "presented".

...If we were in situation where multiple swap chains were presented,
the overlay would fail badly.

The FPS counter, for example, would count wrong.
Also, if these swap chains had differently sized back buffers, the overlay would constantly resize itself.

The expectation is that the "screen" or "window" has just one size.

  • What does "associated" even mean in that case? Is it always the implicit one, does it change and is there even something like a "special" swap-chain for the device?

The "associated" one, that is, the one you get from ID3D9Device::GetBackBuffer() is, I believe, the back buffer of the
implicit swap chain for the device.

The IDirect3D9SwapChain type only comes into play when additional swap chains have been created.

  • If we just draw ourselves to any swap-chain back-buffer we can get our hands on will we appear multiple times on the screen of a single application in some situations? (e.g. are additional swap-chains something you would use for "picture-in-picture" functionality)

I don't know. I suppose it's possible. (The PIP stuff.)
The back buffer is, after all, just an IDirect3D9Surface.

Though I believe you only have extra swap chains for extra windows.

To be 100% correct, we'd have to handle cases where an application draws to multiple swapchains per frame.

However, that's not feasible without some refactoring of the DevState/Pipe classes.

Right now, the assumption is one overlay per process.

At the very least, it's probably something we should try to track in the overlay.

Draws to multiple swap chains?
Implicit, Additional Swapchain 1, Additional Swapchain 2 -- all drawn to at the same time?
It's possible. But seeing as the overlay currently only allows one "surface" per process,
we should probably stick to the one we were asked to render to first.

Not quite sure about the logic, though.

With that said, this PR is implemented with the following assumption:

  • Only one swapchain used at the same time.

  • Windowed? IDirect3D9SwapChain::present() is used. ("Additional swapchain" is used)

  • Full screen? IDirect3D9Device::present() is used. (Implicit swapchain for IDirect3D9Device is used)

    It's either/or.

If you found any more decent information on d3d swap-chains that go over these issues that would also be helpful.

Only thing I have handy is this:
https://msdn.microsoft.com/en-us/library/windows/desktop/bb147290(v=vs.85).aspx
Found via "See More" from https://msdn.microsoft.com/en-us/library/windows/desktop/bb174354(v=vs.85).aspx

@hacst

This comment has been minimized.

Copy link
Member

commented Jul 4, 2016

Thanks for the clarifications. If you add the assumption bullet points to the patch it LGTM. As long as we manage to restrict what applications we overlay on sufficiently we don't have to be correct for every corner case but if there are assumptions having them be explicit somewhere will be handy if we ever have to revisit them.

overlay: in D3D9's doPresent(), use swapchain's backbuffer and dimens…
…ions if drawn via IDirect3DSwapChain9::present().

Fixes #1056

@mkrautz mkrautz force-pushed the mkrautz:overlay-swapchain-present-use-swapchains-backbuffer branch from b342341 to 7c8b8ab Jul 5, 2016

@mkrautz

This comment has been minimized.

Copy link
Member Author

commented Jul 5, 2016

Added the following:

/// The doPresent function expects the following assumptions to be valid:
///
/// - Only one swap chain used at the same time.
/// - Windowed? IDirect3D9SwapChain::present() is used. ("Additional swap chain" is used)
/// - Full screen? IDirect3D9Device::present() is used. (Implicit swap chain for IDirect3D9Device is used)
///
/// It's either/or.
///
/// If doPresent is called multiple times per frame (say, for different swap chains),
/// the overlay will break badly when DevState::draw() is called. FPS counting will be off,
/// different render targets with different sizes will cause a size-renegotiation with Mumble
/// every frame, etc.

Will land.

@mkrautz mkrautz merged commit 7c8b8ab into mumble-voip:master Jul 5, 2016

mkrautz added a commit that referenced this pull request Jul 5, 2016
Merge PR #2367: overlay: in D3D9's doPresent(), use swapchain's backb…
…uffer and dimensions if drawn via IDirect3DSwapChain9::present().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.