Skip to content

Fix compile-time errors in D3D12MeshShaders samples (error C2102: '&' requires l-value)#683

Merged
sebmerry merged 5 commits intomicrosoft:masterfrom
robbiesri:gh-652-mesh-shaders-buildbuild
Jan 4, 2021
Merged

Fix compile-time errors in D3D12MeshShaders samples (error C2102: '&' requires l-value)#683
sebmerry merged 5 commits intomicrosoft:masterfrom
robbiesri:gh-652-mesh-shaders-buildbuild

Conversation

@robbiesri
Copy link
Copy Markdown
Contributor

@robbiesri robbiesri commented Dec 21, 2020

As referenced in issue #652, there are a number of compile-time errors in the Mesh Shaders sample set. They all relate to using r-values (temporary copies of various D3D12 structs) to generate pointers to pass into the D3D API surface.

I converted all of the problematic spots by using const locals to temporarily hold the struct contents. I also took the (small) liberty of merging some barriers in Model::UploadGpuResources (search for postCopyBarriers near the end of the method. Let me know if you want to undo them and match the original style of one barrier per copied resource.

Trickiest part for me was figuring out how to match the existing style, considering so much of the code was using this short-hand style. I was able to model off some of the other samples, so I hope it's ok.

Extra: I did not fix all the samples in the repo, because I thought that it was a bit of scope for this PR, especially for my first PR against this repo. Maybe I'll fix the rest in a follow-up PR!

Just throwing structures into temporaries, might be better ways to do this
This seems relatively decent, besides the loop for vertex resource barriers

Fixed a bug where I set the wrong end state
@ghost
Copy link
Copy Markdown

ghost commented Dec 21, 2020

CLA assistant check
All CLA requirements met.

@robbiesri
Copy link
Copy Markdown
Contributor Author

cc @jenatali: You suggested fixing the code in #652, so I took that as a signal that this work was ok to do 😄

Copy link
Copy Markdown
Member

@jenatali jenatali left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for doing this!

@sebmerry
Copy link
Copy Markdown
Member

sebmerry commented Jan 4, 2021

This looks great, thank you!

@sebmerry sebmerry merged commit 1f1c045 into microsoft:master Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants