MomentsInGraphics / vulkan_renderer

A toy renderer written in C using Vulkan to perform real-time ray tracing research.
GNU General Public License v3.0
344 stars 24 forks source link

Email cristoph@momentsingraphics.de doesn't exist #1

Closed DaemonDave closed 7 months ago

DaemonDave commented 7 months ago

Hello;

After reading one blog post I will recommend that you read the art of Unix programming as a testament to why Unix was valuable and why the most successful open source software project the Linux kernel is STILL written in C. It ripples through all long living code bases.

https://www.catb.org/~esr/writings/taoup/html/

His followup The Cathedral and The Bazaar is insightful as well.

I was a C++ programmer at General Dynamics before cross converting to C. The Unix rules like single point of truth and single point of failure demonstrates how bloated and opaque C++ can be so coders can write "simple" designs. We made a discovery of C++ flaws with another German open source software called MIRO based on the TAO the ACE Orb as a terribly slow middleware. It was a disaster.

I'm reading your toy Vulkan renderer and have some questions for you:

I wonder if you've used GNU auto tools?

I'm going to convert a version of your Vulkan code toy renderer for GNU auto tools would you like that folder back? I make a habit of converting to GNU auto tools version to avoid encumbrances like cmake and potential downstream proprietary restrictions.

I'm writing this after just a day scanning your code. So please bare with some naivety.

You don't have make recipes for generating .spv files? Why? You don't have xxd headers as well. Why?

Vulkan specified that shaders must be in SPIR-V so why do your shaders end with postfix .glsl? Most Vulkan code I've reviewed ends with .spv.

I read this and have ambiguities with:

"Thus, my renderer uses the GLSL preprocessor extensively, especially in the form of #if. Any selection of a rendering technique corresponds to a bunch of preprocessor defines. They are toggled by the C-code that compiles the shaders at run time by invoking glslangValidator. Some functions are implemented by long chains of #if and #elif directives to implement the same functionality with many different techniques. While that can be slightly cumbersome to read, the same could be said of all other solutions I've encountered thus far. It also means that shaders get recompiled whenever a setting changes. There are so few of them that recompilation is fast and there is no point in caching all possible variants."

So, to be clear, you exploit glsl ifdef conditions to change code inclusions into a final form and THEN you COMPILE for SPIR-V?

So the final code uses op decorators etc. of SPIR-V?

Do you have a logical variable and push constant naming method within shaders so one might alter subpasses and attachments easily?

Do you use threads and or forks in your code? Any non reentrant code?

Do you use OpenCL or other parallel processing?

How involved would it be to include order independent transparency into the shaders arrayed in this code base?

Just another comment:

"Wrappers are pointless"

I doubt you've lived long enough to have written 300,000 LOC. When you deal with up converting code from another code source or a prior version of something like a middleware sometimes as a workaround you wrap code to "shoe horn" old into new. Also for language API like a c++ wrapper around C or C code around FORTRAN like the CBLAS / ATLAS linear algebra suite. I've seen lots of successful wrappers. Software reuse is a goal of mine ( https://leanpub.com/untrappedvalue101 )

Eschew the one true way.

There are millions of lines of code that we could stop rewriting if developers had spent more time making designs rational and clear, especially documentation.

As an aside, I recommend doxygen into all code bases. I will include make recipes for doxygen into whatever I return to you.

Cheers!

Dave Erickson Error Icon Address not found Your message wasn't delivered to cristoph@momentsingraphics.de because the address couldn't be found, or is unable to receive mail. The response from the remote server was: 550 Address invalid (ID:550:1:1 (mi015.mc1.hosteurope.de))

Show quoted text

MomentsInGraphics commented 7 months ago

Next time, try spelling my name correctly ;) .

Regarding your questions:

I haven't read the books you recommended but I appreciate that you like my approach.

I haven't used GNU auto tools. The CMake scripts for this renderer are hardly more than a list of C files to compile/link into an executable, so porting to any other build system should be easy. If you fork my renderer, feel free to make this fork public, but I'll stick to CMake on this repository.

If you run the renderer, you will see lots of settings that you can change at runtime. These turn into defines at runtime here: https://github.com/MomentsInGraphics/vulkan_renderer/blob/main/src/main.c#L752 Shaders are compiled after that has happened. So that's why compilation into SPIR-V is not part of the build scripts. This is research code, so its purpose is to compare lots of different ways to accomplish more or less the same thing. There are very many possible combinations of techniques such that this approach is quite handy. If the renderer were repurposed for something else, it might make more sense to compile into SPIR-V at compile time.

I do not use push constants. There is a single constant buffer for the most important shaders here: https://github.com/MomentsInGraphics/vulkan_renderer/blob/main/src/shaders/shared_constants.glsl Other bindings are mostly defined at the top of the respective shaders. There is not really a convention there. I think for the most part, I assigned binding IDs incrementally as the need for them came up.

There is no multithreading (on the CPU). The whole renderer hardly does any work on the CPU. The purpose of the CPU code is to shovel data from disk to VRAM, prepare the pipeline state objects and then queue some fairly small command buffers each frame. The only thing where CPU-side parallelism may give an appreciable speedup is shader compilation (i.e. launching multiple instances of glslangValidator concurrently).

I do not use OpenCL. Some prior versions of this code base have had compute shaders and I'd prefer those over OpenCL to keep things simple.

The renderer currently uses a visibility buffer, which is hard to combine with OIT. And handling transparent surfaces in real-time ray tracing is also still a bit of an open problem. You could absolutely change the renderer to use an OIT technique but its a bigger change.

What I wrote about wrappers referred primarily to wrappers for graphics APIs. Among graphics devs, it is fairly common to write a wrapper that is supposed to provide a common interface for say Direct3D 12 and Vulkan. But in my opinion that never works out very well.

I've used doxygen in the past and I think my comments use the appropriate syntax for it. I just did not bother setting it up for this project. Looking directly at the comments is good enough for me.

Best wishes, Christoph with an "h" after the "C" ;)

DaemonDave commented 7 months ago

Thanks.

I looked at your name about 6 times and didn't see the extra 'h'. Mea culpa.

Ok, so I won't find any dynamic parallelism inside your glsl? And no compute pipelines, that clarifies how it's scoped. Thanks.

Your code is well documented, and rationally organized which makes it rare amongst open source code. It has a chance to live.

If your code won't make OIT workable I'll probably add another pipeline. Thanks for the warning!

As a second observation, there is an immediate mode GUI called nuklear which would obviate the need for IMGUI and C++. https://github.com/m0ppers/nuklear-glfw-vulkan Battlefield 4 used NUKLEAR as the window system.

Cheers!

Dave

MomentsInGraphics commented 7 months ago

Nothing that deserves the name dynamic parallelism happens on the GPU in this renderer, unless you count triangles of different size causing different numbers of fragment shader invocations in the raster passes. Most of the GPU work happens in a fragment shader for deferred texturing/shading, which uses exactly one thread per pixel.

I hadn't heard of Nuklear yet. I like imgui but it's been bothering me that it's the only reason why I still use C++ in this code base. Maybe I'll give Nuklear a try for my next project.

DaemonDave commented 7 months ago

The reason why so many people end up recoding 60- 80% of code slices that exist in other code bases is because it's so hard or slow to read through code repositories to find it. And developers make it harder. I bet you've never heard of the code chrestomathy? It's explained here: [Uploading book-1.01.pdf…](Untrapped Value 1.01)

Kinda why I started a side project for code reuse in Untrapped Value

DaemonDave commented 7 months ago

book-1.01.pdf