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

OpenGL backend on windows #168

Open
wants to merge 12 commits into
base: dev
Choose a base branch
from
Open

Conversation

ramenguy99
Copy link

Hi,

I added an OpenGL backend with equal feature support as the D3D11 backend. The code is mostly a one-to-one mapping from the D3D11 backend, but with some minor OpenGL specific differences (e.g. different viewport/scissor/texture coordinates and GLSL shaders) and some bigger ones (e.g. loading functions and win32 context weirdnesses).

The main difference between the two backends is that currently this backend does not support asynchronous upload of textures and buffers. When one of these is allocated, filled or destroyed, a temporary copy is made and the actual operation happens synchronously on the next frame.
This is due to weirdness (especially on windows, not sure about other platforms) with respect to multithreaded operations on the same OpenGL context. I think the best solution would be to have every worker thread create it's own OpenGL context and share OpenGL lists with the main one (e.g. with wglShareLists on windows). I did not implement this, as it requires additional synchronization and more changes throughout the codebase and I was trying to keep the footprint of this change as small as possible.
Obviously this will cause slow-down on large transfers, but it should always result in valid and correct behavior, which is a good starting point for more porting.

My goal is to bring the debugger closer to working natively on Linux, I am currently planning on also adding the following feature (likely in this incremental order):

  • Freetype font backend (on Windows first)
  • Linux OS core and gfx (including cross platform OpenGL support)
  • Bringing everything together to build and run the frontend on Linux

From my understanding currently Linux support is not the highest priority, so feel free to ignore this. If you think it could be useful I would also be happy to maintain this and future changes as a separate fork.

If you know of anyone else currently working on similar things it could be nice to have some place to coordinate this work to speed things up and to avoid redundant PRs with similar goals.

@ratchetfreak
Copy link

Instead of using a user allocated memory arena you could also

create a opengl Buffer,
memory map it,
use the pointer in a non-growable arena shared between client and renderer
And then use glCopyBufferSubData and glTexImage2D (after binding the buffer to GL_PIXEL_UNPACK_BUFFER) to actually update the backend gpu data.

That can save you a copy depending on the driver implementation.

@ramenguy99
Copy link
Author

ramenguy99 commented Feb 28, 2024

Yeah, I guess if the driver actually returns you a pointer to mapped GPU local memory that could be a nice performance improvement. But you probably would still need some fallback if you exhaust the scratch buffer within a frame.

@mmozeiko
Copy link
Contributor

In d3d11 blending is disabled for blur draw calls.

Also I'm very suspicious about ReleaseDC calls. If HWND is not created with CS_OWNDC class style then I have seen GL drivers crash in terrible ways because HGLRC context is messed up if DC is released. I would suggest to keep HDC alive during all lifetime of HGLRC. And do ReleaseDC in r_window_unequip when you know GL context will be gone.

In r_ogl_initialize it seems that you do not need any of gl.BufferData calls, because all those buffers specify their data fully on use.

Both gl.BindBufferBase calls in code seems unnecessary, as you already have same uniform buffer bound to GL_UNIFORM_BUFFER target. Or just replace BindBuffer calls with BindBufferBase.

@ramenguy99
Copy link
Author

ramenguy99 commented Feb 29, 2024

In d3d11 blending is disabled for blur draw calls.

Fixed.

Also I'm very suspicious about ReleaseDC calls. If HWND is not created with CS_OWNDC class style then I have seen GL drivers crash in terrible ways because HGLRC context is messed up if DC is released. I would suggest to keep HDC alive during all lifetime of HGLRC. And do ReleaseDC in r_window_unequip when you know GL context will be gone.

I see. I now save the HDC in the window and free it on unequip as you said. I don't think windows are created with CS_OWNDC right now, should I change that? There could also be an issue with multiple windows. As I currently have only one HGLRC for all of them and call wglMakeCurrent before drawing to one, this means that if the user opens a new window and closes the first one, the original HDC that was used to create the OpenGL context is now released. Could this be a problem?

In r_ogl_initialize it seems that you do not need any of gl.BufferData calls, because all those buffers specify their data fully on use.

True, I got rid of them. I needed those when I was mapping buffers, but I then switched to just specify data on use.

Both gl.BindBufferBase calls in code seems unnecessary, as you already have same uniform buffer bound to GL_UNIFORM_BUFFER target. Or just replace BindBuffer calls with BindBufferBase.

Right, I replaced BindBuffer with BindBufferBase, I think the Base version is needed to specify the binding point, does not work without it for me.

@mmozeiko
Copy link
Contributor

mmozeiko commented Mar 1, 2024

That's a good point about multiple windows - I've never really tested one-gl-context but multi-window solution.
It sounds like same problem with HDC would happen there. If window is gone, the HDC is gone too. Easiest fix probably would be to have a dummy window created on demand and never destroyed. Then for actual windows you would need to only call SetPixelFormat, no need to choose pixel format, can reuse exactly the same pixel format value. As long as it is the same, the HGLRC context will work there.

@ramenguy99
Copy link
Author

Makes sense, I changed this to have a dummy window created on r_init that persists for the whole duration of the application. This also simplifies a bit the initialization logic, and could be useful in the future to create extra OpenGL contexts for worker threads, which would also need some window for binding their context.

It's a bit sad that we now need to create not just one, but two extra windows on startup (one for the old context and one for the persistent modern one), but I guess this is the most robust solution

@mmozeiko
Copy link
Contributor

mmozeiko commented Mar 1, 2024

Don't do multiple contexts on different threads. That is way more unstable solution, drivers have way too many bugs related to that.

Do what ratchet suggests:

  • when main threads wants to upload anything, it allocates staging buffer (BufferData with NULL) and calls glMap on it to get pointer, and puts data to upload + this pointer on queue
  • background threads does whatever processing on data it needs, and calls memcpy into pointer, when finished puts back on "ready" queue
  • main threads ever frame dequeues these "ready" requests, calls glUnmap and copies data to actual buffer for use. Or in case it is texture, it does PBO to an copies buffer to texture.

And if you detect that you're on high enough GL version (4.4) then you can do immutable storage for buffer objects and have buffer mapped just once, and leave it permanently mapped. Pointer will always be valid.

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.

3 participants