Proposal to solve the overreaching hardware support issue

[VReaperV]

It is no secret that the continued support for ancient fossilised hardware has caused various issues and stalled progress (see e.g: #1567 and #1587). It also seems that such support in the OpenGL renderer isn't going away anytime soon.

As such, I make the following proposal to project head @illwieckz to resolve this problem in a way that would benefit everyone involved:

1. General

This describes the steps towards the creation of a Vulkan-based Daemon derivative that can be readily swapped with it. There are 2 main ways of structuring it in relation to Daemon that I think would work well:

1. Both repositories use a common base. Changes that do not pertain to the renderer or only pertain to the API/ABI would be simply merged from one into the other. This has the potential added benefit that non-ABI/non-API changes could be tested more rapidly in one of the versions, while keeping the other as a more stable backup (see section 3 about the more rapid distribution).
2. Both renderers are loaded as dlls. This might require more work to separate out the renderer parts.

In either case these would be 2 different repositories, because I want to avoid cmake hell and submodules, plus this would be a very clean solution to having renderer-specific issues, since they would use separate issue trackers and therefore would be easy to keep track of. Also, the main repository then wouldn't be receiving constant changes and a barrage of Vulkan-specific issues burying everything else, which are sure to follow for some time after beginning such a development.

Additionally, I would need to separate out some of the code into the shared renderer part. E. g. splitting the lightmapping shader, so there would be one shared part that would just take API-agnostic input, one GL part to process those inputs from how they're stored with GL, and one Vulkan part to process the inputs from how they'd be stored with Vulkan; that is just an example though, the actual split of shared vs data input would need to be pretty sane to avoid a lot of duplication, plus likely some macros or something for things that are used in the middle of the shader code multiple times.

This also would make it a lot more visible, instead of just burying it somewhere in the settings, where it would be easily overlooked, at least after some initial post announcing it once it's in a working condition.

2. Project organisation

I would have the ultimate authority on how the Vulkan renderer is developed. This includes:

1. Deciding if something is to be merged/not merged into or reverted from the Vulkan renderer, the shared renderer parts or elsewhere that would affect the Vulkan renderer (e. g. functions that call into the Vulkan renderer), regardless of reviews/comments/etc.
2. Ignoring the 24-hour merge rule for the Vulkan renderer, the shared renderer parts or elsewhere that would affect the Vulkan renderer (we should just get rid of this rule in general too I think, I've never seen it being useful for anything).
3. Addition, removal, changes of the Vulkan renderer's development direction, supported features, supported hardware.
4. Build system/github/organisational changes on the Vulkan renderer repository.
5. Documentation of the Vulkan renderer, the shared renderer parts or elsewhere that would affect the Vulkan renderer in whatever way I see fit.
6. Changes to Unvanquished and dpks distributed with it that have to do with the Vulkan renderer (e. g. Vulkan-specific settings or presets), which includes deciding on merging/not merging/reverting, ignoring the 24-hour merge rule, documentation, that pertain to the Vulkan renderer.

3. Distribution

The Vulkan-based derivative (name pending) would be distributed alongside Daemon in the updater, as a separate binary. This would make it easy to both avoid disk space pollution by using only one or the other, as well as provide the option of using both to those who would like to have a fallback (in which case both can exist in the same directory).

This would also help with getting bug reports from players, which is an important concern I've seen from people who've hosted Tremulous servers for a long time, because there's a pretty high likelihood that if you asked someone, "Which rendering API are you using?" it would be met with a "wtf is that?", whereas "Are you using Daemon or [Vulkan binary]?" is a lot more straightforward.
That is, of course, something that would more so work with option 1 in section 1.

The updater would allow updating it without requiring an update to Unvanquished/Daemon.

The same binary would also be distributed as part of the universal zip.

I don't know what Daemon does for distributing through package systems, like Flatpak, so I don't yet have anything concrete for that.

4. Versioning

The Vulkan renderer would have a separate versioning system from Daemon, and I would have the ultimate authority on said system (this ties in to section 2.3). This version is to be displayed in the updater as well.

Given @illwieckz's words about wanting experts to lead their areas, which according to various people would be me in this case, I believe this would be a very reasonable way to both do some rapid improvements, test them without waiting for a long time for release, while providing a more stable version at the same time, and backport those changes where possible.

[illwieckz]

Just doing a quick answer because I'm very busy today and tomorrow, so I'll read it more in details and respond more deeply (and likely ask questions 😃️) later. But I thik the global idea goes the right way because yes I want experts to lead their areas and contributors being more autonomous on their own topic. Also for tackling a huge work like doing a Vulkan renderer, we should aim for the faster development rythme possible or this will never succeed. Fortunately having a stable OpenGL renderer means we can be very much relaxed on something like that. So it looks like it's the right time and right landscape to consider such effort.

[slipher]

Although the renderer DLL thing was done both in our own codebase's history (scars remain in ri/re function tables) and separately in ioq3, I don't see what the sense of it was. The DLL makes everything much more difficult since you have to plumb all the engine APIs--filesystem, logging, cvars, clock, and so on--through an interface made for the DLL. I can only imagine they lacked an equivalent of GLEW and so you couldn't build the GL2 renderer on a GL1 system, or if you could you got a dynamic linking error when loading GL2 functions. Meanwhile, ioq3 probably does it because one of its main functions is to be a museum piece, showing Q3 code almost as it originally was. It seems that DLLs are being proposed anew for org chart reasons?

I'm not sure how a separate release cycle would work given the need for an agreed-upon ABI. The client and all rendering DLLs would have to be updated simultaneously on any rendering ABI change.

Anyway, supposing we surmount the challenges of the ABI and managing large bodies of code that needs to be shared between the implementations.... DLLs suck. If we we're going to build the renderer as a separate unit, let's go all the way and make it a separate process. DLLs give you all of the drawbacks and none of the benefits of running in a separate process: memory leaks, memory corruption, and crashes still affect the main thread, but you nonetheless have to redo the plumbing for all facilities like cvars, logging, filesystem, etc. The only good side is being easier to check in a debugger.

Due to the interfaces designed for r_smp, we already have an architecture that is suitable for running the renderer in a separate process: all the instructions are written to a big command buffer that can be executed later. All the FooCmd->ExecuteSelf() stuff could easily be converted to an IPC HandleMsg<FooMsg>. Having the code in a separate process would give us a lot more confidence about parallelism being safe (unlike crashy r_smp) and we could actually let the renderer run in parallel to other tasks by default. The facilities such as cvars, logging, etc. can be provided by the IPC services we already have.

The one thing I don't understand how to do is having the rendering be in a separate process while having the window managed by the main process. But I think browsers do this, with different tabs running and rendering from separate processes, so it must be possible.

[enneract]

DLLs are being proposed because a Vulkan renderer would have a brand new and vastly different architecture.

Instead of doing IPC, I'd go to the other extreme and just use #ifdefs to have two separate Daemon builds with different renderers. A lot less work too, probably.

[VReaperV]

I'm not sure how a separate release cycle would work given the need for an agreed-upon ABI. The client and all rendering DLLs would have to be updated simultaneously on any rendering ABI change.

Those would be releases that don't require compatibility-breaking changes.

If we we're going to build the renderer as a separate unit, let's go all the way and make it a separate process.

That sounds way worse.

DLLs give you all of the drawbacks and none of the benefits of running in a separate process: memory leaks, memory corruption, and crashes still affect the main thread

Hiding memory issues behind process separation is not the solution to them. I also don't get how it's supposed to help with crashes, since at that point the renderer wouldn't be running anymore anyway.

but you nonetheless have to redo the plumbing for all facilities like cvars, logging, filesystem, etc.

Same as with IPC? Except that the IPC implementation is much worse to work with with all of the templated stuff.

The only good side is being easier to check in a debugger.

That is, in fact, a huge advantage.

Due to the interfaces designed for r_smp, we already have an architecture that is suitable for running the renderer in a separate process: all the instructions are written to a big command buffer that can be executed later. All the FooCmd->ExecuteSelf() stuff could easily be converted to an IPC HandleMsg. Having the code in a separate process would give us a lot more confidence about parallelism being safe (unlike crashy r_smp) and we could actually let the renderer run in parallel to other tasks by default. The facilities such as cvars, logging, etc. can be provided by the IPC services we already have.

As was noted earlier by enneract, the architecture would be entirely different. I don't really get why the FooCmd->ExecuteSelf() stuff would be relevant, it's quite likely such commands wouldn't exist at all in the Vulkan renderer.

Instead of doing IPC, I'd go to the other extreme and just use #ifdefs to have two separate Daemon builds with different renderers. A lot less work too, probably.

I like this idea. It's more similar to the proposed option 1 if I understood correctly, which wouldn't have the drawbacks of extra maintenance for interfaces. Having a separate repository is something I'd still want, just to avoid flooding this repo.

[slipher]

DLLs give you all of the drawbacks and none of the benefits of running in a separate process: memory leaks, memory corruption, and crashes still affect the main thread

Hiding memory issues behind process separation is not the solution to them. I also don't get how it's supposed to help with crashes, since at that point the renderer wouldn't be running anymore anyway.

Why not? Works great for gamelogic. Since the proposal is to allow renderer releases without any testing, code review, or other validation, it would be nice to have some fault isolation for that module. A renderer crash would be treated the same as a cgame crash now: drop to the main menu and restart cgame/renderer.

Due to the interfaces designed for r_smp, we already have an architecture that is suitable for running the renderer in a separate process: all the instructions are written to a big command buffer that can be executed later. All the FooCmd->ExecuteSelf() stuff could easily be converted to an IPC HandleMsg. Having the code in a separate process would give us a lot more confidence about parallelism being safe (unlike crashy r_smp) and we could actually let the renderer run in parallel to other tasks by default. The facilities such as cvars, logging, etc. can be provided by the IPC services we already have.

As was noted earlier by enneract, the architecture would be entirely different. I don't really get why the FooCmd->ExecuteSelf() stuff would be relevant, it's quite likely such commands wouldn't exist at all in the Vulkan renderer.

You're right, that suggestion did not make sense. We already have a renderer IPC interface: the cgame->client trap_R_xxx functions. The FooCmd stuff is rather the renderer frontend -> renderer backend interface.

but you nonetheless have to redo the plumbing for all facilities like cvars, logging, filesystem, etc.

Same as with IPC? Except that the IPC implementation is much worse to work with with all of the templated stuff.

Not sure what you mean here. In either case I assume the normal programmer interfaces like Log::Warn, Cvar::SetValue etc. would be used, but backends must be implemented for them which communicate the operations to the main process. The backend could look like our existing IPC ones for a separate process, or some new callback functions for a DLL.

Instead of doing IPC, I'd go to the other extreme and just use #ifdefs to have two separate Daemon builds with different renderers. A lot less work too, probably.

I like this idea. It's more similar to the proposed option 1 if I understood correctly, which wouldn't have the drawbacks of extra maintenance for interfaces. Having a separate repository is something I'd still want, just to avoid flooding this repo.

I don't like the idea that selecting the rendering backend selects a different client as a whole. It would be weird that some non-rendering-related features can appear or disappear based on the renderer settings.

Instead of having a renderer-specific release cycle, why not have separate testing and stable channels, selectable in the updater, for Daemon as a whole? (This has been proposed before.) Then everyone could benefit from faster testing of new features, not only renderer devs. And just make the experimental renderer part of the normal binary. It could work like this: there is a vtable/function table defining the interface for the client to call the renderer, probably consisting of R_Init, R_Shutdown, and all the trap_R_xxx functions. The default renderer fills the table with all the functions we have now. The experimental renderer functions can live in a submodule repository. But the submodule code would be linked into the main binary the same as any other object files. Then there would be no need for special cvar/logging interfaces and the experimental code could freely choose to reuse any functions from the standard renderer or not without any need to formally decide on a shared body of code. Assuming we got nightly builds going, pushing an experimental renderer update just means bumping the submodule.

[illwieckz]

I just kindly remind that I am the one whose response is expected from. So until I get time to properly respond (I hope starting with Friday), most of this discussion is just wasting people's time and attention.

[illwieckz]

I temporary lock the thread to prevent @VReaperV's time to be wasted.
I will unlock the thread when I will be available to respond to @VReaperV.

[illwieckz]

So, I read all the first post! I finally got not time on last Friday, now the deadlines I had are behind me. So I have more time for this discussion.

I have not yet read the answers you got from others, so I'm only answering to your first post for now. I will read the rest later. I hope to have not forgotten anything.

As a start, I want to say I like the idea of a vulkan renderer a lot, especially because not only I like to see the engine getting new techniques as at heart (in fact the Dæmon-name was already the name of the effort to modernize the engine for Trem-derivated games, and it dates from way before the TremZ story and the OpenWolf episode (example in 2009](https://web.archive.org/web/20090822020349/http://unvanquished.net/forum/index.php?topic=242.0)).

Though, our current OpenGL renderer took years to stabilize, and we also need some stability, so there is some opposing forces between that need of some stability and that desire to push things forward.

Also, our OpenGL renderer has now almost reached the level of completeness I want it to have, the last regressions from Quake 3 to take care about to claim having no renderer regression are the dynamic shadows and some Quake 3 specular stuff. Some extra things I would like to see are the linear light blending, the finishing of the PBR code, and some water shader.

I hope I forgot nothing, but that's the things we may see being worked on for the OpenGL renderer in the future, and outside of that, it looks pretty “finished”.

I believe such Vulkan renderer effort solves many problems because:

• It makes possible to take care of some OpenGL renderer stability and use some related development practices for it (like asking for a 1 day review delay before merging some stuff on the OpenGL renderer).
• While having very fast development and more “work in progress” code being done (and acceptable) for Vulkan.
• Not requiring from the Vulkan renderer to have a lage compatibility because we already have an OpenGL renderer for that as a fallback.

Also, I find it wrong if updating the OpenGL renderer makes it only runnable it on devices supporting Vulkan. We better run Vulkan on devices supporting Vulkan than requiring a device that supports Vulkan for running OpenGL.

Also the fact we have a stable OpenGL renderer also makes it easier to ship some bleeding-edge work-in-progress Vulkan renderer, because we can redirect the user to the stable OpenGL renderer if something goes wrong with the Vulkan one.

The OpenGL renderer is not meant to be dropped soon, first because it works, it is stable (and that's now very precious), and on some systems like macOS that's our current only option anyway (and even not latest macOS).

I don't mind if the Vulkan renderer doesn't support some old Vulkan devices, we already have the OpenGL renderer for supporting old devices anyway.

Branch-dedicated forge

The idea of having a separate repository can have some drawbacks but also other benefits and I believe that for such task it is good. For example splitting engine and Unvanquished main repository broughts annoyances related to submodules, but on the other hand offered dedicated issue and pull request list, which is very appreciable. Having a similar mechanism for a specific development on the engine can help, especially when there are two different levels of stability between two components. A rendered being in a submodule would be too painful.

The best solution fo the problem may be a branch-dedicated forge, where a git branch would have its own issue and pull requests list and merge rules etc. But GitHub doesn't propose such feature and I even don't know if that exists in any forge software. So the closest we can get to this is like you said, having a speparate repository.

It can simply be named github.com/DaemonEngine/Daemon-vulkan. The master branch fom this repository would be merged back and forth to github.com/DæmonEngine/Daemon from time to time, like we do with “future” branches like for-0.56.0/sync.

Such Daemon-vulkan repository would have the coding rythm and merging policy you @VReaperV decides to have.
And when merging into Daemon, I guess most of the review would only be to make sure the OpenGL variant is not broken and that the Vulkan code at least builds. Discussing the actual Vulkan implementation would not be expected in PRs merging the Vulkan code from the Daemon-vulkan repository (that would not be the right time and place to do so), such PRs would just be there to make sure it merges and builds and very basic technical details like that.

For convenience, and easy merges between the two branches, we may even keep in sync with a cron task “the other repository's branch” in each repository. So from a repository point of view, one could just create a merge PR to merge the other branch.

Renderer selection

I believe that for now, the best is to propduces different engine binaries, selecting the renderer at build time, with some ifdef compiling statically either the OpenGL renderer or the Vulkan renderer. Doing like this minimizes a lot of work by just reusing the existing infrastructure and scripts to build a static engine with almost no modification.

In th future we may think about turning the vulkan renderer as a loadable component later once stabilized, but that's not for today.

And also, having the updater unpacking a linux-amd64-vulkan.zip to run a daemon-vulkan binary instead of unpacking linux-amd64.zip and running daemon when some “Use Vulkan” checkbox is expected to onlyrequire minimal changes in the updater.

Project organization

So with a github.com/DaemonEngine/Daemon-vulkan repository being like a “branch-dedicated forge” for the Vulkan effort, and the choice of a renderer-selection mechanism by simply having an stable OpenGL binary and a less-stable Vulkan binary, I think I can say i*yes** to this:

I would have the ultimate authority on how the Vulkan renderer is developed. This includes:

1. Deciding if something is to be merged/not merged into or reverted from the Vulkan renderer.
2. Ignoring the 24-hour merge rule for the Vulkan renderer, the shared renderer parts or elsewhere that would affect the Vulkan renderer.
3. Addition, removal, changes of the Vulkan renderer's development direction, supported features, supported hardware.

5. Documentation of the Vulkan renderer, the shared renderer parts or elsewhere that would affect the Vulkan renderer in whatever way I see fit.

Now for:

1. Deciding if something is to be merged/not merged in the shared renderer parts or elsewhere that would affect the Vulkan renderer (e. g. functions that call into the Vulkan renderer), regardless of reviews/comments/etc.

We at least need to make sure that when merging the Vulkan code from time to time, the OpenGL code remains buildable, working, and stable enough, as I just said. Such merge thread would not be to discuss the Vulkan implementation itself as I said.

For the shared part, and if it doesn't break the OpenGL code, I think we better go your way in any case. That's already what I want for your current contributions, as you know what you are doin, and I don't want to slow you down. My condition is that it should not break the OpenGL renderer.

6. Changes to Unvanquished and dpks distributed with it that have to do with the Vulkan renderer (e. g. Vulkan-specific settings or presets), which includes deciding on merging/not merging/reverting, ignoring the 24-hour merge rule, documentation, that pertain to the Vulkan renderer.

I'm not sure what exactly you need to put in DPK files, but I believe it's fine to do that way. Like for shared code with OpenGL, if a file in a DPK happens to be shared with the OpenGL code, we may have more review on this, but even this I don't really believe it would happen that much and in a way it would break the current OpenGL code. I expect that most in-DPK file shared with OpenGL code and Vulkan would be for experimental, disabled by default OpenGL features anyway, so we could go on the fast merge route for them too.

I deeply recommend to put engine in-DPK files in the unvanquished DPK like we already do, to avoid submodule hell. The unvanquished dpk is supposed to provide all the engine needs to start.

I remind that at some point, I would like to see som “daemon” dpk to be embedded as a builtin within the engine itself, like we already do with the GLSL files, implementing that may help updating the in-DPK engine files without touching the Unvanquished repository, and would be perfect for such vulkan engine branch:

This has been discussed there:

• engine code requires some shaders and assets currently shipped as game assets #71

Implementing this is not required to havin you starting working on your Vulkan backend, but this could help.

4. Build system/github/organisational changes on the Vulkan renderer repository.

I'm not sure to get everything you meant on that part.

For such github.com/DaemonEngine/Daemon-vulkan GitHub repository, I think it is fine that you get extensive management permissions on this. I don't remember well what are the options proposed by GitHub, but with reseves from what's actually technically possible on GitHub, I agree that you better be able to do things like creating some team for such repository or things like that.

For the build system, I don't really know what you mean. If you talk about CMake, it would probably a huge waste of time to switch to something else especially if the vulkan flavour would just be the whole daemon binary with some ifdefs.

I expect the CMake plumbing for the Vulkan flavour to just add some USE_DAEMON_VULKAN_RENDERER definition and some different list of source files. You maintaining such source file list would be the best. I can help you on that plumbing if you need, but I expect it to be minimal (just being a different set of source files and some definition and ifdef).

I’ll be happy to help on the CMake code anyway, if that is needed.

You would have to clarify your mind more for me to fully answer on such topic.

Distribution

The Vulkan-based derivative (name pending) would be distributed alongside Daemon in the updater, as a separate binary.

The vulkan engine branch repository will be github.com/DaemonEngine/Daemon-vulkan, engine client executable file name will be daemon-vulkan[.exe], and the engine package built in docker for release purpose will be named <linux>-<amd64>-vulkan.zip.

I can help on setting up the docker part for the release build of the Vulkan engine flavour.

The updater would allow updating it without requiring an update to Unvanquished/Daemon.

Having the updater downloading only one engine would be over-complicated. How the updater works is that it downloads a whole unizip content as torrent and unpacks the engine zip. I would like to keep this with the most minimal changes. But unpacking some linux-amd64-vulkan.zip and running daemon-vulan instead of unpacking linux-amd64.zip and running daemon whn some checkbox to use Vulkan is ticked look both featureful enough and minimal.

Updating the vulkan flavour without requiring to update the base engine is over-complicated. The updater isn't designed for that at all. Having such mechanism was actually discussed for delivering some nightly builds of the engine. Doing a separate vulkan engine update or delivering some nightly engine build in the renderer requires the same very deep work on the updater. So we better implement the nightly-engine download instead. Once we have a way to download a nightly engine, this would also update the engine, whatever the flavour. Such deep updater rework cannot be promised. If implemented by someone, it will be welcome, though:

• Allow option for nightly engine build Unvanquished/updater#127

Note that we are now very close to the point of being able to build nightly builds of the engine, but that's not the same as distributing it.

So for technical reasons, do not hold your breath waiting for a separate update mechanism for a special engine flavour, but contribution on adding generic nightly engine update is welcome.

Also, it would help if we can implement the builtin engine DPK so we only have to implement engine nightly update delivery while allowing us to ship some engine extra in-DPK files.

The same binary would also be distributed as part of the universal zip.

Yes, I remind what does the updater is to download the universal zip content and unpack the engine zip, so it is the way to do it.

We build the Vvulkan client in Docker like other stuff, we ship the vulkan client within the unizip, add some checkbox to the updater to prefer Vulkan, iwhen the checkbox is ticked, the vulkan engine archive is unpacked instead and the vulkan engine is ued instead.

I don't know what Daemon does for distributing through package systems, like Flatpak, so I don't yet have anything concrete for that.

We can delay such topic for later.

Versioning

We already have a versionning mechanism where some components can have their own versions. For example texture dpk and map dpk. So v2.3 tag is for the component while unvanquished/0.55.2 makes sure we have the version from the Unvanquishd release itself.

That said, having a different versioning for the engine is very troublesome. Actually the engine is the only component for which the version should match the game code version (modulo the 3rd number).

So I don't think thats doable for the binary unfortunately. That would be doable for a renderer as a library in a submodule, not for a static binary flavour from a branch and built with some ifdef.

It's fine to add an extra version variable in some src/engine/renderer/vulkan/version.cpp file though and a related cvar to expose it to the client, so the engine or game UI displays something like Dæmon 0.56.0 (Vulkan renderer 1.237). That's the kind of change I expect some shared files or code between OpenGL and Vulkan may exist in the Unvanquished DPK (and never in any engine builtin DPK), like some RML code to display the renderer version, and you would have fast merge policy on such RML code as long as it doesn't break the UI of course.

So it means you can have a static variable and a cvar for the renderer version, different from the engine, with any number you want (just avoid funny inappropriate things like 81680085 😉️). You technically cannot have a vX.X.X tag for it (this is reserved for the Unvanquished/Daemon release version), but you can have some daemon-vulkan/X.X.X tag for the renderer version or something like that.

This version is to be displayed in the updater as well.

For now I see no way to display this unfortunately, neither to implement that easily. For technical reasons, don't hold your breath waiting for that as well.

Extra words

Given @illwieckz's words about wanting experts to lead their areas, which according to various people would be me in this case, I believe this would be a very reasonable way to both do some rapid improvements, test them without waiting for a long time for release, while providing a more stable version at the same time, and backport those changes where possible.

Yes, just you described what I want in general, not just for the Vulkan renderer. Though some things are not ready yet, like us not having yet a nightly build update option in the updater, but even that is even not a new idea.

I want to insist that:

1. I want some experts leading their areas, possible driving a team if there are more than one expert on such areas.
   This project is huge, we can't expect anyone to be able to review every topic we touch, even from the heads. We need some pyarmid of trust with some trusted people in some areas. A bit like the Linux development where there are people being some maintainers or some areas (filesystem, graphics drivers, networking…), with the head relying on the fact what's finally merged for the release was already discussed by such maintainers. We need to not require micro-management from head, to reduce the pressure on reviewers, and make producers more autonomous.
2. I want some faster merge policies for specific areas of the code, like some gameplay changes, or some optional renderer if we have a stable one as backup, things like that.

[VReaperV]

Also, I find it wrong if updating the OpenGL renderer makes it only runnable it on devices supporting Vulkan. We better run Vulkan on devices supporting Vulkan than requiring a device that supports Vulkan for running OpenGL.

I agree, both should be available.

It can simply be named github.com/DaemonEngine/Daemon-vulkan. The master branch fom this repository would be merged back and forth to github.com/DæmonEngine/Daemon from time to time, like we do with “future” branches like for-0.56.0/sync.

Such Daemon-vulkan repository would have the coding rythm and merging policy you @VReaperV decides to have.
And when merging into Daemon, I guess most of the review would only be to make sure the OpenGL variant is not broken and that the Vulkan code at least builds. Discussing the actual Vulkan implementation would not be expected in PRs merging the Vulkan code from the Daemon-vulkan repository (that would not be the right time and place to do so), such PRs would just be there to make sure it merges and builds and very basic technical details like that.

That sounds good. For shader compilation I'll see if I can also setup something later in the CI to make sure all shader permutations build without errors.

It can simply be named github.com/DaemonEngine/Daemon-vulkan. The master branch fom this repository would be merged back and forth to github.com/DæmonEngine/Daemon from time to time, like we do with “future” branches like for-0.56.0/sync.

For convenience, and easy merges between the two branches, we may even keep in sync with a cron task “the other repository's branch” in each repository. So from a repository point of view, one could just create a merge PR to merge the other branch.

This would be ideal, yeah. I find this future-branch like style of updating between the two to be way better than submodules.

We at least need to make sure that when merging the Vulkan code from time to time, the OpenGL code remains buildable, working, and stable enough, as I just said. Such merge thread would not be to discuss the Vulkan implementation itself as I said.

For the shared part, and if it doesn't break the OpenGL code, I think we better go your way in any case. That's already what I want for your current contributions, as you know what you are doin, and I don't want to slow you down. My condition is that it should not break the OpenGL renderer.

Sure, that is reasonable.

I'm not sure what exactly you need to put in DPK files, but I believe it's fine to do that way. Like for shared code with OpenGL, if a file in a DPK happens to be shared with the OpenGL code, we may have more review on this, but even this I don't really believe it would happen that much and in a way it would break the current OpenGL code. I expect that most in-DPK file shared with OpenGL code and Vulkan would be for experimental, disabled by default OpenGL features anyway, so we could go on the fast merge route for them too.

I was thinking of presets, where Vulkan might not have some OpenGL-specific things like r_ext_texture_filter_anisotropic (the extension doesn't exist in Vulkan, and anisotropic filtering is set through other means), or there could be some Vulkan-specific feature that doesn't exist in OpenGL.

I deeply recommend to put engine in-DPK files in the unvanquished DPK like we already do, to avoid submodule hell. The unvanquished dpk is supposed to provide all the engine needs to start.

I remind that at some point, I would like to see som “daemon” dpk to be embedded as a builtin within the engine itself, like we already do with the GLSL files, implementing that may help updating the in-DPK engine files without touching the Unvanquished repository, and would be perfect for such vulkan engine branch:

This has been discussed there:

#71

True. I guess this is where the hard requirement to load Unvanquished base pak comes from?

For the build system, I don't really know what you mean. If you talk about CMake, it would probably a huge waste of time to switch to something else especially if the vulkan flavour would just be the whole daemon binary with some ifdefs.

Ah, no, I mean things like pulling https://github.com/KhronosGroup/glslang into external_deps when such USE_DAEMON_VULKAN_RENDERER is defined.

I expect the CMake plumbing for the Vulkan flavour to just add some USE_DAEMON_VULKAN_RENDERER definition and some different list of source files. You maintaining such source file list would be the best. I can help you on that plumbing if you need, but I expect it to be minimal (just being a different set of source files and some definition and ifdef).

Yeah, I believe it can even be something like:

if ( USE_DAEMON_VULKAN_RENDERER )
    include(src/engine/renderer-vulkan/src.cmake)
else()
    include(src/engine/renderer/src.cmake)
endif()

IIRC that would allow cmake changes to the disabled include without requiring a full rebuild.

The vulkan engine branch repository will be github.com/DaemonEngine/Daemon-vulkan, engine client executable file name will be daemon-vulkan[.exe], and the engine package built in docker for release purpose will be named --vulkan.zip.

I can help on setting up the docker part for the release build of the Vulkan engine flavour.

Yeah, the docker setup would be hugely appreciated. Repo + binary/zip naming sounds good.

Having the updater downloading only one engine would be over-complicated. How the updater works is that it downloads a whole unizip content as torrent and unpacks the engine zip. I would like to keep this with the most minimal changes. But unpacking some linux-amd64-vulkan.zip and running daemon-vulan instead of unpacking linux-amd64.zip and running daemon whn some checkbox to use Vulkan is ticked look both featureful enough and minimal.

Oh, I see, didn't realise it was downloading the whole thing.

Updating the vulkan flavour without requiring to update the base engine is over-complicated. The updater isn't designed for that at all. Having such mechanism was actually discussed for delivering some nightly builds of the engine. Doing a separate vulkan engine update or delivering some nightly engine build in the renderer requires the same very deep work on the updater. So we better implement the nightly-engine download instead. Once we have a way to download a nightly engine, this would also update the engine, whatever the flavour. Such deep updater rework cannot be promised. If implemented by someone, it will be welcome, though:

Unvanquished/updater#127

Yeah, nightly-engine + checkmark should work well.

It's fine to add an extra version variable in some src/engine/renderer/vulkan/version.cpp file though and a related cvar to expose it to the client, so the engine or game UI displays something like Dæmon 0.56.0 (Vulkan renderer 1.237). That's the kind of change I expect some shared files or code between OpenGL and Vulkan may exist in the Unvanquished DPK (and never in any engine builtin DPK), like some RML code to display the renderer version, and you would have fast merge policy on such RML code as long as it doesn't break the UI of course.

That is the kind of version string in the UI I was thinking of, yes. I guess RML can have a conditional there for if Vulkan is used, and if it is - add the Vulkan version there from such cvar.

For now I see no way to display this unfortunately, neither to implement that easily. For technical reasons, don't hold your breath waiting for that as well.

That's fine, just the nightly-engine + checkmark would be hugely helpful.

[slipher]

Is there really some good reason to build two different client binaries? Building a single engine binary and switching renderers with a cvar seems greatly preferable. Separate binaries would force us to write a bunch of extra code in the updater and other release infra. A CMake option to disable the Vulkan renderer would be good of course, to avoid extra dependencies etc., but I see no reason that enabling Vulkan should force one to disable OpenGL. I assume Vulkan would be informed by the GLEW experience and not prevent the application from starting (in OpenGL mode) even if the system lacks minimum Vulkan functionality, CMIIW. We can have a single binary but stuff in a directory or submodule designated for Vulkan-mode-only code would have the special rules of not needing review.

I agree that it makes sense to have a special case for skipping review on Vulkan code since it is an infamously verbose API, whose usage no one will be able to meaningfully critique. I'm glad to forego reading 50k lines of VK API mumbo-jumbo. But I oppose anything outside the Vulkan-mode renderer code being exempt from code review. There is no such thing as "safe" changes, or at least not ones reliably identifiable by developers. For example, the UI drawing code for the non-default BP vampire mode sounds like a safe experimental change. But in fact this code also ran in the default mode and could crash the renderer when doing so. Anything that affects users who did not enable Vulkan mode in any way should be subject to code review. (Some RML code with a conditional does fall into this bucket as the RML is still processed, element objects created, etc.)

[VReaperV]

Somebody might look at this project as if it had two heads.

Is there really some good reason to build two different client binaries?

Yes, the reasons were outlined already.

Building a single engine binary and switching renderers with a cvar seems greatly preferable.

Preferable for what? Increased build times and burying it somewhere in settings?

but I see no reason that enabling Vulkan should force one to disable OpenGL

Why would you want to enable OpenGL if you're using Vulkan, wat.

I assume Vulkan would be informed by the GLEW experience

GLEW doesn't have anything Vulkan-related.

and not prevent the application from starting (in OpenGL mode) even if the system lacks minimum Vulkan functionality

User, by some miracle, finds the Vulkan setting -> engine restarts, falls back on OpenGL -> user thinks that it's running Vulkan, even though it's not, attributing any issues to the Vulkan version.

I'm glad to forego reading 50k lines of VK API mumbo-jumbo

I have doubts that you read the GL spec either.

For example, the UI drawing code for the non-default BP vampire mode sounds like a safe experimental change. But in fact this code also ran in the default mode and could crash the renderer when doing so.

Which you reviewed and approved.

Some RML code with a conditional does fall into this bucket as the RML is still processed, element objects created, etc.

This was already addressed anyway:

Like for shared code with OpenGL, if a file in a DPK happens to be shared with the OpenGL code, we may have more review on this, but even this I don't really believe it would happen that much and in a way it would break the current OpenGL code.

[VReaperV]

Anything that affects users who did not enable Vulkan mode in any way should be subject to code review.

This was addressed as well:

My condition is that it should not break the OpenGL renderer.

[slipher]

Is there really some good reason to build two different client binaries?

Yes, the reasons were outlined already.

Where? I haven't seen any discussion of technical reasons for this. Only the organizational one of possibly making it easier to develop in a fork.

Building a single engine binary and switching renderers with a cvar seems greatly preferable.

Preferable for what? Increased build times and burying it somewhere in settings?

There is no imaginable way that having two flavors of client binaries would improve your build times. Building one binary would be at least as fast for any fair comparison between equal sets of features. (A scheme of ifdefs to build exactly one renderer can be equally well used to build any subset of renderers.) Anyway, to go over the advantages of a unified binary:

• Easy in-game switching between renderers.
• Not adding complications to updater and release infra.

but I see no reason that enabling Vulkan should force one to disable OpenGL

Why would you want to enable OpenGL if you're using Vulkan, wat.

For switching back and forth while testing. For going back to OpenGL if the Vulkan renderer doesn't work for some map. Any other reason that we switch the material system on or off without exiting the program.

I assume Vulkan would be informed by the GLEW experience

GLEW doesn't have anything Vulkan-related.

and not prevent the application from starting (in OpenGL mode) even if the system lacks minimum Vulkan functionality

"Informed by the GLEW experience" does not mean "using GLEW". My hope is that a Vulkan-enabled binary can still run fine in OpenGL mode on a system without good Vulkan support. If that's not the case, there would be a technical reason for using separate binaries or DLLs.

I'm glad to forego reading 50k lines of VK API mumbo-jumbo

I have doubts that you read the GL spec either.

Sure. I would pretend to understand the Daemon renderer architecture reasonably well, but not the OpenGL API.

For example, the UI drawing code for the non-default BP vampire mode sounds like a safe experimental change. But in fact this code also ran in the default mode and could crash the renderer when doing so.

Which you reviewed and approved.

I missed a spot? Darn, guess the whole methodology of code review is invalidated.

Anything that affects users who did not enable Vulkan mode in any way should be subject to code review.

This was addressed as well:

My condition is that it should not break the OpenGL renderer.

We can better ensure that it does not break the OpenGL renderer by giving other contributors the chance to review and test.

[VReaperV]

Where? I haven't seen any discussion of technical reasons for this. Only the organizational one of possibly making it easier to develop in a fork.

Sections 1 and 3 of the proposal and illwieckz's comment.

Also:

Doing like this minimizes a lot of work by just reusing the existing infrastructure and scripts to build a static engine with almost no modification.

Sounds like there aren't many changes required to do that.

There is no imaginable way that having two flavors of client binaries would improve your build times.

It would certainly improve my build times because I'm not insane enough to build both every time.

Building one binary would be at least as fast for any fair comparison between equal sets of features.

Just build the binary separately, what's the problem? Besides, why would you even need to rebuild the GL binary given that most of the features in it are pretty set in stone?

Easy in-game switching between renderers.

Why would you want to switch them in-game? No end-user is gonna bother doing that.

Not adding complications to updater and release infra.

illwieckz seems to be fine with that, so it mustn't be that much of an issue.

For switching back and forth while testing.

Just switch the binaries or run both at the same time?

For going back to OpenGL if the Vulkan renderer doesn't work for some map.

What exactly do you mean by doesn't work? Crashes or features missing?

Any other reason that we switch the material system on or off without exiting the program.

Totally different as it cannot run outside of the existing GL renderer in the first place. Also, I don't know of any reason to toggle it during normal use.

I missed a spot? Darn, guess the whole methodology of code review is invalidated.

Yet that was the only example used.

Anyway, you seem to be arguing with something that the project head agrees with and wrote in-depth about, so I'm not sure what the point of such argument is supposed to be.

[illwieckz]

So to answer @VReaperV:

That sounds good. For shader compilation I'll see if I can also setup something later in the CI to make sure all shader permutations build without errors.

That would be nice! I remember there had been discussions for that about OpenGL as well. Note that on Linux CI we can use some Mesa software renderer (llvmpipe for OpenGL and lavapipe for Vulkan) and actually run the graphical client. We have not implemented that yet in CI but this is doable.

I was thinking of presets, where Vulkan might not have some OpenGL-specific things like r_ext_texture_filter_anisotropic (the extension doesn't exist in Vulkan, and anisotropic filtering is set through other means), or there could be some Vulkan-specific feature that doesn't exist in OpenGL.

We can easily do some presets/opengl/high.cfg and presets/vulkan/high.cfg if needed.

And actually setting a cvar not used by the current renderer (or even not defined) would have no effect any way.

I deeply recommend to put engine in-DPK files in the unvanquished DPK like we already do, to avoid submodule hell. The unvanquished dpk is supposed to provide all the engine needs to start.

True. I guess this is where the hard requirement to load Unvanquished base pak comes from?

Yes. Until we get a way to embed random assets in the engine, assets required by the engine should be stored in Unvanquished/pkg/unvanquished_src.dpkdir. I don't require you to wait a day to merge such engine data things in that dpkdir.

Yeah, I believe it can even be something like:

if ( USE_DAEMON_VULKAN_RENDERER )
    include(src/engine/renderer-vulkan/src.cmake)
else()
    include(src/engine/renderer/src.cmake)

endif()

IIRC that would allow cmake changes to the disabled include without requiring a full rebuild.

Indeed.

Yeah, the docker setup would be hugely appreciated. Repo + binary/zip naming sounds good.

Let's do this.

That is the kind of version string in the UI I was thinking of, yes. I guess RML can have a conditional there for if Vulkan is used, and if it is - add the Vulkan version there from such cvar.

Yes that's doable, If I'm right we have a <if> markup, and in all cases, some C++ glue should be hard to do if needed.

Somebody might look at this project as if it had two heads.

We do what I said.

[illwieckz]

Here are some answers to @slipher:

Is there really some good reason to build two different client binaries? Building a single engine binary and switching renderers with a cvar seems greatly preferable. Separate binaries would force us to write a bunch of extra code in the updater and other release infra.

There are multiple solutions available there, the drawback of the dual-binary solution is the need of extra code in release infra. I'm fine with doing myself that extra effort on the release infra, so I'm OK with that dual-binary solution.

For the updater, I guess it's only a matter of concatenating some string to the engine name before starting it.

For the docker build, it requires some extra code but I'm very confident with that code as I'm now the author of almost everything in it now.

Actually me having added recently the ability to build in docker random engine branches without building the game and actually fetching an game repository that references that engine branch will help testing early “release-like” vulkan engine builds. It's like I already started to write that infrastructure anyway…

I agree that it makes sense to have a special case for skipping review on Vulkan code since it is an infamously verbose API, whose usage no one will be able to meaningfully critique. I'm glad to forego reading 50k lines of VK API mumbo-jumbo. But I oppose anything outside the Vulkan-mode renderer code being exempt from code review. There is no such thing as "safe" changes, or at least not ones reliably identifiable by developers. For example, the UI drawing code for the non-default BP vampire mode sounds like a safe experimental change.

That's why I believe the dual-binary is currently the best solution because we have less change to distribute a broken binary if the experimental vulkan code that gets frenetic 50k lines of VK API mumbo-jumbo rewrites isn't even touched by the compiler at build time to begin with.

Anyway, we're not gods and we have to start with something, even if not the perfect architecture on paper. Maybe one day we will find it better to do in another way, but the dual binary seems to be the right solution for the today problem.

[illwieckz]

I will not discuss more in that thread things that I have already decided for that proposal. Proposals for other architectures can be discussed in dedicated threads that are not this one. Like, if one day we think the Vulkan renderer is mature enough to have the API be a runtime switch, or if we face unsolvable technical problems with the current decided workflow, we would discuss such new proposals in other and new threads.

Now further discussions in that thread are about actually implementing the decided workflow.

[illwieckz]

So, I created the repository:

• https://github.com/DaemonEngine/Daemon-vulkan

The branch is master-vulkan, which is the default one on this repository.

I wrote a small script that pulls every night master from Daemon into Daemon-vulkan and master-vulkan from Daemon-vulkan into Daemon.

So, we can do pull requests to merge master-vulkan into master in Daemon forge, or do pull requests to merge master into master-vulkan in Daemon-vulkan forge, in the same manner we do it for merging back and forth between master and for-0.56.0/sync.

[VReaperV]

Thanks, that is very good. I'll do a pr for the above cmake change to avoid rebuilding both all the time, in a bit (or just push it to the other repo, then we can make use of the script to create the pr automatically - though I'll need to know how to make use of that script).

[illwieckz]

All the script is doing is to mirror the branches across repositories. It is just purposed to make it easy to create a pull request to merge master-vulkan to master, or the other way. The script just makes the branch available “to the other repository”.

So you can start doing work on the vulkan repo, and from time to time we will merge it to master.

[illwieckz]

Here is some vulkan toolchain bits:

• stub vulkan build Daemon-vulkan#1

It introduces the USE_VULKAN CMake option, that set some compiler defines. I flagged with a TODO: comment the parts we need to split in CMakeLists.txt and src.cmake.

That produces a client executable named daemon-vulkan when the Vulkan build is enabled.

I added an r_rendererApi cvar that reports if it is a Vulkan or an OpenGL build, as a demonstration.

[VReaperV]

All the script is doing is to mirror the branches across repositories. It is just purposed to make it easy to create a pull request to merge master-vulkan to master, or the other way. The script just makes the branch available “to the other repository”.

So you can start doing work on the vulkan repo, and from time to time we will merge it to master.

Oh, I see how it works now.