Diligent Renderer integration about rbfx HOT 53 CLOSED

Texture3D and Texture2DArray are implemented. I also refactored texture implementation to reduce code size and improve flexibility.

from rbfx.

Given that I would like to drop all existing backends, Mobiles and Web are kinda mandatory.
I am not saying that current GL backend is bad, but it is quite poorly organized and outdated, and it is pulling us back.

from rbfx.

I rebased @rbnpontes work on top of master and formatted & cleaned it up just a bit.
We are getting our first green jobs in CI. So, current diligent branch can be the baseline for further work.

from rbfx.

I think I've dealt with vertex layout, I also refactored uniforms processing a bit.
Now I am trying to get rid of fixed texture slots and just use texture names instead, because this is both something that we wanted to have in the engine and it's more Diligent-friendly.

from rbfx.

Change Texture Units to name based is difficult because Texture Units is heavily tied to Materials, RenderPath and Passes.

I didn't change resources like Material because it's not that important right now, we can just map TextureUnit to name on the material level.
The rest is done in the latest commit.

Also, I noticed that I need to refactor SRB binding cache so it is properly invalidated when texture is e.g. reloaded. I will probably do what I did with pipeline state hashes in Material and VertexBuffer, and add some events for SRB invalidation.

from rbfx.

from rbfx.

I fixed bad input bindings on Vulkan. I still get black screen, but the depth is actually filled with geometry, so it's the issue with output now. Apparently the uniform buffers are all bad in the shaders.

from rbfx.

I don't think I changed cbuffer management beyond your initial implementation, only removed unused code for GLES2 backend.

PS: I will likely change cbuffer management too, I just didn't do it yet.

from rbfx.

All backends now run RenderingShowcase warningless.
Shadows on OpenGL backend are still slightly off, but I will debug it later.

from rbfx.

It's a small change, and I'm afraid that your changes could affect the implementation.

I know that and this is intentional.
First, we need to start doing things the right way, with right architecture.
Then, we restore all missing features (including Compute).
Don't worry about Compute, it is so detached from the rest of the graphics it probably won't be affected much.

from rbfx.

Refactoring plans:

• Continue moving stuff to Urho3D/RenderAPI/. We should not have anything backend-related in Urho3D/Graphics/, basically Urho3D/Graphics/Diligent should become empty. This way we will separate rendering backend from high-level graphic abstractions.
• Add sample for Texture 2D Array and Texture 3D, because they are currently not covered and therefore very hard to test
• Same goes for ComputeDevice.
• Strip as much as possible from Graphics.h/cpp, this class is really overloaded. Maybe remove old renderer at this point? It will not work with Diligent anyway.

from rbfx.

If you have time, you can help by testing and/or patching platforms from top post (whichever are avaiable for you).
I did test what I could, but there cannot be "too much testing" heh. Also, no Mac for me.

Also, you can try to figure out how to enable MoltenVK in our Actions build. You can run Actions in your fork freely to test stuff.

Mac has this error but it has Vulkan enabled:

2023-06-05T17:38:12.5355810Z CMake Warning at Source/ThirdParty/Diligent/Graphics/GraphicsEngineVulkan/CMakeLists.txt:232 (message):
2023-06-05T17:38:12.5458120Z   Vulkan library is not found.  Executables may fail to start because the
2023-06-05T17:38:12.5562200Z   library may not be in rpath.  Define VULKAN_SDK environment or CMake
2023-06-05T17:38:12.5663960Z   variable to specify the SDK path.

iOS has no Vulkan and this error:

Neither VULKAN_SDK nor MOLTENVK_LIBRARY is defined. Vulkan backend will be disabled.

Or maybe this is closer to @rokups in topic...

from rbfx.

Here is some things that we can do

• Complete Shader Computer, there's no implementation. But it's simple, just get currently DeviceContext on GraphicsImpl, create compute pipeline and execute compute. (ALMOST DONE)
• Fix washed rendering on OpenGL
• Fix D3D12 rendering, specifically we must make it run RenderingShowcase without problems.
• Maybe fix some issues on Mobile devices like Android and iOS
• Fix UWP, basically must compile to UWP and see if is rendering pretty well, it's easy
• Implement MSAA
• Finish Texture Resources implementation, I think I haven't finish all texture resources impl, I think that Texture3D is incomplete
• Web Build
• Create unit tests ?

from rbfx.

Great

from rbfx.

We've got our first fully green CI build with Diligent integration. Now I can start actually working on the code.
https://github.com/rbfx/rbfx/actions/runs/4837989758

from rbfx.

Okay, the biggest issue I see is vertex layout. I am not sure how to resolve that.

• Diligent requires input layout to be mapped via vertex element indexes. No position/normal/etc, just 0,1,2...
• Input layout should be provided during pipeline state creation,
• How can we learn layout indexes before pipeline state creation, especially on OpenGL?

Current prototype uses SPIRV-Reflect to peek into vertex shader, but it is not viable release option because mobile/web platforms don't go thru SpirV at all, the build may not even have anything SPIR-V related.

Maybe the easiest way for OpenGL is to quickly parse shader source code for layout(location=x)

from rbfx.

I don't had tested but maybe you can use macros for OpenGL and resolve into runtime.

Ex:

#define POSITION_LOCATION 0
#define COLOR_LOCATION 1

layout(location=POSITION_LOCATION)

On OpenGL and Vulkan environments, even though you won't use vertex input in shader, you still must to declare Input Layout list and macro approach maybe work.
This is not the same to DirectX because unused vertex elements will be removed by the shader

from rbfx.

Great.
Change Texture Units to name based is difficult because Texture Units is heavily tied to Materials, RenderPath and Passes.

from rbfx.

@rbnpontes do you think it's worth having cached SRB, considering the issues with invalidation?
Or we can just do "Set(texture)" on every frame? Did you try this approach?

from rbfx.

Its not recommended because they cause performance losses
You can make some or all resources with dynamic, in this case Diligent expects that this binding can be changed at higher frequency.

At PipelineState building you can pass what resources will be Static, Dynamic and Mutable, you can add resource after pipeline created off course.
On old backends this is not a big problem, but when you deal with modern backends like Vulkan and D3D12 this can be problem.

Usually, i use Static binding in Render Passes, Light Passes and Shadow Passes pipelines because they won't change this resources along the frames.
The same way i recommend to use Static binding for Constant Buffers because they rarely changes.

Generally i use Mutable binding to materials.

Pipeline State is cached, and its good but what about your resources ?
You can reuse pipeline and you can have multiple SRB's, this means that you don't need to create another pipeline just because you have changed the texture, just create a new SRB and add to.
You can add a resource to SRB even though you don't use on shader. ( I made this on DrawCommand, i bind all cbuffers in one SRB).

If you have any question about this model, you can check on the Diligent blog they explain
better why they did use this method http://diligentgraphics.com/2016/03/23/resource-binding-model-in-diligent-engine-2-0/

from rbfx.

Its not recommended because they cause performance losses

Yeah, I understand that this is suboptimal. However, proper SRB invalidation is not an easy feat either. E.g we need to recreate SRB when texture filtering mode changes, or when texture is reloaded, and so on.
So, I wonder if performance loss is acceptable if we just use Mutable bindings for everything.

PS: I may have missed something, but you make all resources mutable, no? The only place where resource type is specified is the line "ci.PSODesc.ResourceLayout.DefaultVariableType = SHADER_RESOURCE_VARIABLE_TYPE_MUTABLE".
So we shouldn't get much from ditching SRB cache?

I think that SRB cache is good thing in general, but the proper implementation is much more complicated than one in your prototype. So maybe we want to postpone that until we deal with more important issues?

from rbfx.

Yeah i made all resources as mutable on pipeline.

I don't see any problems, like i said on D3D11 and OpenGL maybe won't loses too much performance, idk about D3D12 and Vulkan, we need to profile.

PS: PipelineStateCreateInfo contains PSODesc property that contains ResourceLayout property of type PipelineResourceLayoutDesc, this type allows resources to be Static (Default), Mutable or Dynamic

from rbfx.

Good news: shader processing is now like 10 times simpler.
Bad news: I broke Vulkan xD. It cannot bind sampler automatically for whatever reason. We probably need to assign sampler to the texture. Anyway, this is totally solvable, so I won't worry too much.

from rbfx.

Do you have generated GLSL code ?
I was seen this error before

from rbfx.

I had compile your code, its seems the problem is at step to generate universal shader.
Your GLSL conversion to SPIRV code is not including _sampler object, for this reason this error occurs.
You can do it manually by calling ITextureView::SetSampler and it will work, but in this case the whole texture will share unique sampler, this lead to unexpected behaviors in some rbfx samples

from rbfx.

Its work if you force sampler with ITextureView::SetSampler.

But StaticScene sample is not rendering, do you know what it could be ?

PS: I can't inspect this problem on RenderDoc and Nvidia Nsight, they crash at OpaquePass, maybe is a driver bug ? i think is not.
PS(2): I tested with D3D12, D3D11 and OpenGL, it seems okay, this black screen only occurs on Vulkan

from rbfx.

@rbnpontes yeah, it crashes for me too, something's off with inputs heh
However, it crashes only on one of OpaquePass draws, you can still inspect others.

Apparently, the input mapping is off. Vertex shader doesn't receive the position.
I am yet to debug why exactly this happens.

Since I completely changed the way SpirV generates (because I don't want to do GLSL->SPIRV->HLSL->SPIRV for vulkan lol), this kind of issues is sadly expected.

from rbfx.

Great, I will test later

from rbfx.

I fixed bad input bindings on Vulkan. I still get black screen, but the depth is actually filled with geometry, so it's the issue with output now. Apparently the uniform buffers are all bad in the shaders.

Generally this problem occurs on Constant Buffer Manager, do you use Constant Buffer alignment ?
Diligent does not requires cbuffer alignment they do automatically , at least in my tests

from rbfx.

@rbnpontes cbuffers were okay, but shader reflection from Diligent with Vulkan backend doesn't match its documentation heh.
I fixed that. StaticScene now works in some capacity on all 4 backends.

from rbfx.

Great, but what's problem with Reflection ?

from rbfx.

Well, Diligent reflection lied to me and had number of vertex components in NumRows, not NumColumns (like docs said).
So the uniform buffers had only the first element filled in vectors.

from rbfx.

That's why diligent follow the same order of OpenGL ? I mean, on GL matrix columns and rows is not the same way of Directx.
Idk, it seems weird

from rbfx.

I worked on using immutable samplers. I am not 100% happy with that, because we really need to carefully prepare PipelineState objects now, and sadly we create pipeline state objects in multiple places. It needs better incapsulation, maybe... Or at least warnings or even asserts when we don't have immutable sampler for the resource, so we can catch all the places.

from rbfx.

Why not use Immutable Samplers at first moment on Render Passes ?
Scene Passes maybe can use Dynamic samplers until we find a better way to handle this

from rbfx.

Scene Passes maybe can use Dynamic samplers until we find a better way to handle this

Diligent on DX12 backend cannot handle more than 2048 dynamic samplers per frame. Not "unique samplers", just samplers total. If you have 16 sampled textures per shader and you use dynamic samplers, then you can only change resources 128 times per frame... Which is just ridiculously low. Imagine having a limit of 50 materials per frame.

from rbfx.

So, we either treat samplers as part of PipelineState object and process them accordingly, or we face heavy penalties.

from rbfx.

Agreed, i don't see any problem
Caching is here to help us, basically we can trust on PSO Cache.

The basic problem that i see is deal with unused pipelines along the frames

from rbfx.

Great. Just a reminder, please review my Compute PR. It's a small change, and I'm afraid that your changes could affect the implementation. If you have free time, of course.

I created a compute pipeline on ComputeDevice to make this feature work. I'm asking for your review because you may have a better idea of where the compute pipeline should be built.
Some of these changes, such as Samplers, are not included in Compute; they all use dynamic samplers.

from rbfx.

Agreed!

from rbfx.

I was thinking on something.
What do you think about cache Render Passes ?
I mean, we could record all render passes in json, xml or binary. Diligent had a solution for this case, but i think this will won't work very well on rbfx case.

Obviously this can go after diligent integration

from rbfx.

I don't understand some weird issues with Web tho. In RenderingShowcase demo, there is this weird web-only bug:

• Reflective ball in reflection demo scene is not rendered.
• Probe object attached to the camera is rendered fine in main scene and in bike scene, but is not rendered in lightmap demo and reflection demo scenes.

That's really weird, yeah. I have no clue why this specific object is having issues, and why it happens only in Web.

Also, there are a couple of weird opengl errors too in Web.

from rbfx.

This error occurs rarely on legacy.
i had some investigations but i reached dead line without any explanations, its very weird.
If you switch the samples and go back to reflective ball sample it will work.

The only suspects that i have is Render Surfaces

from rbfx.

Okay, we reached another big milestone. The engine works with Diligent backend with green CI and without disabled components. It is still buggy and somewhere messy tho.

from rbfx.

Update: I fixed Android (no Vulkan yet), Web and UWP. See platform matrix in top post.

Sadly, Vulkan on Android crashes (while Diligent samples work) and our API is too messy to bisect what exactly we are doing wrong.

from rbfx.

Log all graphics commands, you can rebuild all commands later and find what goes wrong

from rbfx.

Why not make backend available and move this hard refactoring to another version ?

from rbfx.

from rbfx.

Sure, i fully understand.
Do you need any help ? Maybe add new samples like Compute Shader and Tessellation ?

from rbfx.

Sure.
I can't test iOS right now because i need to pay apple license to test on my devices, maybe next month will buy the license and bring your fixes

About VULKAN_SDK, this errors occurs because environment variables has not been configured correctly.

from rbfx.

I can't test iOS right now because i need to pay apple license to test on my devices, maybe next month will buy the license and bring your fixes

Maybe you shouldn't bother and we will just test it later via C# nuget, because C# deployment for Apple is easier.
It would be awesome even if we just test every platform except iOS. I can personally test Windows, UWP, Android, Web, so you don't need to bother with those if you don't have build tools.

And for Metal support in Mac/iOS GitHub Actions, you can just rely on GitHub and not pay anything heh.

PS: Known issue -- deferred rendering is broken because Diligent does not support read-only depth buffer. I will deal with it later.

from rbfx.

from rbfx.

Unfortunately i can't, build works but run iOS build requires sign, and sign requires a license from apple

from rbfx.