Skip to content

Commit

Permalink
Only enable RAM Clears for the SOCOM games that require it.
Browse files Browse the repository at this point in the history 
Should remove the performance impact of #8994 which is bigger than
expected, it seems (cache pollution?)
  • Loading branch information
@hrydgard
hrydgard committed Jan 28, 2017
1 parent f28fec3 commit 9c55e1e
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 9 deletions.
11 changes: 6 additions & 5 deletions Core/Compatibility.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ void Compatibility::Clear() {
}

void Compatibility::CheckSettings(IniFile &iniFile, const std::string &gameID) {
CheckSetting(iniFile, gameID, "VertexDepthRounding", flags_.VertexDepthRounding);
CheckSetting(iniFile, gameID, "PixelDepthRounding", flags_.PixelDepthRounding);
CheckSetting(iniFile, gameID, "DepthRangeHack", flags_.DepthRangeHack);
CheckSetting(iniFile, gameID, "VertexDepthRounding", &flags_.VertexDepthRounding);
CheckSetting(iniFile, gameID, "PixelDepthRounding", &flags_.PixelDepthRounding);
CheckSetting(iniFile, gameID, "DepthRangeHack", &flags_.DepthRangeHack);
CheckSetting(iniFile, gameID, "ClearToRAM", &flags_.ClearToRAM);
}

void Compatibility::CheckSetting(IniFile &iniFile, const std::string &gameID, const char *option, bool &flag) {
iniFile.Get(option, gameID.c_str(), &flag, flag);
void Compatibility::CheckSetting(IniFile &iniFile, const std::string &gameID, const char *option, bool *flag) {
iniFile.Get(option, gameID.c_str(), flag, *flag);
}
3 changes: 2 additions & 1 deletion Core/Compatibility.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ struct CompatFlags {
bool VertexDepthRounding;
bool PixelDepthRounding;
bool DepthRangeHack;
bool ClearToRAM;
};

class IniFile;
Expand All @@ -66,7 +67,7 @@ class Compatibility {
private:
void Clear();
void CheckSettings(IniFile &iniFile, const std::string &gameID);
void CheckSetting(IniFile &iniFile, const std::string &gameID, const char *option, bool &flag);
void CheckSetting(IniFile &iniFile, const std::string &gameID, const char *option, bool *flag);

CompatFlags flags_;
};
2 changes: 2 additions & 0 deletions GPU/Common/DrawEngineCommon.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ void DrawEngineCommon::ApplyClearToMemory(int x1, int y1, int x2, int y2, u32 cl
}

// This will most often be true - rarely is the width not aligned.
// TODO: We should really use non-temporal stores here to avoid the cache,
// as it's unlikely that these bytes will be read.
if ((width & 3) == 0 && (x1 & 3) == 0) {
u64 val64 = clearColor | ((u64)clearColor << 32);
int xstride = 2;
Expand Down
2 changes: 1 addition & 1 deletion GPU/Directx9/DrawEngineDX9.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ void DrawEngineDX9::DoFlush() {
int scissorY2 = gstate.getScissorY2() + 1;
framebufferManager_->SetSafeSize(scissorX2, scissorY2);

if (g_Config.bBlockTransferGPU && gstate.isClearModeColorMask() && (gstate.isClearModeAlphaMask() || gstate.FrameBufFormat() == GE_FORMAT_565)) {
if (g_Config.bBlockTransferGPU && (gstate_c.featureFlags & GPU_USE_CLEAR_RAM_HACK) && gstate.isClearModeColorMask() && (gstate.isClearModeAlphaMask() || gstate.FrameBufFormat() == GE_FORMAT_565)) {
ApplyClearToMemory(scissorX1, scissorY1, scissorX2, scissorY2, clearColor);
}
}
Expand Down
4 changes: 4 additions & 0 deletions GPU/Directx9/GPU_DX9.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,10 @@ void GPU_DX9::CheckGPUFeatures() {
features |= GPU_ROUND_DEPTH_TO_16BIT;
}

if (PSP_CoreParameter().compat.flags().ClearToRAM) {
features |= GPU_USE_CLEAR_RAM_HACK;
}

gstate_c.featureFlags = features;
}

Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/DrawEngineGLES.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,7 @@ void DrawEngineGLES::DoFlush() {
int scissorY2 = gstate.getScissorY2() + 1;
framebufferManager_->SetSafeSize(scissorX2, scissorY2);

if (g_Config.bBlockTransferGPU && colorMask && (alphaMask || gstate.FrameBufFormat() == GE_FORMAT_565)) {
if (g_Config.bBlockTransferGPU && (gstate_c.featureFlags & GPU_USE_CLEAR_RAM_HACK) && colorMask && (alphaMask || gstate.FrameBufFormat() == GE_FORMAT_565)) {
ApplyClearToMemory(scissorX1, scissorY1, scissorX2, scissorY2, clearColor);
}
}
Expand Down
4 changes: 4 additions & 0 deletions GPU/GLES/GPU_GLES.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -607,6 +607,10 @@ void GPU_GLES::CheckGPUFeatures() {
features |= GPU_USE_DEPTH_RANGE_HACK;
}

if (PSP_CoreParameter().compat.flags().ClearToRAM) {
features |= GPU_USE_CLEAR_RAM_HACK;
}

#ifdef MOBILE_DEVICE
// Arguably, we should turn off GPU_IS_MOBILE on like modern Tegras, etc.
features |= GPU_IS_MOBILE;
Expand Down
1 change: 1 addition & 0 deletions GPU/GPUState.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ enum {
GPU_USE_DEPTH_RANGE_HACK = FLAG_BIT(6),
GPU_SUPPORTS_WIDE_LINES = FLAG_BIT(7),
GPU_SUPPORTS_ANISOTROPY = FLAG_BIT(8),
GPU_USE_CLEAR_RAM_HACK = FLAG_BIT(9),
GPU_SUPPORTS_LARGE_VIEWPORTS = FLAG_BIT(16),
GPU_SUPPORTS_ACCURATE_DEPTH = FLAG_BIT(17),
GPU_SUPPORTS_VAO = FLAG_BIT(18),
Expand Down
2 changes: 1 addition & 1 deletion GPU/Vulkan/DrawEngineVulkan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,7 @@ void DrawEngineVulkan::DoFlush(VkCommandBuffer cmd) {
int scissorY2 = gstate.getScissorY2() + 1;
framebufferManager_->SetSafeSize(scissorX2, scissorY2);

if (g_Config.bBlockTransferGPU && gstate.isClearModeColorMask() && (gstate.isClearModeAlphaMask() || gstate.FrameBufFormat() == GE_FORMAT_565)) {
if (g_Config.bBlockTransferGPU && (gstate_c.featureFlags & GPU_USE_CLEAR_RAM_HACK) && gstate.isClearModeColorMask() && (gstate.isClearModeAlphaMask() || gstate.FrameBufFormat() == GE_FORMAT_565)) {
ApplyClearToMemory(scissorX1, scissorY1, scissorX2, scissorY2, result.color);
}
}
Expand Down
5 changes: 5 additions & 0 deletions GPU/Vulkan/GPU_Vulkan.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,11 @@ void GPU_Vulkan::CheckGPUFeatures() {
if (vulkan_->GetFeaturesEnabled().samplerAnisotropy) {
gstate_c.featureFlags |= GPU_SUPPORTS_ANISOTROPY;
}

if (PSP_CoreParameter().compat.flags().ClearToRAM) {
gstate_c.featureFlags |= GPU_USE_CLEAR_RAM_HACK;
}

// Mandatory features on Vulkan, which may be checked in "centralized" code
gstate_c.featureFlags |= GPU_SUPPORTS_TEXTURE_LOD_CONTROL;
gstate_c.featureFlags |= GPU_SUPPORTS_FBO;
Expand Down
12 changes: 12 additions & 0 deletions assets/compat.ini
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,3 +101,15 @@ ULJM91018 = true # Infinity demo disc?
NPJH90157 = true # Infinity demo
ULJM05732 = true
NPJH50332 = true

[ClearToRAM]
# SOCOM Navy Seals games require this. See issue #8973.
# Navy Seals
UCUS98615 = true
ULES00038 = true
UCKS45021 = true
# Fireteam Bravo 3
UCJS10102 = true
NPJG00035 = true
UCUS98716 = true
UCES01242 = true

4 comments on commit 9c55e1e

@unknownbrackets
Copy link
Collaborator

Choose a reason for hiding this comment

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

Which games specifically were slow? This seems like an ugly hack.

-[Unknown]

@hrydgard
Copy link
Owner Author

Choose a reason for hiding this comment

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

Rendering to buffers separately from RAM is a hack in itself, and writing to RAM only on rectangular clears (but not, say, fullscreen triangles) is not very pretty either.

I saw it eating >1% in several games I profiled, don't remember exactly which ones.

@unknownbrackets
Copy link
Collaborator

Choose a reason for hiding this comment

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

Darn, that's a shame. As you know, I just hate hacks by game id - they'll never feel right to me out of a pool of ~4000 game ids, even if no more are being released. But it's true that the entire non-software rendering could be considered a hack...

-[Unknown]

@hrydgard
Copy link
Owner Author

@hrydgard hrydgard commented on 9c55e1e Mar 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I'm myself a little conflicted. But a 1% penalty on a fairly large number of games, while only fixing issues in 2, while those issues are only there because our whole rendering pipeline is a "hack", is just on the wrong side of the balance to me...

If we want rendering entirely without hacks, we should work on speeding up the software renderer..

Dangan Ronpa is another nasty case, that address check would be better off as a ID-based hack, IMHO.

Please sign in to comment.