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

UI threading improvements #20072

Merged
merged 7 commits into from
Mar 3, 2025
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
3 changes: 3 additions & 0 deletions Common/System/System.h
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@ enum class SystemRequestType {
RUN_CALLBACK_IN_WNDPROC,
};

// Run a closure on the main thread. Used to safely implement UI that runs on another thread.
void System_RunOnMainThread(std::function<void()> func);

// Implementations are supposed to process the request, and post the response to the g_RequestManager (see Message.h).
// This is not to be used directly by applications, instead use the g_RequestManager to make the requests.
// This can return false if it's known that the platform doesn't support the request, the app is supposed to handle
Expand Down
2 changes: 1 addition & 1 deletion Core/Config.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2015,7 +2015,7 @@ void Config::ResetControlLayout() {
g_Config.fRightStickHeadScale = 1.0f;
}

void Config::GetReportingInfo(UrlEncoder &data) {
void Config::GetReportingInfo(UrlEncoder &data) const {
for (size_t i = 0; i < numSections; ++i) {
const std::string prefix = std::string("config.") + sections[i].section;
for (size_t j = 0; j < sections[i].settingsCount; j++) {
Expand Down
2 changes: 1 addition & 1 deletion Core/Config.h
Original file line number Diff line number Diff line change
Expand Up @@ -632,7 +632,7 @@ struct Config {

void ResetControlLayout();

void GetReportingInfo(UrlEncoder &data);
void GetReportingInfo(UrlEncoder &data) const;

bool IsPortrait() const;
int NextValidBackend();
Expand Down
1 change: 0 additions & 1 deletion Core/Core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,6 @@ const char *BreakReasonToString(BreakReason reason) {
case BreakReason::SavestateCrash: return "savestate.crash";
case BreakReason::MemoryBreakpoint: return "memory.breakpoint";
case BreakReason::CpuBreakpoint: return "cpu.breakpoint";
case BreakReason::BreakpointUpdate: return "cpu.breakpoint.update";
case BreakReason::MemoryAccess: return "memory.access"; // ???
case BreakReason::JitBranchDebug: return "jit.branchdebug";
case BreakReason::RABreak: return "ra.break";
Expand Down
1 change: 0 additions & 1 deletion Core/Core.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ enum class BreakReason {
SavestateCrash,
MemoryBreakpoint,
CpuBreakpoint,
BreakpointUpdate,
MemoryAccess, // ???
JitBranchDebug,
BreakOnBoot,
Expand Down
83 changes: 25 additions & 58 deletions Core/Debugger/Breakpoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@

BreakpointManager g_breakpoints;

void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) {
void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) const {
if (result & BREAK_ACTION_LOG) {
const char *type = write ? "Write" : "Read";
if (logFormat.empty()) {
Expand Down Expand Up @@ -71,11 +71,9 @@ BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc, const char
}

// Note: must lock while calling this.
size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp)
{
size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp) {
size_t found = INVALID_BREAKPOINT;
for (size_t i = 0; i < breakPoints_.size(); ++i)
{
for (size_t i = 0; i < breakPoints_.size(); ++i) {
const auto &bp = breakPoints_[i];
if (bp.addr == addr && (!matchTemp || bp.temporary == temp))
{
Expand All @@ -90,10 +88,8 @@ size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp)
return found;
}

size_t BreakpointManager::FindMemCheck(u32 start, u32 end)
{
for (size_t i = 0; i < memChecks_.size(); ++i)
{
size_t BreakpointManager::FindMemCheck(u32 start, u32 end) {
for (size_t i = 0; i < memChecks_.size(); ++i) {
if (memChecks_[i].start == start && memChecks_[i].end == end)
return i;
}
Expand Down Expand Up @@ -155,14 +151,11 @@ int BreakpointManager::AddBreakPoint(u32 addr, bool temp) {

breakPoints_.push_back(pt);
anyBreakPoints_ = true;
guard.unlock();
Update(addr);

return (int)breakPoints_.size() - 1;
} else if (!breakPoints_[bp].IsEnabled()) {
breakPoints_[bp].result |= BREAK_ACTION_PAUSE;
breakPoints_[bp].hasCond = false;
guard.unlock();
Update(addr);
return (int)bp;
} else {
Expand All @@ -183,7 +176,6 @@ void BreakpointManager::RemoveBreakPoint(u32 addr) {
breakPoints_.erase(breakPoints_.begin() + bp);

anyBreakPoints_ = !breakPoints_.empty();
guard.unlock();
Update(addr);
}
}
Expand All @@ -196,8 +188,6 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, bool status) {
breakPoints_[bp].result |= BREAK_ACTION_PAUSE;
else
breakPoints_[bp].result = BreakAction(breakPoints_[bp].result & ~BREAK_ACTION_PAUSE);

guard.unlock();
Update(addr);
}
}
Expand All @@ -207,7 +197,6 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, BreakAction result) {
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT) {
breakPoints_[bp].result = result;
guard.unlock();
Update(addr);
}
}
Expand All @@ -220,7 +209,6 @@ void BreakpointManager::ClearAllBreakPoints()
if (!breakPoints_.empty())
{
breakPoints_.clear();
guard.unlock();
Update();
}
}
Expand All @@ -230,20 +218,14 @@ void BreakpointManager::ClearTemporaryBreakPoints()
if (!anyBreakPoints_)
return;
std::unique_lock<std::mutex> guard(breakPointsMutex_);

bool update = false;
for (int i = (int)breakPoints_.size()-1; i >= 0; --i)
{
if (breakPoints_[i].temporary)
{
breakPoints_.erase(breakPoints_.begin() + i);
update = true;
Update();
}
}

guard.unlock();
if (update)
Update();
}

void BreakpointManager::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &cond)
Expand All @@ -254,38 +236,32 @@ void BreakpointManager::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &
{
breakPoints_[bp].hasCond = true;
breakPoints_[bp].cond = cond;
guard.unlock();
Update(addr);
}
}

void BreakpointManager::ChangeBreakPointRemoveCond(u32 addr)
{
void BreakpointManager::ChangeBreakPointRemoveCond(u32 addr) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT)
{
if (bp != INVALID_BREAKPOINT) {
breakPoints_[bp].hasCond = false;
guard.unlock();
Update(addr);
}
}

BreakPointCond *BreakpointManager::GetBreakPointCondition(u32 addr)
{
BreakPointCond *BreakpointManager::GetBreakPointCondition(u32 addr) {
std::lock_guard<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT && breakPoints_[bp].hasCond)
return &breakPoints_[bp].cond;
return NULL;
return nullptr;
}

void BreakpointManager::ChangeBreakPointLogFormat(u32 addr, const std::string &fmt) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr, true, false);
if (bp != INVALID_BREAKPOINT) {
breakPoints_[bp].logFormat = fmt;
guard.unlock();
Update(addr);
}
}
Expand Down Expand Up @@ -338,18 +314,18 @@ int BreakpointManager::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, B

memChecks_.push_back(check);
bool hadAny = anyMemChecks_.exchange(true);
if (!hadAny)
if (!hadAny) {
MemBlockOverrideDetailed();
guard.unlock();
}
Update();
return (int)memChecks_.size() - 1;
} else {
memChecks_[mc].cond = (MemCheckCondition)(memChecks_[mc].cond | cond);
memChecks_[mc].result = (BreakAction)(memChecks_[mc].result | result);
bool hadAny = anyMemChecks_.exchange(true);
if (!hadAny)
if (!hadAny) {
MemBlockOverrideDetailed();
guard.unlock();
}
Update();
return (int)mc;
}
Expand All @@ -366,7 +342,6 @@ void BreakpointManager::RemoveMemCheck(u32 start, u32 end)
bool hadAny = anyMemChecks_.exchange(!memChecks_.empty());
if (hadAny)
MemBlockReleaseDetailed();
guard.unlock();
Update();
}
}
Expand All @@ -379,7 +354,6 @@ void BreakpointManager::ChangeMemCheck(u32 start, u32 end, MemCheckCondition con
{
memChecks_[mc].cond = cond;
memChecks_[mc].result = result;
guard.unlock();
Update();
}
}
Expand All @@ -394,7 +368,6 @@ void BreakpointManager::ClearAllMemChecks()
bool hadAny = anyMemChecks_.exchange(false);
if (hadAny)
MemBlockReleaseDetailed();
guard.unlock();
Update();
}
}
Expand All @@ -406,7 +379,6 @@ void BreakpointManager::ChangeMemCheckAddCond(u32 start, u32 end, const BreakPoi
if (mc != INVALID_MEMCHECK) {
memChecks_[mc].hasCondition = true;
memChecks_[mc].condition = cond;
guard.unlock();
// No need to update jit for a condition add/remove, they're not baked in.
Update(-1);
}
Expand All @@ -417,7 +389,6 @@ void BreakpointManager::ChangeMemCheckRemoveCond(u32 start, u32 end) {
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK) {
memChecks_[mc].hasCondition = false;
guard.unlock();
// No need to update jit for a condition add/remove, they're not baked in.
Update(-1);
}
Expand All @@ -436,7 +407,6 @@ void BreakpointManager::ChangeMemCheckLogFormat(u32 start, u32 end, const std::s
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK) {
memChecks_[mc].logFormat = fmt;
guard.unlock();
Update();
}
}
Expand Down Expand Up @@ -620,30 +590,27 @@ std::vector<BreakPoint> BreakpointManager::GetBreakpoints() {
return breakPoints_;
}

void BreakpointManager::Update(u32 addr) {
if (MIPSComp::jit && addr != -1) {
bool resume = false;
if (Core_IsStepping() == false) {
Core_Break(BreakReason::BreakpointUpdate, addr);
Core_WaitInactive();
resume = true;
}
void BreakpointManager::Frame() {
// outside the lock here, should be ok.
if (!needsUpdate_) {
return;
}

std::lock_guard<std::mutex> guard(breakPointsMutex_);
if (MIPSComp::jit && updateAddr_ != -1) {
// In case this is a delay slot, clear the previous instruction too.
if (addr != 0)
mipsr4k.InvalidateICache(addr - 4, 8);
if (updateAddr_ != 0)
mipsr4k.InvalidateICache(updateAddr_ - 4, 8);
else
mipsr4k.ClearJitCache();

if (resume)
Core_Resume();
}

if (anyMemChecks_ && addr != -1)
if (anyMemChecks_ && updateAddr_ != -1)
UpdateCachedMemCheckRanges();

// Redraw in order to show the breakpoint.
System_Notify(SystemNotification::DISASSEMBLY);
needsUpdate_ = false;
}

bool BreakpointManager::ValidateLogFormat(MIPSDebugInterface *cpu, const std::string &fmt) {
Expand Down
12 changes: 10 additions & 2 deletions Core/Debugger/Breakpoints.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ struct MemCheck {
// Called on a copy.
BreakAction Action(u32 addr, bool write, int size, u32 pc, const char *reason);

void Log(u32 addr, bool write, int size, u32 pc, const char *reason);
void Log(u32 addr, bool write, int size, u32 pc, const char *reason) const;

bool IsEnabled() const {
return (result & BREAK_ACTION_PAUSE) != 0;
Expand Down Expand Up @@ -183,12 +183,17 @@ class BreakpointManager {
return anyMemChecks_;
}

void Update(u32 addr = 0);
void Frame();

bool ValidateLogFormat(MIPSDebugInterface *cpu, const std::string &fmt);
bool EvaluateLogFormat(MIPSDebugInterface *cpu, const std::string &fmt, std::string &result);

private:
// Should be called under lock.
void Update(u32 addr = 0) {
needsUpdate_ = true;
updateAddr_ = addr;
}
size_t FindBreakpoint(u32 addr, bool matchTemp = false, bool temp = false);
// Finds exactly, not using a range check.
size_t FindMemCheck(u32 start, u32 end);
Expand All @@ -208,6 +213,9 @@ class BreakpointManager {
std::vector<MemCheck> memChecks_;
std::vector<MemCheck> memCheckRangesRead_;
std::vector<MemCheck> memCheckRangesWrite_;

bool needsUpdate_ = true;
u32 updateAddr_ = 0;
};

extern BreakpointManager g_breakpoints;
Expand Down
2 changes: 1 addition & 1 deletion GPU/Common/GPUDebugInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ struct GPUDebugVertex {

class GPUDebugInterface {
public:
virtual ~GPUDebugInterface() {}
virtual ~GPUDebugInterface() = default;
virtual bool GetCurrentDisplayList(DisplayList &list) = 0;
virtual int GetCurrentPrimCount() = 0;
virtual std::vector<DisplayList> ActiveDisplayLists() = 0;
Expand Down
10 changes: 5 additions & 5 deletions GPU/Debugger/Breakpoints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const static u8 textureRelatedCmds[] = {
GE_CMD_TEXFLUSH, GE_CMD_TEXSYNC,
};

void GPUBreakpoints::Init() {
GPUBreakpoints::GPUBreakpoints() {
ClearAllBreakpoints();

nonTextureCmds.clear();
Expand Down Expand Up @@ -268,20 +268,20 @@ bool GPUBreakpoints::IsRenderTargetBreakpoint(u32 addr) {
return breakRenderTargets.find(addr) != breakRenderTargets.end();
}

bool GPUBreakpoints::IsOpBreakpoint(u32 op, bool &temp) {
bool GPUBreakpoints::IsOpBreakpoint(u32 op, bool &temp) const {
return IsCmdBreakpoint(op >> 24, temp);
}

bool GPUBreakpoints::IsOpBreakpoint(u32 op) {
bool GPUBreakpoints::IsOpBreakpoint(u32 op) const {
return IsCmdBreakpoint(op >> 24);
}

bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd, bool &temp) {
bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd, bool &temp) const {
temp = breakCmdsTemp[cmd];
return breakCmds[cmd];
}

bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd) {
bool GPUBreakpoints::IsCmdBreakpoint(u8 cmd) const {
return breakCmds[cmd];
}

Expand Down
Loading
Loading