-
Notifications
You must be signed in to change notification settings - Fork 14
Cleanup AMD specific features. #682
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
base: main
Are you sure you want to change the base?
Conversation
hysw
commented
Dec 25, 2025
- Keep BufferView for now, as it looks somewhat reusable.
- Remove sqtt views.
- Keep BufferView for now, as it looks somewhat reusable. - Remove sqtt views.
|
|
||
| private: | ||
| QPointF m_mouse_pos; | ||
| #ifdef ENABLE_CAPTURE_BUFFERS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think could remove ENABLE_CAPTURE_BUFFERS, is it expensive of enabling it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ENABLE_CAPTURE_BUFFERS was an AMDVLK-only experimental feature that does not apply for our new Adrenal world. I would remove the entirety of buffers_view.cpp, since it's very old.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is the part i am confused, i saw some later code in this PR when we parse the PM4, we actually get heap types from PM4, and show it in UI when the buffer capture is enabled. and the Heap type seems to be valid for Adreno gpu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we can also infer some basic information from Vulkan capture.
| class ErrorDialog; | ||
| class EventSelection; | ||
| class EventStateView; | ||
| class EventTimingView; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think we could remove event timing for now, but one of the tasks i want @GrantComm to work on is to use the timeline view (not necessarily this one) to replace the gpu timing table that we have now. The old event timing view could be a good reference for Grant.
| DIVE_ASSERT(false); | ||
| } | ||
| treeItem->setText(8, tr(str_buffer)); | ||
| treeItem->setText(3, GetHeapString(static_cast<Dive::MemoryAllocationData::GpuHeap>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wow, i didn't know we can check the heap info. where can i see this heap info? i tried a capture but couldn't find this info in the shader tab or pm4 packets tab.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh i see, need to enable ENABLE_CAPTURE_BUFFERS? i never tried it. can you add a screenshot of the heap info in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The source information was from AMD open source driver. This view is compile time disabled (if constexpr) at the moment, I removed #if defined so compiler sees the code and enforce syntax correctness.