Skip to content
This repository has been archived by the owner on Mar 30, 2019. It is now read-only.

Direct3d11 small fix and improvement #1132

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mrvux
Copy link
Contributor

@mrvux mrvux commented Feb 26, 2019

Hello, just a small change in Direct3D11.

  • Fix GetUnorderedAccess view call in case no depth buffer is bound
  • Add ComArray binding for CSSetUnorderedAccessViews (same as Constant buffer/SRV counterpart)

Thanks

…if depth stencil view was not bound (and cause a nullreferenceexception in that case)
public unsafe void SetUnorderedAccessViews(int startSlot, ComArray<SharpDX.Direct3D11.UnorderedAccessView> unorderedAccessViews, int[] uavInitialCounts)
{
fixed (void* puav = uavInitialCounts)
SetUnorderedAccessViews(startSlot, unorderedAccessViews == null ? 0 : unorderedAccessViews.Length, unorderedAccessViews.NativePointer, (IntPtr)puav);
Copy link

@h1cks h1cks Feb 26, 2019

Choose a reason for hiding this comment

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

Won't you get a null pointer exception on unorderedAccessViews.NativePointer for SetUnorderedAccessViews as you are not testing the 3rd parameter that the object is also Null? You would need to repeat the same test on the 3rd parameter also, so that you do not get a null reference exception if the object is null when calling NativePointer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, thinking of that in pretty much all existing DeviceContext "Set[]" which use ComArray, there is no null check at all, so could actually remove the first check.

Copy link

Choose a reason for hiding this comment

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

There are a couple of such cases, agreed, but I think if you did check Null Pointer on unorderedAccessViews then that woudl be fine. OR for removing ambiguity, set the code as:

fixed (void* puav = uavInitialCounts) { if (unorderedAccessViews != null) { SetUnorderedAccessViews(startSlot, unorderedAccessViews.Length, uorderedAccessViews.NativePointer, (IntPtr)puav); } else { SetUnorderedAccessViews(startSlot, 0, null, (IntPtr)puav); } }

I think the null pointer checks are correct that you are doing. I think some expansion to protect against a legitimate use case where the UAV is set to null. But yes, there are some checks missed in the code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants