Skip to content

Commit

Permalink
Merge pull request #11694 from unknownbrackets/draw-bugs
Browse files Browse the repository at this point in the history
Vulkan: Limit stencil workaround to Adreno 5xx
  • Loading branch information
hrydgard authored Dec 26, 2018
2 parents da5fa5d + 58ef662 commit 8e17caf
Show file tree
Hide file tree
Showing 31 changed files with 162 additions and 111 deletions.
9 changes: 8 additions & 1 deletion GPU/Common/ShaderCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@

#include <cstdint>

namespace Draw {
class DrawContext;
}

enum ShaderLanguage {
GLSL_140,
GLSL_300,
Expand Down Expand Up @@ -116,10 +120,13 @@ enum : uint64_t {

class ShaderManagerCommon {
public:
ShaderManagerCommon() {}
ShaderManagerCommon(Draw::DrawContext *draw) : draw_(draw) {}
virtual ~ShaderManagerCommon() {}

virtual void DirtyLastShader() = 0;

protected:
Draw::DrawContext *draw_ = nullptr;
};

struct TBuiltInResource;
Expand Down
12 changes: 8 additions & 4 deletions GPU/Common/ShaderId.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include <string>
#include <sstream>

#include "thin3d/thin3d.h"
#include "Common/StringUtils.h"
#include "Core/Config.h"

Expand Down Expand Up @@ -225,7 +226,7 @@ std::string FragmentShaderDesc(const ShaderID &id) {

// Here we must take all the bits of the gstate that determine what the fragment shader will
// look like, and concatenate them together into an ID.
void ComputeFragmentShaderID(ShaderID *id_out) {
void ComputeFragmentShaderID(ShaderID *id_out, const Draw::Bugs &bugs) {
ShaderID id;
if (gstate.isModeClear()) {
// We only need one clear shader, so let's ignore the rest of the bits.
Expand Down Expand Up @@ -292,9 +293,6 @@ void ComputeFragmentShaderID(ShaderID *id_out) {
if (stencilToAlpha != REPLACE_ALPHA_NO) {
// 4 bits
id.SetBits(FS_BIT_REPLACE_ALPHA_WITH_STENCIL_TYPE, 4, ReplaceAlphaWithStencilType());
} else {
// Use those bits instead for whether stencil output is disabled.
id.SetBit(FS_BIT_REPLACE_ALPHA_WITH_STENCIL_TYPE, IsStencilTestOutputDisabled());
}

// 2 bits.
Expand All @@ -312,6 +310,12 @@ void ComputeFragmentShaderID(ShaderID *id_out) {
id.SetBit(FS_BIT_FLATSHADE, doFlatShading);

id.SetBit(FS_BIT_SHADER_DEPAL, useShaderDepal);

if (g_Config.bVendorBugChecksEnabled) {
if (bugs.Has(Draw::Bugs::NO_DEPTH_CANNOT_DISCARD_STENCIL)) {
id.SetBit(FS_BIT_NO_DEPTH_CANNOT_DISCARD_STENCIL, !IsStencilTestOutputDisabled() && !gstate.isDepthWriteEnabled());
}
}
}

*id_out = id;
Expand Down
9 changes: 7 additions & 2 deletions GPU/Common/ShaderId.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@ enum {
FS_BIT_FLATSHADE = 46,
FS_BIT_BGRA_TEXTURE = 47,
FS_BIT_TEST_DISCARD_TO_ZERO = 48,
// 49+ are free.
FS_BIT_NO_DEPTH_CANNOT_DISCARD_STENCIL = 49,
// 50+ are free.
};

struct ShaderID {
Expand Down Expand Up @@ -175,11 +176,15 @@ struct FShaderID : ShaderID {
}
};

namespace Draw {
class Bugs;
}


void ComputeVertexShaderID(ShaderID *id, uint32_t vertexType, bool useHWTransform);
// Generates a compact string that describes the shader. Useful in a list to get an overview
// of the current flora of shaders.
std::string VertexShaderDesc(const ShaderID &id);

void ComputeFragmentShaderID(ShaderID *id);
void ComputeFragmentShaderID(ShaderID *id, const Draw::Bugs &bugs);
std::string FragmentShaderDesc(const ShaderID &id);
2 changes: 1 addition & 1 deletion GPU/D3D11/GPU_D3D11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ GPU_D3D11::GPU_D3D11(GraphicsContext *gfxCtx, Draw::DrawContext *draw)

stockD3D11.Create(device_);

shaderManagerD3D11_ = new ShaderManagerD3D11(device_, context_, featureLevel);
shaderManagerD3D11_ = new ShaderManagerD3D11(draw, device_, context_, featureLevel);
framebufferManagerD3D11_ = new FramebufferManagerD3D11(draw);
framebufferManager_ = framebufferManagerD3D11_;
textureCacheD3D11_ = new TextureCacheD3D11(draw);
Expand Down
7 changes: 4 additions & 3 deletions GPU/D3D11/ShaderManagerD3D11.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "math/lin/matrix4x4.h"
#include "math/math_util.h"
#include "math/dataconv.h"
#include "thin3d/thin3d.h"
#include "util/text/utf8.h"
#include "Common/Common.h"
#include "Core/Config.h"
Expand Down Expand Up @@ -88,8 +89,8 @@ std::string D3D11VertexShader::GetShaderString(DebugShaderStringType type) const
}
}

ShaderManagerD3D11::ShaderManagerD3D11(ID3D11Device *device, ID3D11DeviceContext *context, D3D_FEATURE_LEVEL featureLevel)
: device_(device), context_(context), featureLevel_(featureLevel), lastVShader_(nullptr), lastFShader_(nullptr) {
ShaderManagerD3D11::ShaderManagerD3D11(Draw::DrawContext *draw, ID3D11Device *device, ID3D11DeviceContext *context, D3D_FEATURE_LEVEL featureLevel)
: ShaderManagerCommon(draw), device_(device), context_(context), featureLevel_(featureLevel), lastVShader_(nullptr), lastFShader_(nullptr) {
codeBuffer_ = new char[16384];
memset(&ub_base, 0, sizeof(ub_base));
memset(&ub_lights, 0, sizeof(ub_lights));
Expand Down Expand Up @@ -190,7 +191,7 @@ void ShaderManagerD3D11::GetShaders(int prim, u32 vertType, D3D11VertexShader **

if (gstate_c.IsDirty(DIRTY_FRAGMENTSHADER_STATE)) {
gstate_c.Clean(DIRTY_FRAGMENTSHADER_STATE);
ComputeFragmentShaderID(&FSID);
ComputeFragmentShaderID(&FSID, draw_->GetBugs());
} else {
FSID = lastFSID_;
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/D3D11/ShaderManagerD3D11.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ class D3D11PushBuffer;

class ShaderManagerD3D11 : public ShaderManagerCommon {
public:
ShaderManagerD3D11(ID3D11Device *device, ID3D11DeviceContext *context, D3D_FEATURE_LEVEL featureLevel);
ShaderManagerD3D11(Draw::DrawContext *draw, ID3D11Device *device, ID3D11DeviceContext *context, D3D_FEATURE_LEVEL featureLevel);
~ShaderManagerD3D11();

void GetShaders(int prim, u32 vertType, D3D11VertexShader **vshader, D3D11FragmentShader **fshader, bool useHWTransform);
Expand Down
2 changes: 1 addition & 1 deletion GPU/Directx9/GPU_DX9.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ GPU_DX9::GPU_DX9(GraphicsContext *gfxCtx, Draw::DrawContext *draw)
lastVsync_ = g_Config.bVSync ? 1 : 0;
dxstate.SetVSyncInterval(g_Config.bVSync);

shaderManagerDX9_ = new ShaderManagerDX9(device_);
shaderManagerDX9_ = new ShaderManagerDX9(draw, device_);
framebufferManagerDX9_ = new FramebufferManagerDX9(draw);
framebufferManager_ = framebufferManagerDX9_;
textureCacheDX9_ = new TextureCacheDX9(draw);
Expand Down
6 changes: 4 additions & 2 deletions GPU/Directx9/ShaderManagerDX9.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#include "math/lin/matrix4x4.h"
#include "math/math_util.h"
#include "math/dataconv.h"
#include "thin3d/thin3d.h"
#include "util/text/utf8.h"

#include "Common/Common.h"
Expand Down Expand Up @@ -499,7 +500,8 @@ void ShaderManagerDX9::VSUpdateUniforms(u64 dirtyUniforms) {
}
}

ShaderManagerDX9::ShaderManagerDX9(LPDIRECT3DDEVICE9 device) : device_(device), lastVShader_(nullptr), lastPShader_(nullptr) {
ShaderManagerDX9::ShaderManagerDX9(Draw::DrawContext *draw, LPDIRECT3DDEVICE9 device)
: ShaderManagerCommon(draw), device_(device), lastVShader_(nullptr), lastPShader_(nullptr) {
codeBuffer_ = new char[16384];
}

Expand Down Expand Up @@ -554,7 +556,7 @@ VSShader *ShaderManagerDX9::ApplyShader(int prim, u32 vertType) {
FShaderID FSID;
if (gstate_c.IsDirty(DIRTY_FRAGMENTSHADER_STATE)) {
gstate_c.Clean(DIRTY_FRAGMENTSHADER_STATE);
ComputeFragmentShaderID(&FSID);
ComputeFragmentShaderID(&FSID, draw_->GetBugs());
} else {
FSID = lastFSID_;
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/Directx9/ShaderManagerDX9.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class VSShader {

class ShaderManagerDX9 : public ShaderManagerCommon {
public:
ShaderManagerDX9(LPDIRECT3DDEVICE9 device);
ShaderManagerDX9(Draw::DrawContext *draw, LPDIRECT3DDEVICE9 device);
~ShaderManagerDX9();

void ClearCache(bool deleteThem); // TODO: deleteThem currently not respected
Expand Down
36 changes: 1 addition & 35 deletions GPU/GLES/GPU_GLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,26 +142,6 @@ GPU_GLES::~GPU_GLES() {
delete textureCacheGL_;
}

static constexpr int MakeIntelSimpleVer(int v1, int v2, int v3) {
return (v1 << 16) | (v2 << 8) | v3;
}

static bool HasIntelDualSrcBug(int versions[4]) {
// Intel uses a confusing set of at least 3 version numbering schemes. This is the one given to OpenGL.
switch (MakeIntelSimpleVer(versions[0], versions[1], versions[2])) {
case MakeIntelSimpleVer(9, 17, 10):
case MakeIntelSimpleVer(9, 18, 10):
return false;
case MakeIntelSimpleVer(10, 18, 10):
return versions[3] < 4061;
case MakeIntelSimpleVer(10, 18, 14):
return versions[3] < 4080;
default:
// Older than above didn't support dual src anyway, newer should have the fix.
return false;
}
}

// Take the raw GL extension and versioning data and turn into feature flags.
void GPU_GLES::CheckGPUFeatures() {
u32 features = 0;
Expand All @@ -170,21 +150,7 @@ void GPU_GLES::CheckGPUFeatures() {
features |= GPU_SUPPORTS_VS_RANGE_CULLING;

if (gl_extensions.ARB_blend_func_extended || gl_extensions.EXT_blend_func_extended) {
if (!gl_extensions.VersionGEThan(3, 0, 0)) {
// Don't use this extension on sub 3.0 OpenGL versions as it does not seem reliable
} else if (gl_extensions.gpuVendor == GPU_VENDOR_INTEL) {
// Also on Intel, see https://github.com/hrydgard/ppsspp/issues/10117
// TODO: Remove entirely sometime reasonably far in driver years after 2015.
const std::string ver = draw_->GetInfoString(Draw::InfoField::APIVERSION);
int versions[4]{};
if (sscanf(ver.c_str(), "Build %d.%d.%d.%d", &versions[0], &versions[1], &versions[2], &versions[3]) == 4) {
if (!HasIntelDualSrcBug(versions)) {
features |= GPU_SUPPORTS_DUALSOURCE_BLEND;
}
} else {
features |= GPU_SUPPORTS_DUALSOURCE_BLEND;
}
} else {
if (!g_Config.bVendorBugChecksEnabled || !draw_->GetBugs().Has(Draw::Bugs::DUAL_SOURCE_BLENDING_BROKEN)) {
features |= GPU_SUPPORTS_DUALSOURCE_BLEND;
}
}
Expand Down
8 changes: 5 additions & 3 deletions GPU/GLES/ShaderManagerGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,12 @@
#include "base/timeutil.h"
#include "gfx/gl_debug_log.h"
#include "gfx_es2/gpu_features.h"
#include "thin3d/GLRenderManager.h"
#include "i18n/i18n.h"
#include "math/math_util.h"
#include "math/lin/matrix4x4.h"
#include "profiler/profiler.h"
#include "thin3d/thin3d.h"
#include "thin3d/GLRenderManager.h"

#include "Common/FileUtil.h"
#include "Core/Config.h"
Expand Down Expand Up @@ -571,7 +572,7 @@ void LinkedShader::UpdateUniforms(u32 vertType, const ShaderID &vsid) {
}

ShaderManagerGLES::ShaderManagerGLES(Draw::DrawContext *draw)
: lastShader_(nullptr), shaderSwitchDirtyUniforms_(0), diskCacheDirty_(false), fsCache_(16), vsCache_(16) {
: ShaderManagerCommon(draw), lastShader_(nullptr), shaderSwitchDirtyUniforms_(0), diskCacheDirty_(false), fsCache_(16), vsCache_(16) {
render_ = (GLRenderManager *)draw->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
codeBuffer_ = new char[16384];
lastFSID_.set_invalid();
Expand Down Expand Up @@ -610,6 +611,7 @@ void ShaderManagerGLES::DeviceLost() {

void ShaderManagerGLES::DeviceRestore(Draw::DrawContext *draw) {
render_ = (GLRenderManager *)draw->GetNativeObject(Draw::NativeObject::RENDER_MANAGER);
draw_ = draw;
}

void ShaderManagerGLES::DirtyShader() {
Expand Down Expand Up @@ -701,7 +703,7 @@ LinkedShader *ShaderManagerGLES::ApplyFragmentShader(VShaderID VSID, Shader *vs,
FShaderID FSID;
if (gstate_c.IsDirty(DIRTY_FRAGMENTSHADER_STATE)) {
gstate_c.Clean(DIRTY_FRAGMENTSHADER_STATE);
ComputeFragmentShaderID(&FSID);
ComputeFragmentShaderID(&FSID, draw_->GetBugs());
} else {
FSID = lastFSID_;
}
Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/TextureCacheGLES.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -672,7 +672,7 @@ void TextureCacheGLES::BuildTexture(TexCacheEntry *const entry) {
}
} else {
// Avoid PowerVR driver bug
if (canAutoGen && w > 1 && h > 1 && !(h > w && (gl_extensions.bugs & BUG_PVR_GENMIPMAP_HEIGHT_GREATER))) { // Really! only seems to fail if height > width
if (canAutoGen && w > 1 && h > 1 && !(h > w && draw_->GetBugs().Has(Draw::Bugs::PVR_GENMIPMAP_HEIGHT_GREATER))) { // Really! only seems to fail if height > width
// NOTICE_LOG(G3D, "Generating mipmap for texture sized %dx%d%d", w, h, (int)format);
genMips = true;
} else {
Expand Down
10 changes: 3 additions & 7 deletions GPU/Vulkan/FragmentShaderGeneratorVulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,11 @@ bool GenerateVulkanGLSLFragmentShader(const FShaderID &id, char *buffer, uint32_

const char *shading = doFlatShading ? "flat" : "";
bool earlyFragmentTests = ((!enableAlphaTest && !enableColorTest) || testForceToZero) && !gstate_c.Supports(GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT);
bool hasStencilOutput = stencilToAlpha != REPLACE_ALPHA_NO || id.Bit(FS_BIT_REPLACE_ALPHA_WITH_STENCIL_TYPE) == 0;

// TODO: This is a bug affecting shader cache generality - we CANNOT check anything but the shader ID and (indirectly) the game ID in here really.
// Need to move this check somehow to the shader ID generator. That's tricky though because it's generic...
bool isAdreno = vulkanVendorId == VULKAN_VENDOR_QUALCOMM && g_Config.bVendorBugChecksEnabled;
bool useAdrenoBugWorkaround = id.Bit(FS_BIT_NO_DEPTH_CANNOT_DISCARD_STENCIL);

if (earlyFragmentTests) {
WRITE(p, "layout (early_fragment_tests) in;\n");
} else if (isAdreno && hasStencilOutput && !gstate_c.Supports(GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT)) {
} else if (useAdrenoBugWorkaround && !gstate_c.Supports(GPU_ROUND_FRAGMENT_DEPTH_TO_16BIT)) {
WRITE(p, "layout (depth_unchanged) out float gl_FragDepth;\n");
}

Expand Down Expand Up @@ -588,7 +584,7 @@ bool GenerateVulkanGLSLFragmentShader(const FShaderID &id, char *buffer, uint32_
WRITE(p, " z = (1.0/65535.0) * floor(z * 65535.0);\n");
}
WRITE(p, " gl_FragDepth = z;\n");
} else if (!earlyFragmentTests && isAdreno && hasStencilOutput) {
} else if (!earlyFragmentTests && useAdrenoBugWorkaround) {
// Adreno (and possibly MESA/others) apply early frag tests even with discard in the shader.
// Writing depth prevents the bug, even with depth_unchanged specified.
WRITE(p, " gl_FragDepth = gl_FragCoord.z;\n");
Expand Down
20 changes: 3 additions & 17 deletions GPU/Vulkan/GPU_Vulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ GPU_Vulkan::GPU_Vulkan(GraphicsContext *gfxCtx, Draw::DrawContext *draw)
UpdateVsyncInterval(true);
CheckGPUFeatures();

shaderManagerVulkan_ = new ShaderManagerVulkan(vulkan_);
shaderManagerVulkan_ = new ShaderManagerVulkan(draw, vulkan_);
pipelineManager_ = new PipelineManagerVulkan(vulkan_);
framebufferManagerVulkan_ = new FramebufferManagerVulkan(draw, vulkan_);
framebufferManager_ = framebufferManagerVulkan_;
Expand Down Expand Up @@ -224,23 +224,9 @@ void GPU_Vulkan::CheckGPUFeatures() {
features |= GPU_SUPPORTS_DEPTH_CLAMP;
}
if (vulkan_->GetFeaturesEnabled().dualSrcBlend) {
switch (vulkan_->GetPhysicalDeviceProperties(vulkan_->GetCurrentPhysicalDevice()).vendorID) {
// We thought we had a bug here on nVidia but turns out we accidentally #ifdef-ed out crucial
// code on Android.
case VULKAN_VENDOR_INTEL:
// Workaround for Intel driver bug. TODO: Re-enable after some driver version
break;
case VULKAN_VENDOR_AMD:
// See issue #10074, and also #10065 (AMD) and #10109 for the choice of the driver version to check for
if (vulkan_->GetPhysicalDeviceProperties(vulkan_->GetCurrentPhysicalDevice()).driverVersion >= 0x00407000)
features |= GPU_SUPPORTS_DUALSOURCE_BLEND;
break;
default:
if (!g_Config.bVendorBugChecksEnabled || !draw_->GetBugs().Has(Draw::Bugs::DUAL_SOURCE_BLENDING_BROKEN)) {
features |= GPU_SUPPORTS_DUALSOURCE_BLEND;
break;
}
if (!g_Config.bVendorBugChecksEnabled)
features |= GPU_SUPPORTS_DUALSOURCE_BLEND;
}
if (vulkan_->GetFeaturesEnabled().logicOp) {
features |= GPU_SUPPORTS_LOGIC_OP;
Expand Down Expand Up @@ -535,7 +521,7 @@ void GPU_Vulkan::DeviceRestore() {
drawEngine_.DeviceRestore(vulkan_, draw_);
pipelineManager_->DeviceRestore(vulkan_);
textureCacheVulkan_->DeviceRestore(vulkan_, draw_);
shaderManagerVulkan_->DeviceRestore(vulkan_);
shaderManagerVulkan_->DeviceRestore(vulkan_, draw_);
depalShaderCache_.DeviceRestore(draw_, vulkan_);
}

Expand Down
10 changes: 6 additions & 4 deletions GPU/Vulkan/ShaderManagerVulkan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include "math/math_util.h"
#include "math/dataconv.h"
#include "profiler/profiler.h"
#include "thin3d/thin3d.h"
#include "util/text/utf8.h"
#include "Common/Vulkan/VulkanContext.h"
#include "Common/Vulkan/VulkanMemory.h"
Expand Down Expand Up @@ -156,8 +157,8 @@ std::string VulkanVertexShader::GetShaderString(DebugShaderStringType type) cons
}
}

ShaderManagerVulkan::ShaderManagerVulkan(VulkanContext *vulkan)
: vulkan_(vulkan), lastVShader_(nullptr), lastFShader_(nullptr), fsCache_(16), vsCache_(16) {
ShaderManagerVulkan::ShaderManagerVulkan(Draw::DrawContext *draw, VulkanContext *vulkan)
: ShaderManagerCommon(draw), vulkan_(vulkan), lastVShader_(nullptr), lastFShader_(nullptr), fsCache_(16), vsCache_(16) {
codeBuffer_ = new char[16384];
uboAlignment_ = vulkan_->GetPhysicalDeviceProperties(vulkan_->GetCurrentPhysicalDevice()).limits.minUniformBufferOffsetAlignment;
memset(&ub_base, 0, sizeof(ub_base));
Expand All @@ -174,8 +175,9 @@ ShaderManagerVulkan::~ShaderManagerVulkan() {
delete[] codeBuffer_;
}

void ShaderManagerVulkan::DeviceRestore(VulkanContext *vulkan) {
void ShaderManagerVulkan::DeviceRestore(VulkanContext *vulkan, Draw::DrawContext *draw) {
vulkan_ = vulkan;
draw_ = draw;
uboAlignment_ = vulkan_->GetPhysicalDeviceProperties(vulkan_->GetCurrentPhysicalDevice()).limits.minUniformBufferOffsetAlignment;
}

Expand Down Expand Up @@ -238,7 +240,7 @@ void ShaderManagerVulkan::GetShaders(int prim, u32 vertType, VulkanVertexShader
FShaderID FSID;
if (gstate_c.IsDirty(DIRTY_FRAGMENTSHADER_STATE)) {
gstate_c.Clean(DIRTY_FRAGMENTSHADER_STATE);
ComputeFragmentShaderID(&FSID);
ComputeFragmentShaderID(&FSID, draw_->GetBugs());
} else {
FSID = lastFSID_;
}
Expand Down
Loading

0 comments on commit 8e17caf

Please sign in to comment.