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

D3D: Add exclusive fullscreen support. #506

Merged
merged 22 commits into from Jul 26, 2014

Conversation

CrossVR
Copy link
Contributor

@CrossVR CrossVR commented Jun 18, 2014

This PR adds support for exclusive fullscreen when "Render to Main Window" is disabled. Please test it out and provide me with some feedback so I can polish things were necessary.

Build available at: https://dl.dolphin-emu.org/prs/pr-506-dolphin-latest-x64.7z

Changes

  • Makes exclusive fullscreen support the default fullscreen mode for D3D.
  • Exits fullscreen when the stop confirmation is shown and exclusive mode is active.
  • Removes CPanel so the separate RenderFrame is now a top-level window with no childs.
  • Adds a borderless fullscreen option in the advanced video configuration to switch back to the old fullscreen behaviour.
  • Removes a redundant hack for 3D Vision.
  • Fixes the D3D renderer choosing the wrong refresh rate upon initialization. It now chooses the appropriate refresh rate for the current desktop resolution.

To-do

  • Restore original window size when switching back from fullscreen.
  • Set fullscreen upon initialization of the backend if it is enabled.
  • Restore exclusive fullscreen when focus is returned after Alt+Tab.
  • Fix deadlock when stopping emulation in fullscreen. (PR Make the emulation stop asynchronous to prevent deadlocks. #515)

Considerations

Will exclusive fullscreen be an option or always on? Would such an option will be the default or optional?

At this moment I consider support for OpenGL or "Render to Main Window" outside of the scope of this PR, unless such support is considered essential before adding this feature.

Fixed issues

Issue 6713 and 7334 should be fixed by this change.

3D vision support is restored in DX11 with a much cleaner implementation which fixes issues 3979, 4726, 4732 and 5990.

It may also help Nvidia Optimus detect Dolphin as an application that requires high performance, can anyone test that?

@LasagnaPie
Copy link

this should fix gsync in D3D

@CrossVR
Copy link
Contributor Author

CrossVR commented Jun 19, 2014

The only thing the previous "3D Vision" option did was enable exclusive fullscreen and apply a hack: https://github.com/dolphin-emu/dolphin/blob/master/Source/Core/VideoCommon/FramebufferManagerBase.cpp#L234

I suspect the hack was necessary due to the hacky implementation of exclusive fullscreen. I'm pretty sure this PR fully restores the 3D Vision functionality in Dolphin.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jun 19, 2014

Currently this PR deadlocks while the GUI thread is waiting for the EmuThread to shut down, which is waiting for the VideoBackend to shut down, which in turn is waiting for the GUI thread to respond.

Having this kind of synchronisation between the GUI thread and the VideoBackend will cause deadlocks when using exclusive fullscreen according to the DXGI documentation.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 10, 2014

@delroth Ready for review, though I could squash 057beaa into 2d2b421 if we want a cleaner history.

Also we need to discuss whether this will be an option or default behaviour.

@delroth
Copy link
Member

delroth commented Jul 10, 2014

The code looks good to me. I'll try to test this later today on Linux to make sure it doesn't break anything.

@JMC47 can you do some testing on Windows? Especially stuff like render to main window, making sure GL/SW aren't broken, etc.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 10, 2014

I think it should definitely be an option, because some people may prefer the old behaviour which is more responsive when you're often switching between windows. I am in favor of turning it on by default though.

Should such an option also affect the "confirm stop" behaviour which was changed in d6e9e13? This commit was necessary, because you might end up with a black screen if you don't fully exit exclusive fullscreen first.

@SirBaron
Copy link

Hey, I try this with 3D Vision, but the full screen results in a small window within a huge black border, I've tried setting internal resolution to 1920x1080, Auto, Aspect Ratio to all of the available settings and unable to get a FULL screen.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 10, 2014

@SirBaron Thanks for reporting that, it must be an issue with the wrong display mode being selected by DXGI when going to fullscreen. I thought I solved it already, I'll provide a better solution tomorrow.

@SirBaron
Copy link

Just incase my wording was poor.

It does go into Exclusive Full screen and trigger the 3D Vision to work, but it's just a tiny portion of the screen is the actual game. This only happens with this build, old builds of Dolphin the screen goes full screen, but that was the DirectX 9 builds.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 10, 2014

@SirBaron I think I understood it correctly the first time. It's selecting a resolution that is too low right? Though most displays would automatically stretch the image.

Are you able to move your cursor into the black border?

@SirBaron
Copy link

No I am unable to move the mouse cursor into the black border.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 10, 2014

Then that confirms that an incorrect display mode is being selected, I will fix it tomorrow.

@SirBaron
Copy link

Hmmm might have been a false bug, deleting a prior profile from nvidia inspector and restarting the PC fixed it.

Sorry! ¬_¬

@@ -44,6 +44,8 @@ static u32 s_LastAA = 0;

static Television s_television;

static bool s_LastFS = false;

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

CrossVR added a commit to CrossVR/dolphin that referenced this pull request Jul 11, 2014
CrossVR added a commit to CrossVR/dolphin that referenced this pull request Jul 11, 2014
@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 11, 2014

I've made changes based on feedback and also fixed two more bugs. I'm now doing some more testing to try and find more issues. After I'm done testing I will make an option in the video settings that allows you to turn this fullscreen behaviour on/off.

@JMC47 Reported that when spamming the fullscreen hotkey there is a race condition that causes the render window to cover the entire screen. Solving this problem would make the hack used in 1aa515a significantly more complex, time would be better invested in trying to get rid of it entirely.

I think it's an acceptable down-side right now, I will do my best to eventually get rid of the hack. I've also included all information I have about the problem in the comments. But I'd rather not make this fullscreen behaviour the default as long as unreliable hacks are still being used.

CrossVR added a commit to CrossVR/dolphin that referenced this pull request Jul 11, 2014
CrossVR added a commit to CrossVR/dolphin that referenced this pull request Jul 11, 2014
@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 11, 2014

Had to do a rebase because of a merge conflict with the current master. Only the commits beginning at 0513457 are not yet reviewed.

Commit 3d25467 was added in the tree to fix a merge conflict because of PR #579.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 11, 2014

@dolphin-emu-bot rebuild

// as soon as the window is resized. However since the timing
// for it is very difficult to get right and would increase
// coupling with the interface, we apply a hack instead.
// http://msdn.microsoft.com/en-us/library/windows/desktop/ee417025%28v=vs.85%29.aspx#full-screen_issues

This comment was marked as off-topic.

This comment was marked as off-topic.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 21, 2014

Addressed one more issue based on tester feedback, still waiting on confirmation that the issue is fixed.

Also have the renderer remember its own fullscreen state. This is done to prevent a case where we exit exclusive fullscreen through the configuration and a focus shift at the same time. In this case the renderer would fail to detect that the fullscreen state was changed.
@delroth
Copy link
Member

delroth commented Jul 25, 2014

@dolphin-emu-bot rebuild

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 25, 2014

All issues outstanding issues have been addressed, ready for review. One commit was added since the branch was last discussed on IRC.

Switching to fullscreen while paused will have the emulator correctly enter fullscreen, but the user won't be able to exit fullscreen since the renderer is unable to tell the UI whether it is safe to do so. In the case where the user is already in exclusive fullscreen switching out of exclusive fullscreen is never safe anyway. Therefore commit 8a221da prevents fullscreen switches in paused state when exclusive fullscreen is enabled to prevent unexpected behaviour.

Allowing the renderer to respond to fullscreen switches while paused would properly fix this issue, however that is outside of the scope of this PR.

ToggleDisplayMode(fullscreen);
if (m_RenderFrame != nullptr)
m_RenderFrame->ShowFullScreen(fullscreen);
if (m_confirmStop)

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

The hack was needed because the Nvidia 3D Vision heuristics are documented to only support surfaces that are the same size as the backbuffer. This would be the case if you enabled the hack and selected the "Auto (Window Size)" internal resolution.

However, on recent drivers the same effect is achieved by selecting the "Auto (Multiple)" internal resolution. Therefore the hack is no longer required.
CrossVR added a commit to CrossVR/dolphin that referenced this pull request Jul 26, 2014
@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 26, 2014

Implemented changes based on feedback. Also amended bd9953d to remove some remnants from the 3D Vision hack.

@CrossVR
Copy link
Contributor Author

CrossVR commented Jul 26, 2014

Fixed a Mac OS X build error, ready for review.

@neobrain
Copy link
Member

@Armada651: This comment grants you the permission to merge this pull request whenever you think it is ready. After addressing the remaining comments, click this link to merge.


LGTM. Feel free to squash whatever you think should be squashed.

@dolphin-emu-bot allowmerge

delroth added a commit that referenced this pull request Jul 26, 2014
D3D: Add exclusive fullscreen support.
@delroth delroth merged commit 8e865f3 into dolphin-emu:master Jul 26, 2014
@CrossVR CrossVR deleted the d3dfullscreen branch July 26, 2014 12:55
MikeRavenelle pushed a commit to MikeRavenelle/dolphin that referenced this pull request Sep 2, 2014
MikeRavenelle pushed a commit to MikeRavenelle/dolphin that referenced this pull request Sep 2, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
8 participants