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

D3D11 video driver #9126

Closed
wants to merge 2 commits into from
Closed

D3D11 video driver #9126

wants to merge 2 commits into from

Conversation

Milek7
Copy link
Contributor

@Milek7 Milek7 commented Apr 28, 2021

Motivation / Problem

OpenGL experience on Windows might be subpar, fresh Windows installations might not have OpenGL drivers installed, etc.

Description

Mostly port of the OpenGL driver.

Limitations

  • Feature level 10_0 required. Might be possible to get lower with slightly more code.

  • Fullscreen not yet implemented.
    Generally DXGI model doesn't play too well with existing code. What win32 does currently, recreating the window for each mode switch is completely unnecessary, in DXGI we can resize/switch modes without recreating window or even swapchain. However to do that requires rework of win32 base driver window creation code. Moreover, we might want to ditch exclusive fullscreen mode entirely and switch to borderless fullscreen windows. On modern compositors exclusive fullscreen have no benefits

  • Exclusive fullscreen is implemented, but on purpose it is not allowed to change system resolution. It can be changed by adding DXGI_SWAP_CHAIN_FLAG_ALLOW_MODE_SWITCH to swapchain creation flags, but I'm not sure if this is desired.

  • I had to align staging texture width to 128 pixels, otherwise pitch gets screwed, I have no idea why. Fixed.

  • I didn't implement sprite cursor drawing.

  • Might not be that necessary after all, as during development of this I found bug that likely caused most of the OpenGL crashes (fixed in Fix: [OpenGL] Main loop expects to start with the video buffer unmapped. #9100)

  • Codestyle might be off.

src/os/windows/comptr.h Outdated Show resolved Hide resolved
@michicc
Copy link
Member

michicc commented Apr 28, 2021

I haven't tried it, but two general comments after a short look through the diff:

A driver error on init should gracefully fall back to another driver, which means all those throws are not good.

Also, the D3DCompiler DLL exists in various versions among OS versions and stuff. Official MS recommendations is the pre-compile shaders with fxc into byte code instead of using the compiler, otherwise you might have to make sure the proper DLL is packaged together with the exe.

@Milek7
Copy link
Contributor Author

Milek7 commented May 15, 2021

A driver error on init should gracefully fall back to another driver, which means all those throws are not good.

I removed all throws, though I don't think they mattered much as all were after context creation (with proper feature level) so failures weren't expected.

Also, the D3DCompiler DLL exists in various versions among OS versions and stuff. Official MS recommendations is the pre-compile shaders with fxc into byte code instead of using the compiler, otherwise you might have to make sure the proper DLL is packaged together with the exe.

I'm not sure about adding build-time dependency on fxc, is it available eg. on MinGW builds?
d3dcompiler 47 is included since win8.1 as a part of system. For now I changed d3dcompiler loading function to search for all the versions down to 40, which should cover any installed directx runtimes since 2008.

I implemented exclusive fullscreen, but on purpose it is not allowed to change system resolution. It can be changed by adding DXGI_SWAP_CHAIN_FLAG_ALLOW_MODE_SWITCH to swapchain creation flags, but I'm not sure if this is desired. (note that recent Windows versions might transform it into fullwindow, so it won't be true exclusive fullscreen anyway.)

@Milek7 Milek7 marked this pull request as ready for review May 15, 2021 00:12
@Milek7 Milek7 force-pushed the d3d11 branch 8 times, most recently from 53f1cf3 to accd0a6 Compare May 15, 2021 13:11
@TrueBrain
Copy link
Member

TrueBrain commented Sep 13, 2023

Now two years later, sadly, no developer has picked this up.

For me myself this is simply because I always think about maintainability of things we add. As OpenTTD always has low resources (in terms of people helping out), adding another driver for which we need to build up knowledge and understanding, is just a lot. For OpenGL alone we saw it was a pretty steep learning curve. I think D3D will be a less steep, but still a learning curve, and I am not able to add that to the load we already have.

As such, I am going to close this PR. Not because it is a bad idea or that the PR is wrong, but simply because we cannot maintain this functionality long-term, and our OpenGL driver seems to be delivering what we were expecting at the moment, for most of the players.

If any developer disagrees, please do reopen, but also please take a proactive stance and make it happen :)

For the author: sorry, but nevertheless, tnx for your work on this. And sorry it took 2 years for a reply.

@TrueBrain TrueBrain closed this Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants