Skip to content

Conversation

Themperror
Copy link

@Themperror Themperror commented Dec 2, 2022

todo:
Complete Models,
Complete Dear ImGui

todo:
refactor earlier projects

@Themperror Themperror added documentation Improvements or additions to documentation help wanted Extra attention is needed cpp-project labels Dec 2, 2022
@Themperror Themperror added this to the Part 1 milestone Dec 2, 2022
@Themperror Themperror self-assigned this Dec 2, 2022
Copy link
Contributor

@deccer deccer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found 2 unrelated things... we can fix those later or treat them with this as well...

Otherwise this reshuffle should be alright

<VCProjectVersion>16.0</VCProjectVersion>
<Keyword>Win32Proj</Keyword>
<ProjectGuid>{E38C17E3-29B9-40BF-9372-0EB24DDE4B06}</ProjectGuid>
<RootNamespace>My13HelloTriangle</RootNamespace>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oi, where did that come from? :o

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know, multiple projects seem to have that

float4 Main(VSOutput input): SV_Target
{
float4 texel = Texture.Sample(LinearSampler, input.Uv);
//return float4(input.Color, 1.0f) + 0.5 * texel;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed i guess

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, partly, this still exists in some other chapters, but those will all still require adjusting to what each chapter requires (though I'd assume each chapter after Texturing probably needs the texture sample

static size_t GetLayoutByteSize(VertexType vertexType);

void Set(ID3D11DeviceContext* context);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm where would it make sense to expose this function, when you pass the device in the class factory already? (deferred contexts are also kind of a not worthy endeavour in d3d11)


void Rendering3DApplication::CreateDepthStencilView()
{
D3D11_TEXTURE2D_DESC texDesc{};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write it out as textureDescription or textureDescriptor please

ID3D11Texture2D* texture = nullptr;
if (FAILED(_device->CreateTexture2D(&texDesc, nullptr, &texture)))
{
std::cout << "DXGI: Failed to create texture for DepthStencilView\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh std::cout also triggers me, but once all the tuts are in place, we shall replace with spdlog for some more real worldyisms

return;
}

D3D11_DEPTH_STENCIL_VIEW_DESC dsvDesc{};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, can we write it out as dsvDescriptor or dsvDescription


void Rendering3DApplication::CreateDepthState()
{
D3D11_DEPTH_STENCIL_DESC depthDesc{};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here as well

@@ -193,7 +250,6 @@ bool Rendering3DApplication::Load()
//Back
4, 6, 7,
7, 5, 4

};

D3D11_BUFFER_DESC bufferInfo = {};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should also be a ...Descriptor or ...Description to be consistent

ID3D11RenderTargetView* nullTarget = nullptr;
ID3D11RenderTargetView* nullRTV = nullptr;

//set to nullptr so we can clear properly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of saying set to nullptr so we can clear properly
write this will ensure proper clear (its obvious that we are setting it in the line of code below, doesn't need separate mention)

@Themperror Themperror marked this pull request as ready for review April 15, 2023 18:06
@Themperror Themperror merged commit e5b09ef into GraphicsProgramming:main Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cpp-project documentation Improvements or additions to documentation help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants