Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small fixes to NRI Wrapper, Add external descriptor heap to D3D12 wrapper #77

Merged
merged 1 commit into from
Jul 14, 2024

Conversation

01Pollux
Copy link
Contributor

@01Pollux 01Pollux commented Jul 3, 2024

No description provided.

…t be parsed back from D3D12 resource struct, such as vertex usage mask etc, while other parts of library such as validation requires to check for consistency

D3D12: Added way to create external descriptor pool through wrapper
VK wrapper memory, set mapped address for already mapped external memories
@dzhdanNV
Copy link
Collaborator

Hi Yassine! Thanks. I have a question:
Why do you support old code path for textures:

    if (!textureDesc.textureDesc) {
        if (!GetTextureDesc(textureDesc, m_Desc))
            return Result::INVALID_ARGUMENT;
    } else {
        m_Desc = *textureDesc.textureDesc;
    }

but not for buffers:

    m_Desc = *bufferDesc.bufferDesc;

?
it can clearly hurt already written code...

@dzhdanNV
Copy link
Collaborator

dzhdanNV commented Jul 13, 2024

I would suggest the following additions to your commit:

  • return back bool nri::GetBufferDesc(const BufferD3D12Desc& bufferD3D12Desc, BufferDesc& bufferDesc) and fix it:
    • add forgotten ACCELERATION STRUCTURE bit
    • add VERTEX and INDEX bits unconditionally (AFAIR, these usages are always available in D3D)
  • initialize buffer's m_Desc similarly to texture's m_Desc if the description is not provided (i.e. NULL)
  • rename newly added bufferDesc and textureDesc to desc and add comment // (optional) if NULL, will be filled automatically but some info can be missing

@dzhdanNV dzhdanNV merged commit d432a73 into NVIDIAGameWorks:main Jul 14, 2024
2 checks passed
@dzhdanNV
Copy link
Collaborator

I will do necessary changes myself. Thanks! Merged!

dzhdanNV added a commit that referenced this pull request Jul 14, 2024
- "bufferDesc" and "textureDesc" renamed to "desc" since they are already parts of "Buffer" or "Texture" descs
- "desc" parameter made optional
- D3D12: "GetBufferDesc" fills in all implicitly allowed usage bits
- applied formatting
@01Pollux 01Pollux deleted the nri-wrapper-fix branch July 14, 2024 07:45
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.

2 participants