Skip to content

Commit

Permalink
Work on sanitizing use of sizeof and buffer sizes
Browse files Browse the repository at this point in the history
  • Loading branch information
@colincornaby
colincornaby committed Nov 6, 2023
1 parent 2e01bf9 commit 10e2068
Show file tree
Hide file tree
Showing 6 changed files with 104 additions and 42 deletions.
105 changes: 82 additions & 23 deletions Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalDevice.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,77 @@ You can contact Cyan Worlds, Inc. by email [email protected]
#include "pfMetalPipeline/plMetalPipelineState.h"
#include "pfMetalPipeline/ShaderSrc/ShaderTypes.h"

/// Macros for getting/setting data in a vertex buffer
template<typename T>
static inline void inlCopy(uint8_t*& src, uint8_t*& dst)
{
T* src_ptr = reinterpret_cast<T*>(src);
T* dst_ptr = reinterpret_cast<T*>(dst);
*dst_ptr = *src_ptr;
src += sizeof(T);
dst += sizeof(T);
}

static inline void inlCopy(const uint8_t*& src, uint8_t*& dst, size_t sz)
{
memcpy(dst, src, sz);
src += sz;
dst += sz;
}

template<typename T>
static inline const uint8_t* inlExtract(const uint8_t* src, T* val)
{
const T* ptr = reinterpret_cast<const T*>(src);
*val = *ptr++;
return reinterpret_cast<const uint8_t*>(ptr);
}

template<>
inline const uint8_t* inlExtract<hsPoint3>(const uint8_t* src, hsPoint3* val)
{
const float* src_ptr = reinterpret_cast<const float*>(src);
float* dst_ptr = reinterpret_cast<float*>(val);
*dst_ptr++ = *src_ptr++;
*dst_ptr++ = *src_ptr++;
*dst_ptr++ = *src_ptr++;
*dst_ptr = 1.f;
return reinterpret_cast<const uint8_t*>(src_ptr);
}

template<>
inline const uint8_t* inlExtract<hsVector3>(const uint8_t* src, hsVector3* val)
{
const float* src_ptr = reinterpret_cast<const float*>(src);
float* dst_ptr = reinterpret_cast<float*>(val);
*dst_ptr++ = *src_ptr++;
*dst_ptr++ = *src_ptr++;
*dst_ptr++ = *src_ptr++;
*dst_ptr = 0.f;
return reinterpret_cast<const uint8_t*>(src_ptr);
}

template<typename T, size_t N>
static inline void inlSkip(uint8_t*& src)
{
src += sizeof(T) * N;
}

template<typename T>
static inline uint8_t* inlStuff(uint8_t* dst, const T* val)
{
T* ptr = reinterpret_cast<T*>(dst);
*ptr++ = *val;
return reinterpret_cast<uint8_t*>(ptr);
}

matrix_float4x4* hsMatrix2SIMD(const hsMatrix44& src, matrix_float4x4* dst)
{
constexpr auto matrixSize = sizeof(matrix_float4x4);
if (src.fFlags & hsMatrix44::kIsIdent) {
memcpy(dst, &matrix_identity_float4x4, sizeof(float) * 16);
memcpy(dst, &matrix_identity_float4x4, matrixSize);
} else {
memcpy(dst, &src.fMap, sizeof(matrix_float4x4));
memcpy(dst, &src.fMap, matrixSize);
}

return dst;
Expand Down Expand Up @@ -170,11 +235,11 @@ void plMetalDevice::Clear(bool shouldClearColor, simd_float4 clearColor, bool sh
// we're actually drawing to screen.

if (fCurrentRenderTargetCommandEncoder) {
half4 halfClearColor;
halfClearColor[0] = clearColor.r;
halfClearColor[1] = clearColor.g;
halfClearColor[2] = clearColor.b;
halfClearColor[3] = clearColor.a;
half4 clearColor;
clearColor[0] = clearColor.r;
clearColor[1] = clearColor.g;
clearColor[2] = clearColor.b;
clearColor[3] = clearColor.a;
plMetalDevice::plMetalLinkedPipeline* linkedPipeline = plMetalClearPipelineState(this, shouldClearColor, shouldClearDepth).GetRenderPipelineState();

const MTL::RenderPipelineState* pipelineState = linkedPipeline->pipelineState;
Expand All @@ -190,7 +255,7 @@ void plMetalDevice::Clear(bool shouldClearColor, simd_float4 clearColor, bool sh

CurrentRenderCommandEncoder()->setCullMode(MTL::CullModeNone);
CurrentRenderCommandEncoder()->setVertexBytes(&clearCoords, sizeof(clearCoords), 0);
CurrentRenderCommandEncoder()->setFragmentBytes(&halfClearColor, sizeof(halfClearColor), 0);
CurrentRenderCommandEncoder()->setFragmentBytes(&clearColor, sizeof(clearColor), 0);
CurrentRenderCommandEncoder()->setFragmentBytes(&clearDepth, sizeof(float), 1);
CurrentRenderCommandEncoder()->drawPrimitives(MTL::PrimitiveTypeTriangleStrip, NS::UInteger(0), NS::UInteger(4));
} else {
Expand Down Expand Up @@ -434,7 +499,7 @@ bool plMetalDevice::BeginRender()

static uint32_t IGetBufferFormatSize(uint8_t format)
{
uint32_t size = sizeof(float) * 6 + sizeof(uint32_t) * 2; // Position and normal, and two packed colors
uint32_t size = sizeof(hsPoint3) * 2 + sizeof(hsColor32) * 2; // Position and normal, and two packed colors

switch (format & plGBufferGroup::kSkinWeightMask) {
case plGBufferGroup::kSkinNoWeights:
Expand All @@ -446,7 +511,7 @@ static uint32_t IGetBufferFormatSize(uint8_t format)
hsAssert(false, "Invalid skin weight value in IGetBufferFormatSize()");
}

size += sizeof(float) * 3 * plGBufferGroup::CalcNumUVs(format);
size += sizeof(hsPoint3) * plGBufferGroup::CalcNumUVs(format);

return size;
}
Expand Down Expand Up @@ -579,26 +644,20 @@ void plMetalDevice::FillVolatileVertexBufferRef(plMetalDevice::VertexBufferRef*
uint8_t* dst = ref->fData;
uint8_t* src = group->GetVertBufferData(idx);

size_t uvChanSize = plGBufferGroup::CalcNumUVs(group->GetVertexFormat()) * sizeof(float) * 3;
size_t uvChanSize = plGBufferGroup::CalcNumUVs(group->GetVertexFormat()) * sizeof(hsPoint3);
uint8_t numWeights = (group->GetVertexFormat() & plGBufferGroup::kSkinWeightMask) >> 4;

for (uint32_t i = 0; i < ref->fCount; ++i) {
memcpy(dst, src, sizeof(hsPoint3)); // pre-pos
dst += sizeof(hsPoint3);
src += sizeof(hsPoint3);
inlCopy<hsPoint3>(src, dst); // pre-pos

src += numWeights * sizeof(float); // weights

if (group->GetVertexFormat() & plGBufferGroup::kSkinIndices)
src += sizeof(uint32_t); // indices

memcpy(dst, src, sizeof(hsVector3)); // pre-normal
dst += sizeof(hsVector3);
src += sizeof(hsVector3);

memcpy(dst, src, sizeof(uint32_t) * 2); // diffuse & specular
dst += sizeof(uint32_t) * 2;
src += sizeof(uint32_t) * 2;
inlSkip<uint32_t, 1>(src); // indices

inlCopy<hsVector3>(src, dst); // pre-normal
inlCopy<uint32_t>(src, dst); // diffuse
inlCopy<uint32_t>(src, dst); // specular

// UVWs
memcpy(dst, src, uvChanSize);
Expand Down
2 changes: 1 addition & 1 deletion Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalFragmentShader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ bool plMetalFragmentShader::ISetConstants(plMetalPipeline* pipe)
{
if (fOwner->GetNumConsts()) {
float* ptr = (float*)fOwner->GetConstBasePtr();
pipe->GetMetalDevice()->CurrentRenderCommandEncoder()->setFragmentBytes(ptr, fOwner->GetNumConsts() * sizeof(float) * 4, VertexShaderArgumentMaterialShaderUniforms);
pipe->GetMetalDevice()->CurrentRenderCommandEncoder()->setFragmentBytes(ptr, fOwner->GetNumConsts() * sizeof(simd_float4), VertexShaderArgumentMaterialShaderUniforms);
}

return true;
Expand Down
23 changes: 13 additions & 10 deletions Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalPipeline.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,17 +864,19 @@ bool plMetalPipeline::SetGamma(const uint16_t* const tabR, const uint16_t* const
per channel data. The Metal renderer supports up to 10 bit colors - but it can subsample
the texture to interpolate the colors in between what the LUT defines.
*/
constexpr size_t numLuts = 256;

MTL::TextureDescriptor* texDescriptor = MTL::TextureDescriptor::alloc()->init()->autorelease();
texDescriptor->setTextureType(MTL::TextureType1DArray);
texDescriptor->setWidth(256);
texDescriptor->setWidth(numLuts);
texDescriptor->setPixelFormat(MTL::PixelFormatR16Uint);
texDescriptor->setArrayLength(3);

fDevice.fGammaLUTTexture = fDevice.fMetalDevice->newTexture(texDescriptor);

fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, 256), 0, 0, tabR, 256 * sizeof(uint16_t), 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, 256), 0, 1, tabG, 256 * sizeof(uint16_t), 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, 256), 0, 2, tabB, 256 * sizeof(uint16_t), 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, numLuts), 0, 0, tabR, 0, 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, numLuts), 0, 1, tabG, 0, 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, numLuts), 0, 2, tabB, 0, 0);

return true;
}
Expand All @@ -893,18 +895,19 @@ bool plMetalPipeline::SetGamma10(const uint16_t* const tabR, const uint16_t* con
by normalized co-ordinate - not value. So the width of the texture can
vary.
*/
constexpr size_t numLuts = 1024;

MTL::TextureDescriptor* texDescriptor = MTL::TextureDescriptor::alloc()->init()->autorelease();
texDescriptor->setTextureType(MTL::TextureType1DArray);
texDescriptor->setWidth(1024);
texDescriptor->setWidth(numLuts);
texDescriptor->setPixelFormat(MTL::PixelFormatR16Uint);
texDescriptor->setArrayLength(3);

fDevice.fGammaLUTTexture = fDevice.fMetalDevice->newTexture(texDescriptor);

fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, 1024), 0, 0, tabR, 1024 * sizeof(uint16_t), 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, 1024), 0, 1, tabG, 1024 * sizeof(uint16_t), 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, 1024), 0, 2, tabB, 1024 * sizeof(uint16_t), 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, numLuts), 0, 0, tabR, 0, 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, numLuts), 0, 1, tabG, 0, 0);
fDevice.fGammaLUTTexture->replaceRegion(MTL::Region(0, numLuts), 0, 2, tabB, 0, 0);

return true;
}
Expand Down Expand Up @@ -2751,9 +2754,9 @@ void plMetalPipeline::IPreprocessAvatarTextures()
vertexDescriptor->attributes()->object(0)->setOffset(0);
vertexDescriptor->attributes()->object(1)->setFormat(MTL::VertexFormatFloat2);
vertexDescriptor->attributes()->object(1)->setBufferIndex(0);
vertexDescriptor->attributes()->object(1)->setOffset(sizeof(float) * 2);
vertexDescriptor->attributes()->object(1)->setOffset(sizeof(simd_float2));

vertexDescriptor->layouts()->object(0)->setStride(sizeof(float) * 4);
vertexDescriptor->layouts()->object(0)->setStride(sizeof(simd_float2) * 2);

descriptor->setVertexDescriptor(vertexDescriptor);

Expand Down
8 changes: 4 additions & 4 deletions Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalPipelineState.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,14 @@ size_t plMetalMaterialPassPipelineState::GetHash() const
void plMetalRenderSpanPipelineState::ConfigureVertexDescriptor(MTL::VertexDescriptor* vertexDescriptor)
{
int vertOffset = 0;
int skinWeightOffset = vertOffset + (sizeof(float) * 3);
int skinWeightOffset = vertOffset + sizeof(hsPoint3);
if (fHasSkinIndices) {
skinWeightOffset += sizeof(uint32_t);
}
int normOffset = skinWeightOffset + (sizeof(float) * this->fNumWeights);
int colorOffset = normOffset + (sizeof(float) * 3);
int colorOffset = normOffset + sizeof(hsPoint3);
int baseUvOffset = colorOffset + (sizeof(uint32_t) * 2);
int stride = baseUvOffset + (sizeof(float) * 3 * this->fNumUVs);
int stride = baseUvOffset + sizeof(hsPoint3) * this->fNumUVs;

vertexDescriptor->attributes()->object(VertexAttributePosition)->setFormat(MTL::VertexFormatFloat3);
vertexDescriptor->attributes()->object(VertexAttributePosition)->setBufferIndex(0);
Expand All @@ -151,7 +151,7 @@ void plMetalRenderSpanPipelineState::ConfigureVertexDescriptor(MTL::VertexDescri
for (int i = 0; i < this->fNumUVs; i++) {
vertexDescriptor->attributes()->object(VertexAttributeTexcoord + i)->setFormat(MTL::VertexFormatFloat3);
vertexDescriptor->attributes()->object(VertexAttributeTexcoord + i)->setBufferIndex(0);
vertexDescriptor->attributes()->object(VertexAttributeTexcoord + i)->setOffset(baseUvOffset + (i * sizeof(float) * 3));
vertexDescriptor->attributes()->object(VertexAttributeTexcoord + i)->setOffset(baseUvOffset + (i * sizeof(hsPoint3)));
}

vertexDescriptor->attributes()->object(VertexAttributeColor)->setFormat(MTL::VertexFormatUChar4);
Expand Down
6 changes: 3 additions & 3 deletions Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalPlateManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ void plMetalPlateManager::ICreateGeometry()

fVtxBuffer = pipeline->fDevice.fMetalDevice->newBuffer(&vertexBuffer, sizeof(plateVertexBuffer), MTL::StorageModeManaged);
fVtxBuffer->retain();
idxBuffer = pipeline->fDevice.fMetalDevice->newBuffer(&indices, sizeof(uint16_t) * 6, MTL::StorageModeManaged);
idxBuffer = pipeline->fDevice.fMetalDevice->newBuffer(&indices, sizeof(indices), MTL::StorageModeManaged);
}
}

Expand Down Expand Up @@ -157,8 +157,8 @@ void plMetalPlatePipelineState::ConfigureVertexDescriptor(MTL::VertexDescriptor*
vertexDescriptor->attributes()->object(1)->setBufferIndex(VertexAttributeTexcoord);
vertexDescriptor->attributes()->object(1)->setOffset(0);

vertexDescriptor->layouts()->object(0)->setStride(sizeof(float) * 2);
vertexDescriptor->layouts()->object(1)->setStride(sizeof(float) * 2);
vertexDescriptor->layouts()->object(0)->setStride(sizeof(simd_float2));
vertexDescriptor->layouts()->object(1)->setStride(sizeof(simd_float2));
}

void plMetalPlatePipelineState::GetFunctionConstants(MTL::FunctionConstantValues*) const
Expand Down
2 changes: 1 addition & 1 deletion Sources/Plasma/FeatureLib/pfMetalPipeline/plMetalVertexShader.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ bool plMetalVertexShader::ISetConstants(plMetalPipeline* pipe)
{
if (fOwner->GetNumConsts()) {
float* ptr = (float*)fOwner->GetConstBasePtr();
pipe->GetMetalDevice()->CurrentRenderCommandEncoder()->setVertexBytes(ptr, fOwner->GetNumConsts() * sizeof(float) * 4, VertexShaderArgumentMaterialShaderUniforms);
pipe->GetMetalDevice()->CurrentRenderCommandEncoder()->setVertexBytes(ptr, fOwner->GetNumConsts() * sizeof(simd_float4), VertexShaderArgumentMaterialShaderUniforms);
}

return true;
Expand Down

0 comments on commit 10e2068

Please sign in to comment.