Sign in to follow this  
Hyunkel

DX11 Questions about good coding practices for DirectX (C++)

Recommended Posts

I've been using SlimDX (DX11) for quite some time now and I am fluent in C# and feel quite comfortable coding for DX11.
However, I figured that my lack of C++ skills is unacceptable, so I set out to learn C++.

So far, everything seems quite straightforward and obvious, though I obviously had some difficulties in the beginning.
To test and further develop my C++ skills, I decided to port my current engine, which I've written in C#/SlimDX to C++.
I'm not really experiencing any trouble with porting and solving problems...

... however I'm quite at a loss as to how to produce "good / clean code".

Online reading materials do a great job at explaining programming logic and how to solve problems, but they seem to give no indication on what would be a good or clean way to do so.
Due to this, I feel rather insecure about my current way of coding C++.

Here's a random class as an example on how I'm currently approaching things:

GBuffer.h:
[source]#pragma once
#include "stdafx.h"

// Forward Declarations
class CDeferredRenderer;

class CGBuffer
{
public:
CGBuffer(void);
~CGBuffer(void);

ID3D11Texture2D *GDiffuse;
ID3D11Texture2D *GNormal;
ID3D11Texture2D *GDepth;
ID3D11Texture2D *GLight;
ID3D11RenderTargetView *RTDiffuse;
ID3D11RenderTargetView *RTNormal;
ID3D11RenderTargetView *RTDepth;
ID3D11RenderTargetView *RTLight;
ID3D11ShaderResourceView *GDiffuseView;
ID3D11ShaderResourceView *GNormalView;
ID3D11ShaderResourceView *GDepthView;
ID3D11ShaderResourceView *GLightView;

void Init(CDeferredRenderer * Renderer);
void Clear();
void BeginGeometryStage(ID3D11DepthStencilView *DepthBufferView);
void Release();

private:
CDeferredRenderer *Renderer;
ID3D11Device *Device;
ID3D11DeviceContext *Context;

void CreateGBufferTextures();
void CreateGTexture( DXGI_FORMAT Format, ID3D11Texture2D* & Texture, ID3D11RenderTargetView* & RTView, ID3D11ShaderResourceView* & ResView );
};[/source]


GBuffer.cpp:
[source]#include "StdAfx.h"
#include "GBuffer.h"
#include "DeferredRenderer.h"
#include "Engine.h"
#include "Log.h"
#include "StringParser.h"

CGBuffer::CGBuffer(void)
{
}

CGBuffer::~CGBuffer(void)
{
}

void CGBuffer::Init( CDeferredRenderer * Renderer )
{
this->Renderer = Renderer;
this->Device = Renderer->Device;
this->Context = Renderer->Context;

CreateGBufferTextures();
}

void CGBuffer::CreateGBufferTextures()
{
CreateGTexture(DXGI_FORMAT_R8G8B8A8_UNORM, GDiffuse, RTDiffuse, GDiffuseView);
CreateGTexture(DXGI_FORMAT_R16G16_UNORM, GNormal, RTNormal, GNormalView);
CreateGTexture(DXGI_FORMAT_R32_FLOAT, GDepth, RTDepth, GDepthView);
CreateGTexture(DXGI_FORMAT_R16G16B16A16_FLOAT, GLight, RTLight, GLightView);
}

void CGBuffer::CreateGTexture( DXGI_FORMAT Format, ID3D11Texture2D* & Texture, ID3D11RenderTargetView* & RTView, ID3D11ShaderResourceView* & ResView )
{
// Create Texture
D3D11_TEXTURE2D_DESC TextureDesc;
ZeroMemory(&TextureDesc, sizeof(D3D11_TEXTURE2D_DESC));

TextureDesc.Width = SCREEN_WIDTH;
TextureDesc.Height = SCREEN_HEIGHT;
TextureDesc.MipLevels = 1;
TextureDesc.ArraySize = 1;
TextureDesc.Format = Format;
TextureDesc.SampleDesc.Count = 1;
TextureDesc.SampleDesc.Quality = 0;
TextureDesc.Usage = D3D11_USAGE_DEFAULT;
TextureDesc.BindFlags = D3D11_BIND_RENDER_TARGET | D3D11_BIND_SHADER_RESOURCE;
TextureDesc.CPUAccessFlags = 0;
TextureDesc.MiscFlags = 0;

HRESULT hr = Device->CreateTexture2D(&TextureDesc, NULL, &Texture);
assert(SUCCEEDED(hr));

// Create RenderView
D3D11_RENDER_TARGET_VIEW_DESC Desc;
ZeroMemory(&Desc, sizeof(D3D11_RENDER_TARGET_VIEW_DESC));

Desc.Format = Format;
Desc.ViewDimension = D3D11_RTV_DIMENSION_TEXTURE2D;

hr = Device->CreateRenderTargetView(Texture, &Desc, &RTView);
assert(SUCCEEDED(hr));

// Create Shader Resource View
hr = Device->CreateShaderResourceView(Texture, NULL, &ResView);
assert(SUCCEEDED(hr));
}

void CGBuffer::Release()
{
GDiffuse->Release();
GNormal->Release();
GDepth->Release();
GLight->Release();

RTDiffuse->Release();
RTNormal->Release();
RTDepth->Release();
RTLight->Release();

GDiffuseView->Release();
GNormalView->Release();
GDepthView->Release();
GLightView->Release();

GDiffuse = NULL;
GNormal = NULL;
GDepth = NULL;
GLight = NULL;

RTDiffuse = NULL;
RTNormal = NULL;
RTDepth = NULL;
RTLight = NULL;

GDiffuseView = NULL;
GNormalView = NULL;
GDepthView = NULL;
GLightView = NULL;
}

void CGBuffer::Clear()
{
Context->ClearRenderTargetView(RTDiffuse, D3DXCOLOR(0.0f, 0.0f, 0.0f, 0.0f));
Context->ClearRenderTargetView(RTNormal, D3DXCOLOR(0.5f, 0.5f, 0.0f, 0.0f));
Context->ClearRenderTargetView(RTDepth, D3DXCOLOR(1.0f, 0.0f, 0.0f, 0.0f));
Context->ClearRenderTargetView(RTLight, D3DXCOLOR(0.0f, 0.0f, 0.0f, 0.0f));
}

void CGBuffer::BeginGeometryStage(ID3D11DepthStencilView *DepthBufferView)
{
ID3D11RenderTargetView* RenderTargetViews[3] =
{
RTDiffuse,
RTNormal,
RTDepth
};
Context->OMSetRenderTargets(3, RenderTargetViews, DepthBufferView);
}[/source]


This works fine, but there's a lot of questions popping up in my head:
[list][*]I only include stdafx.h (containing rarely changing libs such as DX libs) in my header files. I then add forward declarations for all other classes used in this one.
In my .cpp I include all header files of other classes needed.
It seems like I cannot run into problems this way... is this the correct approach for managing classes in c++?[*]I keep only pointers to all DX specific stuff (Textures, Rendertargetviews etc) which I can easily pass around.
I do the same thing with my own classes (My Renderer Class keeps only a Pointer to my GBuffer class)
Is this okay to do?
Or should I rather define things not as pointers, and then write get methods to pass references to member variables to other classes?
[source]
class CDeferredRenderer
{
private:
CGBuffer *GBuffer;
....
};
void CDeferredRenderer::Init( CEngine * Engine, HWND hWnd )
{
...
GBuffer = new CGBuffer();
GBuffer->Init(this);
...
}
[/source][*]This also means I have to do things like this (although rarely):
[source]void CGBuffer::CreateGTexture( DXGI_FORMAT Format, ID3D11Texture2D* & Texture, ID3D11RenderTargetView* & RTView, ID3D11ShaderResourceView* & ResView )[/source]
Is this okay?[*]Should I make all members private and write Getters for them?
In C# I'm used to do things like:
[source]public CGBuffer GBuffer { get; private set; }[/source]

I obviously can't reproduce this in C++ without writing a Get method for each variable I want to have read-only access to.
Should I really do this, or is it better to focus on functionality rather than wrapping everything with accessor methods?[*]Should I prefer using references over pointers if I'm in a situation where I can use both to achieve the same thing?
I noticed the DirectX11 api doesn't make much use of references. For example:
[source]D3DXMatrixIdentity(D3DXMATRIX *pOut);[/source]
instead of
[source]D3DXMatrixIdentity(D3DXMATRIX & Out);[/source][*]I've seen some code where all member variables are preceded with "m" and pointers with "p" resulting in code like:
[source]ID3D11Texture2D* mpGDiffuse;[/source]
Is this considered a good practice?[/list]

Cheers,
Hyu

Share this post


Link to post
Share on other sites
In general, it is best to use references if you are passing anything because it allows the compiler to do its job and optimize the code better because a reference cannot change, but a pointer can. Getters and Setters I do not use much because its a pain to write out all that code, when I can simply access the variable directly. For simplicity, I usually directly access the member. If I can type the function GetTexture(), I sure as well can type .Texture At least, thats how I look at it. Getters and Setters in my mind should be used when you want to ensure that other things happen when a variable is set. For example, if you set a texture, maybe you want to delete the old one if there is one, and then replace it with the new one. In that case, you need a setter function. But, sometimes you dont need to do any work, nor will you ever need to. In that case, I just go straight to the variable. Same concept with getters. If you think you might need to modify an internal state for each Get call, then create a function. If you are sure you wont need to do any internal work, save yourself some work and access the variable straight.

To your second question about headers and forward declarations, your method is correct. Put your forward declarations in the header, and include the necessary header in the cpp file.

Share this post


Link to post
Share on other sites
1. Yes, you should generally prefer to handle includes this way whenever possible. Putting includes in header files can cause longer recompilation times when you change an included header file.

2. Generally you only want to use pointers when you have to. In your example of the CDeferredRenderer containing a CGBuffer, there's really no reason to make it a pointer and then dynamically allocate an instance of CGBuffer since you're always going to have one. Instead the C++ way of doing it is to just declare the CGBuffer as a regular member. For passing something to a function you should almost always pass as a reference. If necessary that function/class can convert that reference to a pointer so that it can hold onto it.

3. This is more of an OOP thing and not really C++ specific. If you're used to using properties, you can just stick with writing getters and setters.

4. Yes, if you can use a reference you really probably should. Using references can save you from the many pitfalls of pointers. The reason the DirectX API uses pointers everywhere is because it maintains C compatibility, and C doesn't have references.

5. This also isn't really C++-related, some people do the same in C#. It comes from Microsoft, who used this notation frequently for the Win32 API and MFC library. Some people still like it, some hate it...I would say it's totally your call.

Share this post


Link to post
Share on other sites
Thank you both for your suggestions and explanations, I greatly appreciate it!
This really helps, I was feeling very insecure about these things, but now I have a good idea on how to do things better.

Cheers,
Hyu

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this  

  • Partner Spotlight

  • Forum Statistics

    • Total Topics
      627667
    • Total Posts
      2978534
  • Similar Content

    • By evelyn4you
      hi,
      i have read very much about the binding of a constantbuffer to a shader but something is still unclear to me.
      e.g. when performing :   vertexshader.setConstantbuffer ( buffer,  slot )
       is the buffer bound
      a.  to the VertexShaderStage
      or
      b. to the VertexShader that is currently set as the active VertexShader
      Is it possible to bind a constantBuffer to a VertexShader e.g. VS_A and keep this binding even after the active VertexShader has changed ?
      I mean i want to bind constantbuffer_A  to VS_A, an Constantbuffer_B to VS_B  and  only use updateSubresource without using setConstantBuffer command every time.

      Look at this example:
      SetVertexShader ( VS_A )
      updateSubresource(buffer_A)
      vertexshader.setConstantbuffer ( buffer_A,  slot_A )
      perform drawcall       ( buffer_A is used )

      SetVertexShader ( VS_B )
      updateSubresource(buffer_B)
      vertexshader.setConstantbuffer ( buffer_B,  slot_A )
      perform drawcall   ( buffer_B is used )
      SetVertexShader ( VS_A )
      perform drawcall   (now which buffer is used ??? )
       
      I ask this question because i have made a custom render engine an want to optimize to
      the minimum  updateSubresource, and setConstantbuffer  calls
       
       
       
       
       
    • By noodleBowl
      I got a quick question about buffers when it comes to DirectX 11. If I bind a buffer using a command like:
      IASetVertexBuffers IASetIndexBuffer VSSetConstantBuffers PSSetConstantBuffers  and then later on I update that bound buffer's data using commands like Map/Unmap or any of the other update commands.
      Do I need to rebind the buffer again in order for my update to take effect? If I dont rebind is that really bad as in I get a performance hit? My thought process behind this is that if the buffer is already bound why do I need to rebind it? I'm using that same buffer it is just different data
       
    • By Rockmover
      I am really stuck with something that should be very simple in DirectX 11. 
      1. I can draw lines using a PC (position, colored) vertices and a simple shader just fine.
      2. I can draw 3D triangles using PCN (position, colored, normal) vertices just fine (even transparency and SpecularBlinnPhong shaders).
       
      However, if I'm using my 3D shader, and I want to draw my PC lines in the same scene how can I do that?
       
      If I change my lines to PCN and pass them to the 3D shader with my triangles, then the lighting screws them all up.  I only want the lighting for the 3D triangles, but no SpecularBlinnPhong/Lighting for the lines (just PC). 
      I am sure this is because if I change the lines to PNC there is not really a correct "normal" for the lines.  
      I assume I somehow need to draw the 3D triangles using one shader, and then "switch" to another shader and draw the lines?  But I have no clue how to use two different shaders in the same scene.  And then are the lines just drawn on top of the triangles, or vice versa (maybe draw order dependent)?  
      I must be missing something really basic, so if anyone can just point me in the right direction (or link to an example showing the implementation of multiple shaders) that would be REALLY appreciated.
       
      I'm also more than happy to post my simple test code if that helps as well!
       
      THANKS SO MUCH IN ADVANCE!!!
    • By Reitano
      Hi,
      I am writing a linear allocator of per-frame constants using the DirectX 11.1 API. My plan is to replace the traditional constant allocation strategy, where most of the work is done by the driver behind my back, with a manual one inspired by the DirectX 12 and Vulkan APIs.
      In brief, the allocator maintains a list of 64K pages, each page owns a constant buffer managed as a ring buffer. Each page has a history of the N previous frames. At the beginning of a new frame, the allocator retires the frames that have been processed by the GPU and frees up the corresponding space in each page. I use DirectX 11 queries for detecting when a frame is complete and the ID3D11DeviceContext1::VS/PSSetConstantBuffers1 methods for binding constant buffers with an offset.
      The new allocator appears to be working but I am not 100% confident it is actually correct. In particular:
      1) it relies on queries which I am not too familiar with. Are they 100% reliable ?
      2) it maps/unmaps the constant buffer of each page at the beginning of a new frame and then writes the mapped memory as the frame is built. In pseudo code:
      BeginFrame:
          page.data = device.Map(page.buffer)
          device.Unmap(page.buffer)
      RenderFrame
          Alloc(size, initData)
              ...
              memcpy(page.data + page.start, initData, size)
          Alloc(size, initData)
              ...
              memcpy(page.data + page.start, initData, size)
      (Note: calling Unmap at the end of a frame prevents binding the mapped constant buffers and triggers an error in the debug layer)
      Is this valid ? 
      3) I don't fully understand how many frames I should keep in the history. My intuition says it should be equal to the maximum latency reported by IDXGIDevice1::GetMaximumFrameLatency, which is 3 on my machine. But, this value works fine in an unit test while on a more complex demo I need to manually set it to 5, otherwise the allocator starts overwriting previous frames that have not completed yet. Shouldn't the swap chain Present method block the CPU in this case ?
      4) Should I expect this approach to be more efficient than the one managed by the driver ? I don't have meaningful profile data yet.
      Is anybody familiar with the approach described above and can answer my questions and discuss the pros and cons of this technique based on his experience ? 
      For reference, I've uploaded the (WIP) allocator code at https://paste.ofcode.org/Bq98ujP6zaAuKyjv4X7HSv.  Feel free to adapt it in your engine and please let me know if you spot any mistakes
      Thanks
      Stefano Lanza
       
    • By Matt Barr
      Hey all. I've been working with compute shaders lately, and was hoping to build out some libraries to reuse code. As a prerequisite for my current project, I needed to sort a big array of data in my compute shader, so I was going to implement quicksort as a library function. My implementation was going to use an inout array to apply the changes to the referenced array.

      I spent half the day yesterday debugging in visual studio before I realized that the solution, while it worked INSIDE the function, reverted to the original state after returning from the function.

      My hack fix was just to inline the code, but this is not a great solution for the future.  Any ideas? I've considered just returning an array of ints that represents the sorted indices.
  • Popular Now