Skip to content

Fix issues for dx12(2)#2811

Open
zongdu-arm wants to merge 3 commits intoLunarG:devfrom
zongdu-arm:fix_issues_for_dx12
Open

Fix issues for dx12(2)#2811
zongdu-arm wants to merge 3 commits intoLunarG:devfrom
zongdu-arm:fix_issues_for_dx12

Conversation

@zongdu-arm
Copy link
Copy Markdown
Contributor

@zongdu-arm zongdu-arm commented Mar 24, 2026

  1. Fixed a crash where the OpenExistingHeapFromFileMapping and OpenExistingHeapFromAddress1 methods were missing.
  2. Fixed an issue where the SetPrivateDataInterface method was missing during trimming.
  3. Fixed a reference count error in the private data interface, causing a replay crash when releasing resource objects.
  4. Fixed an issue with ID3D10Blob trimmed capture: the ID3D10Blob creation command released it in tracking state, not written to the trimmed file.
  5. Fixed the heap flags with D3D12_HEAP_FLAG_ALLOW_WRITE_WATCH using UseWriteWatch.
  6. Updated the clang-format code.

Track in #2843

@zongdu-arm zongdu-arm requested a review from a team as a code owner March 24, 2026 07:13
@zongdu-arm zongdu-arm force-pushed the fix_issues_for_dx12 branch 3 times, most recently from db7abd8 to 4628883 Compare March 24, 2026 07:44
@zongdu-arm
Copy link
Copy Markdown
Contributor Author

@fabian-lunarg , @davidd-lunarg, @locke-lunarg
Could you review this PR?

@zongdu-arm zongdu-arm force-pushed the fix_issues_for_dx12 branch 2 times, most recently from fa33340 to 34a18be Compare March 31, 2026 10:19
&parameter_stream_);
parameter_stream_.Clear();

StandardCreateWrite(wrapper);
Copy link
Copy Markdown
Contributor

@locke-lunarg locke-lunarg Apr 2, 2026

Choose a reason for hiding this comment

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

Here create a new blob and write this blob. This new blob should be used for CreateRootSignature, but StandardCreateWrite(wrapper); seems to still write the original blob. I feel it should write the new blob for it. Or do both GetBufferPointer() are the same because they have the same blob_value?

Copy link
Copy Markdown
Contributor

@locke-lunarg locke-lunarg Apr 3, 2026

Choose a reason for hiding this comment

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

Plus, in Encode_ID3D12Device_CreateRootSignature, it writes all ID3D10Blob data by encoder->EncodeVoidArray(pBlobWithRootSignature, blobLengthInBytes);. It seems to be not necessary to create ID3D10Blob again. Why does it write D3D12SerializeVersionedRootSignature, ID3D10Blob_GetBufferSize, ID3D10Blob_GetBufferPointer? Or is it just for recording the complete process, not for correctly executing the program?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Here create a new blob and write this blob. This new blob should be used for CreateRootSignature, but StandardCreateWrite(wrapper); seems to still write the original blob. I feel it should write the new blob for it. Or do both GetBufferPointer() are the same because they have the same blob_value?

I believe the blob data is the same at runtime and does not need to be updated.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Plus, in Encode_ID3D12Device_CreateRootSignature, it writes all ID3D10Blob data by encoder->EncodeVoidArray(pBlobWithRootSignature, blobLengthInBytes);. It seems to be not necessary to create ID3D10Blob again. Why does it write D3D12SerializeVersionedRootSignature, ID3D10Blob_GetBufferSize, ID3D10Blob_GetBufferPointer? Or is it just for recording the complete process, not for correctly executing the program?

I'm not sure if the captured blob data on replaying different GPU is the same. However, I haven't adapted the replay code yet.

Copy link
Copy Markdown
Contributor

@locke-lunarg locke-lunarg Apr 3, 2026

Choose a reason for hiding this comment

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

Replay works fine on different GPUs, so this looks good.

My question is: since the capture already writes all the ID3D10Blob data, omitting the creation calls shouldn't hurt. Why do we explicitly record D3D12SerializeVersionedRootSignature, ID3D10Blob_GetBufferSize, and ID3D10Blob_GetBufferPointer?

Does it affect replay correctness, or is it to record the full process to help users identify which ID3D10Blob is being used? I’m happy to keep it for completeness, just want to understand the intent. Thanks!

@bradgrantham-lunarg bradgrantham-lunarg added the approved-to-run-ci Can run CI check on internal LunarG machines label Apr 8, 2026
1. Fixed a crash where the `OpenExistingHeapFromFileMapping` and `OpenExistingHeapFromAddress1` methods were missing.
2. Fixed an issue where the `SetPrivateDataInterface` method was missing during trimming.
3. Fixed a reference count error in the private data interface, causing a replay crash when releasing resource objects.
4. Fixed an issue with ID3D10Blob trimmed capture: the ID3D10Blob creation command released it in tracking state, not written to the trimmed file.
5. Fixed the heap flags with D3D12_HEAP_FLAG_ALLOW_WRITE_WATCH using UseWriteWatch.
6. Updated the `clang-format` code.
@locke-lunarg
Copy link
Copy Markdown
Contributor

locke-lunarg commented Apr 9, 2026

The error in continuous-integration/jenkins/pr-merge is

35>C:\j\workspace\gfxr-pipelines_gfxr_PR-2811\gfxreconstruct\framework\decode\dx12_replay_consumer_base.cpp(2463,78): error C2220: the following warning is treated as an error [C:\j\workspace\gfxr-pipelines_gfxr_PR-2811\gfxreconstruct\build32\framework\decode\gfxrecon_decode.vcxproj]
35>C:\j\workspace\gfxr-pipelines_gfxr_PR-2811\gfxreconstruct\framework\decode\dx12_replay_consumer_base.cpp(2463,78): warning C4293: '>>': shift count negative or too big, undefined behavior [C:\j\workspace\gfxr-pipelines_gfxr_PR-2811\gfxreconstruct\build32\framework\decode\gfxrecon_decode.vcxproj]
HANDLE handle = CreateFileMapping(INVALID_HANDLE_VALUE,
                                  nullptr,
                                  PAGE_READWRITE,
                      >>>>>>>>    static_cast<DWORD>(info.RegionSize >> 32),
                                  static_cast<DWORD>(info.RegionSize & 0xFFFFFFFF),
                                  TEXT("OpenExistingHeapFromFileMapping"));

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved-to-run-ci Can run CI check on internal LunarG machines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants