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

Fix GE framedump playback on Vulkan #18793

Merged
merged 3 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions Common/GPU/Vulkan/VulkanFrameData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ VkCommandBuffer FrameData::GetInitCmd(VulkanContext *vulkan) {
return initCmd;
}

void FrameData::SubmitPending(VulkanContext *vulkan, FrameSubmitType type, FrameDataShared &sharedData) {
void FrameData::Submit(VulkanContext *vulkan, FrameSubmitType type, FrameDataShared &sharedData) {
VkCommandBuffer cmdBufs[3];
int numCmdBufs = 0;

Expand Down Expand Up @@ -200,14 +200,16 @@ void FrameData::SubmitPending(VulkanContext *vulkan, FrameSubmitType type, Frame
hasMainCommands = false;
}

if (hasPresentCommands && type != FrameSubmitType::Pending) {
if (hasPresentCommands) {
_dbg_assert_(type == FrameSubmitType::FinishFrame);
VkResult res = vkEndCommandBuffer(presentCmd);

_assert_msg_(res == VK_SUCCESS, "vkEndCommandBuffer failed (present)! result=%s", VulkanResultToString(res));

cmdBufs[numCmdBufs++] = presentCmd;
hasPresentCommands = false;

if (type == FrameSubmitType::Present) {
if (type == FrameSubmitType::FinishFrame) {
fenceToTrigger = fence;
}
}
Expand All @@ -219,15 +221,15 @@ void FrameData::SubmitPending(VulkanContext *vulkan, FrameSubmitType type, Frame

VkSubmitInfo submit_info{ VK_STRUCTURE_TYPE_SUBMIT_INFO };
VkPipelineStageFlags waitStage[1]{ VK_PIPELINE_STAGE_COLOR_ATTACHMENT_OUTPUT_BIT };
if (type == FrameSubmitType::Present && !skipSwap) {
if (type == FrameSubmitType::FinishFrame && !skipSwap) {
_dbg_assert_(hasAcquired);
submit_info.waitSemaphoreCount = 1;
submit_info.pWaitSemaphores = &acquireSemaphore;
submit_info.pWaitDstStageMask = waitStage;
}
submit_info.commandBufferCount = (uint32_t)numCmdBufs;
submit_info.pCommandBuffers = cmdBufs;
if (type == FrameSubmitType::Present && !skipSwap) {
if (type == FrameSubmitType::FinishFrame && !skipSwap) {
submit_info.signalSemaphoreCount = 1;
submit_info.pSignalSemaphores = &renderingCompleteSemaphore;
}
Expand Down
6 changes: 3 additions & 3 deletions Common/GPU/Vulkan/VulkanFrameData.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct FrameDataShared {
enum class FrameSubmitType {
Pending,
Sync,
Present,
FinishFrame,
};

// Per-frame data, round-robin so we can overlap submission with execution of the previous frame.
Expand Down Expand Up @@ -121,8 +121,8 @@ struct FrameData {
// Generally called from the main thread, unlike most of the rest.
VkCommandBuffer GetInitCmd(VulkanContext *vulkan);

// This will only submit if we are actually recording init commands.
void SubmitPending(VulkanContext *vulkan, FrameSubmitType type, FrameDataShared &shared);
// Submits pending command buffers.
void Submit(VulkanContext *vulkan, FrameSubmitType type, FrameDataShared &shared);

private:
// Metadata for logging etc
Expand Down
2 changes: 1 addition & 1 deletion Common/GPU/Vulkan/VulkanQueueRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ void VulkanQueueRunner::RunSteps(std::vector<VKRStep *> &steps, int curFrame, Fr
if (emitLabels) {
vkCmdEndDebugUtilsLabelEXT(cmd);
}
frameData.SubmitPending(vulkan_, FrameSubmitType::Pending, frameDataShared);
frameData.Submit(vulkan_, FrameSubmitType::Pending, frameDataShared);

// When stepping in the GE debugger, we can end up here multiple times in a "frame".
// So only acquire once.
Expand Down
12 changes: 9 additions & 3 deletions Common/GPU/Vulkan/VulkanRenderManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -952,6 +952,11 @@ void VulkanRenderManager::BindFramebufferAsRenderTarget(VKRFramebuffer *fb, VKRR
EndCurRenderStep();
}

// Sanity check that we don't have binds to the backbuffer before binds to other buffers. It must always be bound last.
if (steps_.size() >= 1 && steps_.back()->stepType == VKRStepType::RENDER && steps_.back()->render.framebuffer == nullptr && fb != nullptr) {
_dbg_assert_(false);
}

// Older Mali drivers have issues with depth and stencil don't match load/clear/etc.
// TODO: Determine which versions and do this only where necessary.
u32 lateClearMask = 0;
Expand Down Expand Up @@ -1383,6 +1388,7 @@ void VulkanRenderManager::Finish() {
EndCurRenderStep();

// Let's do just a bit of cleanup on render commands now.
// TODO: Should look into removing this.
for (auto &step : steps_) {
if (step->stepType == VKRStepType::RENDER) {
CleanupRenderCommands(&step->commands);
Expand Down Expand Up @@ -1469,7 +1475,7 @@ void VulkanRenderManager::Run(VKRRenderThreadTask &task) {
if (!frameTimeHistory_[frameData.frameId].firstSubmit) {
frameTimeHistory_[frameData.frameId].firstSubmit = time_now_d();
}
frameData.SubmitPending(vulkan_, FrameSubmitType::Pending, frameDataShared_);
frameData.Submit(vulkan_, FrameSubmitType::Pending, frameDataShared_);

// Flush descriptors.
double descStart = time_now_d();
Expand Down Expand Up @@ -1506,12 +1512,12 @@ void VulkanRenderManager::Run(VKRRenderThreadTask &task) {

switch (task.runType) {
case VKRRunType::SUBMIT:
frameData.SubmitPending(vulkan_, FrameSubmitType::Present, frameDataShared_);
frameData.Submit(vulkan_, FrameSubmitType::FinishFrame, frameDataShared_);
break;

case VKRRunType::SYNC:
// The submit will trigger the readbackFence, and also do the wait for it.
frameData.SubmitPending(vulkan_, FrameSubmitType::Sync, frameDataShared_);
frameData.Submit(vulkan_, FrameSubmitType::Sync, frameDataShared_);

if (useRenderThread_) {
std::unique_lock<std::mutex> lock(syncMutex_);
Expand Down
1 change: 1 addition & 0 deletions GPU/Common/FramebufferManagerCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,7 @@ void FramebufferManagerCommon::CopyDisplayToOutput(bool reallyDirty) {
// No framebuffer to display! Clear to black.
if (useBufferedRendering_) {
draw_->BindFramebufferAsRenderTarget(nullptr, { Draw::RPAction::CLEAR, Draw::RPAction::CLEAR, Draw::RPAction::CLEAR }, "CopyDisplayToOutput");
presentation_->NotifyPresent();
}
gstate_c.Dirty(DIRTY_VIEWPORTSCISSOR_STATE);
return;
Expand Down
4 changes: 4 additions & 0 deletions GPU/Common/PresentationCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,10 @@ class PresentationCommon {
bool PresentedThisFrame() const {
return presentedThisFrame_;
}
void NotifyPresent() {
// Something else did the present, skipping PresentationCommon.
presentedThisFrame_ = true;
}

void DeviceLost();
void DeviceRestore(Draw::DrawContext *draw);
Expand Down
13 changes: 8 additions & 5 deletions GPU/Debugger/Playback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ class DumpExecute {
void Memcpy(u32 ptr, u32 sz);
void Texture(int level, u32 ptr, u32 sz);
void Framebuf(int level, u32 ptr, u32 sz);
void Display(u32 ptr, u32 sz);
void Display(u32 ptr, u32 sz, bool allowFlip);
void EdramTrans(u32 ptr, u32 sz);

u32 execMemcpyDest = 0;
Expand Down Expand Up @@ -616,7 +616,7 @@ void DumpExecute::Framebuf(int level, u32 ptr, u32 sz) {
}
}

void DumpExecute::Display(u32 ptr, u32 sz) {
void DumpExecute::Display(u32 ptr, u32 sz, bool allowFlip) {
struct DisplayBufData {
PSPPointer<u8> topaddr;
int linesize, pixelFormat;
Expand All @@ -628,7 +628,9 @@ void DumpExecute::Display(u32 ptr, u32 sz) {
SyncStall();

__DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 1);
__DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 0);
if (allowFlip) {
__DisplaySetFramebuf(disp->topaddr.ptr, disp->linesize, disp->pixelFormat, 0);
}
}

void DumpExecute::EdramTrans(u32 ptr, u32 sz) {
Expand Down Expand Up @@ -657,7 +659,8 @@ bool DumpExecute::Run() {
if (gpu)
gpu->SetAddrTranslation(0x400);

for (const Command &cmd : commands_) {
for (size_t i = 0; i < commands_.size(); i++) {
const Command &cmd = commands_[i];
switch (cmd.type) {
case CommandType::INIT:
Init(cmd.ptr, cmd.sz);
Expand Down Expand Up @@ -726,7 +729,7 @@ bool DumpExecute::Run() {
break;

case CommandType::DISPLAY:
Display(cmd.ptr, cmd.sz);
Display(cmd.ptr, cmd.sz, i == commands_.size() - 1);
break;

default:
Expand Down
4 changes: 2 additions & 2 deletions UI/EmuScreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1342,13 +1342,13 @@ ScreenRenderFlags EmuScreen::render(ScreenRenderMode mode) {
if (mode & ScreenRenderMode::TOP) {
System_Notify(SystemNotification::KEEP_SCREEN_AWAKE);
} else if (!Core_ShouldRunBehind() && strcmp(screenManager()->topScreen()->tag(), "DevMenu") != 0) {
// Not on top. Let's not execute, only draw the image.
draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR, }, "EmuScreen_Stepping");
// Just to make sure.
if (PSP_IsInited() && !g_Config.bSkipBufferEffects) {
PSP_BeginHostFrame();
gpu->CopyDisplayToOutput(true);
PSP_EndHostFrame();
} else {
draw->BindFramebufferAsRenderTarget(nullptr, { RPAction::CLEAR, RPAction::CLEAR, RPAction::CLEAR, }, "EmuScreen_Stepping");
}
// Need to make sure the UI texture is available, for "darken".
screenManager()->getUIContext()->BeginFrame();
Expand Down
Loading