Skip to content
Permalink
Browse files

VideoCommon: Remove unnecessary memset on ShaderUid instances.

Zero-initialization zeroes out all members and padding bits, so this is
safe to do. While we're at it, also add static assertions that enforce
the necessary requirements of a UID type explicitly within the ShaderUid
class.

This way, we can remove several memset calls around the shader
generation code that makes sure the underlying UID data is zeroed out.
Now our ShaderUid class enforces this for us, so we don't need to care about
it at the usage sites.
  • Loading branch information...
lioncash committed May 30, 2019
1 parent 80d8173 commit 149a97e396fd1ac22df8ad9d47d4a23e3ef46d25
@@ -5,7 +5,6 @@
#include "VideoCommon/GeometryShaderGen.h"

#include <cmath>
#include <cstring>

#include "Common/CommonTypes.h"
#include "VideoCommon/DriverDetails.h"
@@ -27,10 +26,9 @@ bool geometry_shader_uid_data::IsPassthrough() const

GeometryShaderUid GetGeometryShaderUid(PrimitiveType primitive_type)
{
ShaderUid<geometry_shader_uid_data> out;
geometry_shader_uid_data* uid_data = out.GetUidData<geometry_shader_uid_data>();
memset(uid_data, 0, sizeof(geometry_shader_uid_data));
GeometryShaderUid out;

geometry_shader_uid_data* const uid_data = out.GetUidData();
uid_data->primitive_type = static_cast<u32>(primitive_type);
uid_data->numTexGens = xfmem.numTexGen.numTexGens;

@@ -364,15 +362,14 @@ static void EndPrimitive(ShaderCode& out, const ShaderHostConfig& host_config,
void EnumerateGeometryShaderUids(const std::function<void(const GeometryShaderUid&)>& callback)
{
GeometryShaderUid uid;
std::memset(&uid, 0, sizeof(uid));

const std::array<PrimitiveType, 3> primitive_lut = {
{g_ActiveConfig.backend_info.bSupportsPrimitiveRestart ? PrimitiveType::TriangleStrip :
PrimitiveType::Triangles,
PrimitiveType::Lines, PrimitiveType::Points}};
for (PrimitiveType primitive : primitive_lut)
{
auto* guid = uid.GetUidData<geometry_shader_uid_data>();
geometry_shader_uid_data* const guid = uid.GetUidData();
guid->primitive_type = static_cast<u32>(primitive);

for (u32 texgens = 0; texgens <= 8; texgens++)
@@ -12,7 +12,6 @@
enum class APIType;

#pragma pack(1)

struct geometry_shader_uid_data
{
u32 NumValues() const { return sizeof(geometry_shader_uid_data); }
@@ -21,10 +20,9 @@ struct geometry_shader_uid_data
u32 numTexGens : 4;
u32 primitive_type : 2;
};

#pragma pack()

typedef ShaderUid<geometry_shader_uid_data> GeometryShaderUid;
using GeometryShaderUid = ShaderUid<geometry_shader_uid_data>;

ShaderCode GenerateGeometryShaderCode(APIType ApiType, const ShaderHostConfig& host_config,
const geometry_shader_uid_data* uid_data);
@@ -6,7 +6,6 @@

#include <cmath>
#include <cstdio>
#include <cstring>

#include "Common/Assert.h"
#include "Common/CommonTypes.h"
@@ -162,9 +161,8 @@ static const char* tevAOutputTable[] = {"prev.a", "c0.a", "c1.a", "c2.a"};
PixelShaderUid GetPixelShaderUid()
{
PixelShaderUid out;
pixel_shader_uid_data* uid_data = out.GetUidData<pixel_shader_uid_data>();
memset(uid_data, 0, sizeof(*uid_data));

pixel_shader_uid_data* const uid_data = out.GetUidData();
uid_data->useDstAlpha = bpmem.dstalpha.enable && bpmem.blendmode.alphaupdate &&
bpmem.zcontrol.pixel_format == PEControl::RGBA6_Z24;

@@ -340,7 +338,7 @@ PixelShaderUid GetPixelShaderUid()
void ClearUnusedPixelShaderUidBits(APIType ApiType, const ShaderHostConfig& host_config,
PixelShaderUid* uid)
{
pixel_shader_uid_data* uid_data = uid->GetUidData<pixel_shader_uid_data>();
pixel_shader_uid_data* const uid_data = uid->GetUidData();

// OpenGL and Vulkan convert implicitly normalized color outputs to their uint representation.
// Therefore, it is not necessary to use a uint output on these backends. We also disable the
@@ -162,7 +162,7 @@ struct pixel_shader_uid_data
};
#pragma pack()

typedef ShaderUid<pixel_shader_uid_data> PixelShaderUid;
using PixelShaderUid = ShaderUid<pixel_shader_uid_data>;

ShaderCode GeneratePixelShaderCode(APIType ApiType, const ShaderHostConfig& host_config,
const pixel_shader_uid_data* uid_data);
@@ -8,6 +8,7 @@
#include <cstring>
#include <map>
#include <string>
#include <type_traits>
#include <vector>

#include "Common/CommonTypes.h"
@@ -48,17 +49,6 @@ class ShaderGeneratorInterface
* Tells us that a specific constant range (including last_index) is being used by the shader
*/
void SetConstantsUsed(unsigned int first_index, unsigned int last_index) {}
/*
* Returns a pointer to an internally stored object of the uid_data type.
* @warning since most child classes use the default implementation you shouldn't access this
* directly without adding precautions against nullptr access (e.g. via adding a dummy structure,
* cf. the vertex/pixel shader generators)
*/
template <class uid_data>
uid_data* GetUidData()
{
return nullptr;
}
};

/*
@@ -74,6 +64,9 @@ template <class uid_data>
class ShaderUid : public ShaderGeneratorInterface
{
public:
static_assert(std::is_trivially_copyable_v<uid_data>,
"uid_data must be a trivially copyable type");

bool operator==(const ShaderUid& obj) const
{
return memcmp(this->values, obj.values, data.NumValues() * sizeof(*values)) == 0;
@@ -90,19 +83,22 @@ class ShaderUid : public ShaderGeneratorInterface
return memcmp(this->values, obj.values, data.NumValues() * sizeof(*values)) < 0;
}

template <class uid_data2>
uid_data2* GetUidData()
{
return &data;
}
// Returns a pointer to an internally stored object of the uid_data type.
uid_data* GetUidData() { return &data; }

// Returns a pointer to an internally stored object of the uid_data type.
const uid_data* GetUidData() const { return &data; }

// Returns the raw bytes that make up the shader UID.
const u8* GetUidDataRaw() const { return &values[0]; }

// Returns the size of the underlying UID data structure in bytes.
size_t GetUidDataSize() const { return sizeof(values); }

private:
union
{
uid_data data;
uid_data data{};
u8 values[sizeof(uid_data)];
};
};
@@ -2,13 +2,11 @@
// Licensed under GPLv2+
// Refer to the license.txt file included.

#include <array>
#include <cstring>
#include "VideoCommon/TextureConverterShaderGen.h"

#include "Common/Assert.h"
#include "Common/CommonTypes.h"
#include "VideoCommon/BPMemory.h"
#include "VideoCommon/TextureConverterShaderGen.h"
#include "VideoCommon/VideoCommon.h"
#include "VideoCommon/VideoConfig.h"

@@ -18,9 +16,8 @@ TCShaderUid GetShaderUid(EFBCopyFormat dst_format, bool is_depth_copy, bool is_i
bool scale_by_half, bool copy_filter)
{
TCShaderUid out;
UidData* uid_data = out.GetUidData<UidData>();
memset(uid_data, 0, sizeof(*uid_data));

UidData* const uid_data = out.GetUidData();
uid_data->dst_format = dst_format;
uid_data->efb_has_alpha = bpmem.zcontrol.pixel_format == PEControl::RGBA6_Z24;
uid_data->is_depth_copy = is_depth_copy;
@@ -14,8 +14,8 @@ namespace UberShader
PixelShaderUid GetPixelShaderUid()
{
PixelShaderUid out;
pixel_ubershader_uid_data* uid_data = out.GetUidData<pixel_ubershader_uid_data>();
memset(uid_data, 0, sizeof(*uid_data));

pixel_ubershader_uid_data* const uid_data = out.GetUidData();
uid_data->num_texgens = xfmem.numTexGen.numTexGens;
uid_data->early_depth =
bpmem.UseEarlyDepthTest() &&
@@ -26,13 +26,14 @@ PixelShaderUid GetPixelShaderUid()
(!g_ActiveConfig.bFastDepthCalc && bpmem.zmode.testenable && !uid_data->early_depth) ||
(bpmem.zmode.testenable && bpmem.genMode.zfreeze);
uid_data->uint_output = bpmem.blendmode.UseLogicOp();

return out;
}

void ClearUnusedPixelShaderUidBits(APIType ApiType, const ShaderHostConfig& host_config,
PixelShaderUid* uid)
{
pixel_ubershader_uid_data* uid_data = uid->GetUidData<pixel_ubershader_uid_data>();
pixel_ubershader_uid_data* const uid_data = uid->GetUidData();

// OpenGL and Vulkan convert implicitly normalized color outputs to their uint representation.
// Therefore, it is not necessary to use a uint output on these backends. We also disable the
@@ -1390,11 +1391,10 @@ ShaderCode GenPixelShader(APIType ApiType, const ShaderHostConfig& host_config,
void EnumeratePixelShaderUids(const std::function<void(const PixelShaderUid&)>& callback)
{
PixelShaderUid uid;
std::memset(&uid, 0, sizeof(uid));

for (u32 texgens = 0; texgens <= 8; texgens++)
{
auto* puid = uid.GetUidData<UberShader::pixel_ubershader_uid_data>();
pixel_ubershader_uid_data* const puid = uid.GetUidData();
puid->num_texgens = texgens;

for (u32 early_depth = 0; early_depth < 2; early_depth++)
@@ -21,7 +21,7 @@ struct pixel_ubershader_uid_data
};
#pragma pack()

typedef ShaderUid<pixel_ubershader_uid_data> PixelShaderUid;
using PixelShaderUid = ShaderUid<pixel_ubershader_uid_data>;

PixelShaderUid GetPixelShaderUid();

@@ -15,9 +15,10 @@ namespace UberShader
VertexShaderUid GetVertexShaderUid()
{
VertexShaderUid out;
vertex_ubershader_uid_data* uid_data = out.GetUidData<vertex_ubershader_uid_data>();
memset(uid_data, 0, sizeof(*uid_data));

vertex_ubershader_uid_data* const uid_data = out.GetUidData();
uid_data->num_texgens = xfmem.numTexGen.numTexGens;

return out;
}

@@ -487,11 +488,10 @@ void GenVertexShaderTexGens(APIType ApiType, u32 numTexgen, ShaderCode& out)
void EnumerateVertexShaderUids(const std::function<void(const VertexShaderUid&)>& callback)
{
VertexShaderUid uid;
std::memset(&uid, 0, sizeof(uid));

for (u32 texgens = 0; texgens <= 8; texgens++)
{
auto* vuid = uid.GetUidData<UberShader::vertex_ubershader_uid_data>();
vertex_ubershader_uid_data* const vuid = uid.GetUidData();
vuid->num_texgens = texgens;
callback(uid);
}
@@ -18,7 +18,7 @@ struct vertex_ubershader_uid_data
};
#pragma pack()

typedef ShaderUid<vertex_ubershader_uid_data> VertexShaderUid;
using VertexShaderUid = ShaderUid<vertex_ubershader_uid_data>;

VertexShaderUid GetVertexShaderUid();

@@ -2,28 +2,25 @@
// Licensed under GPLv2+
// Refer to the license.txt file included.

#include <cstring>
#include "VideoCommon/VertexShaderGen.h"

#include "Common/Assert.h"
#include "Common/CommonTypes.h"
#include "VideoCommon/BPMemory.h"
#include "VideoCommon/LightingShaderGen.h"
#include "VideoCommon/NativeVertexFormat.h"
#include "VideoCommon/VertexLoaderManager.h"
#include "VideoCommon/VertexShaderGen.h"
#include "VideoCommon/VideoCommon.h"
#include "VideoCommon/VideoConfig.h"
#include "VideoCommon/XFMemory.h"

VertexShaderUid GetVertexShaderUid()
{
VertexShaderUid out;
vertex_shader_uid_data* uid_data = out.GetUidData<vertex_shader_uid_data>();
memset(uid_data, 0, sizeof(*uid_data));

ASSERT(bpmem.genMode.numtexgens == xfmem.numTexGen.numTexGens);
ASSERT(bpmem.genMode.numcolchans == xfmem.numChan.numColorChans);

VertexShaderUid out;
vertex_shader_uid_data* const uid_data = out.GetUidData();
uid_data->numTexGens = xfmem.numTexGen.numTexGens;
uid_data->components = VertexLoaderManager::g_current_components;
uid_data->numColorChans = xfmem.numChan.numColorChans;
@@ -65,7 +65,7 @@ struct vertex_shader_uid_data
};
#pragma pack()

typedef ShaderUid<vertex_shader_uid_data> VertexShaderUid;
using VertexShaderUid = ShaderUid<vertex_shader_uid_data>;

VertexShaderUid GetVertexShaderUid();
ShaderCode GenerateVertexShaderCode(APIType api_type, const ShaderHostConfig& host_config,

0 comments on commit 149a97e

Please sign in to comment.
You can’t perform that action at this time.