Skip to content

Commit

Permalink
Merge pull request #18695 from hrydgard/expand-prims-fix-2
Browse files Browse the repository at this point in the history 
Bounds-checks for expanding primitives, take 2
  • Loading branch information
@hrydgard
hrydgard committed Jan 15, 2024
2 parents 80547c5 + dac4c7a commit ddeb211
Show file tree
Hide file tree
Showing 8 changed files with 70 additions and 32 deletions.
4 changes: 3 additions & 1 deletion GPU/Common/DrawEngineCommon.h
View file
Expand Up @@ -241,7 +241,9 @@ class DrawEngineCommon {
}
}

uint32_t ComputeDrawcallsHash() const;
inline int RemainingIndices(const uint16_t *inds) const {
return DECODED_INDEX_BUFFER_SIZE / sizeof(uint16_t) - (inds - decIndex_);
}

bool useHWTransform_ = false;
bool useHWTessellation_ = false;
Expand Down
74 changes: 53 additions & 21 deletions GPU/Common/SoftwareTransformCommon.cpp
View file
Expand Up @@ -17,10 +17,10 @@

#include <algorithm>
#include <cmath>

#include "Common/CPUDetect.h"
#include "Common/Math/math_util.h"
#include "Common/GPU/OpenGL/GLFeatures.h"

#include "Core/Config.h"
#include "GPU/GPUState.h"
#include "GPU/Math3D.h"
Expand Down Expand Up @@ -171,7 +171,7 @@ void SoftwareTransform::SetProjMatrix(const float mtx[14], bool invertedX, bool
projMatrix_.translateAndScale(trans, scale);
}

void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &decVtxFormat, int maxIndex, SoftwareTransformResult *result) {
void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &decVtxFormat, int numDecodedVerts, SoftwareTransformResult *result) {
u8 *decoded = params_.decoded;
TransformedVertex *transformed = params_.transformed;
bool throughmode = (vertType & GE_VTYPE_THROUGH_MASK) != 0;
Expand Down Expand Up @@ -212,7 +212,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de
const u32 materialAmbientRGBA = gstate.getMaterialAmbientRGBA();
const bool hasColor = reader.hasColor0();
const bool hasUV = reader.hasUV();
for (int index = 0; index < maxIndex; index++) {
for (int index = 0; index < numDecodedVerts; index++) {
// Do not touch the coordinates or the colors. No lighting.
reader.Goto(index);
// TODO: Write to a flexible buffer, we don't always need all four components.
Expand All @@ -221,7 +221,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de
vert.pos_w = 1.0f;

if (hasColor) {
if (provokeIndOffset != 0 && index + provokeIndOffset < maxIndex) {
if (provokeIndOffset != 0 && index + provokeIndOffset < numDecodedVerts) {
reader.Goto(index + provokeIndOffset);
vert.color0_32 = reader.ReadColor0_8888();
reader.Goto(index);
Expand Down Expand Up @@ -249,7 +249,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de
} else {
const Vec4f materialAmbientRGBA = Vec4f::FromRGBA(gstate.getMaterialAmbientRGBA());
// Okay, need to actually perform the full transform.
for (int index = 0; index < maxIndex; index++) {
for (int index = 0; index < numDecodedVerts; index++) {
reader.Goto(index);

float v[3] = {0, 0, 0};
Expand All @@ -270,7 +270,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de

// Read all the provoking vertex values here.
Vec4f unlitColor;
if (provokeIndOffset != 0 && index + provokeIndOffset < maxIndex)
if (provokeIndOffset != 0 && index + provokeIndOffset < numDecodedVerts)
reader.Goto(index + provokeIndOffset);
if (reader.hasColor0())
reader.ReadColor0(unlitColor.AsArray());
Expand Down Expand Up @@ -439,10 +439,10 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de
// TODO: This bleeds outside the play area in non-buffered mode. Big deal? Probably not.
// TODO: Allow creating a depth clear and a color draw.
bool reallyAClear = false;
if (maxIndex > 1 && prim == GE_PRIM_RECTANGLES && gstate.isModeClear() && throughmode) {
if (numDecodedVerts > 1 && prim == GE_PRIM_RECTANGLES && gstate.isModeClear() && throughmode) {
int scissorX2 = gstate.getScissorX2() + 1;
int scissorY2 = gstate.getScissorY2() + 1;
reallyAClear = IsReallyAClear(transformed, maxIndex, scissorX2, scissorY2);
reallyAClear = IsReallyAClear(transformed, numDecodedVerts, scissorX2, scissorY2);
if (reallyAClear && gstate.getColorMask() != 0xFFFFFFFF && (gstate.isClearModeColorMask() || gstate.isClearModeAlphaMask())) {
result->setSafeSize = true;
result->safeWidth = scissorX2;
Expand All @@ -467,7 +467,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de
}

// Detect full screen "clears" that might not be so obvious, to set the safe size if possible.
if (!result->setSafeSize && prim == GE_PRIM_RECTANGLES && maxIndex == 2 && throughmode) {
if (!result->setSafeSize && prim == GE_PRIM_RECTANGLES && numDecodedVerts == 2 && throughmode) {
bool clearingColor = gstate.isModeClear() && (gstate.isClearModeColorMask() || gstate.isClearModeAlphaMask());
bool writingColor = gstate.getColorMask() != 0xFFFFFFFF;
bool startsZeroX = transformed[0].x <= 0.0f && transformed[1].x > 0.0f && transformed[1].x > transformed[0].x;
Expand All @@ -483,8 +483,7 @@ void SoftwareTransform::Transform(int prim, u32 vertType, const DecVtxFormat &de
}
}

// NOTE: The viewport must be up to date!
void SoftwareTransform::BuildDrawingParams(int prim, int vertexCount, u32 vertType, u16 *&inds, int &maxIndex, SoftwareTransformResult *result) {
void SoftwareTransform::BuildDrawingParams(int prim, int vertexCount, u32 vertType, u16 *&inds, int indsSize, int &numDecodedVerts, SoftwareTransformResult *result) {
TransformedVertex *transformed = params_.transformed;
TransformedVertex *transformedExpanded = params_.transformedExpanded;
bool throughmode = (vertType & GE_VTYPE_THROUGH_MASK) != 0;
Expand All @@ -497,7 +496,12 @@ void SoftwareTransform::BuildDrawingParams(int prim, int vertexCount, u32 vertTy
bool useBufferedRendering = fbman->UseBufferedRendering();

if (prim == GE_PRIM_RECTANGLES) {
ExpandRectangles(vertexCount, maxIndex, inds, transformed, transformedExpanded, numTrans, throughmode, &result->pixelMapped);
if (!ExpandRectangles(vertexCount, numDecodedVerts, inds, indsSize, transformed, transformedExpanded, numTrans, throughmode, &result->pixelMapped)) {
result->drawIndexed = false;
result->drawNumTrans = 0;
result->pixelMapped = false;
return;
}
result->drawBuffer = transformedExpanded;
result->drawIndexed = true;

Expand All @@ -515,15 +519,23 @@ void SoftwareTransform::BuildDrawingParams(int prim, int vertexCount, u32 vertTy
}
}
} else if (prim == GE_PRIM_POINTS) {
ExpandPoints(vertexCount, maxIndex, inds, transformed, transformedExpanded, numTrans, throughmode);
result->pixelMapped = false;
if (!ExpandPoints(vertexCount, numDecodedVerts, inds, indsSize, transformed, transformedExpanded, numTrans, throughmode)) {
result->drawIndexed = false;
result->drawNumTrans = 0;
return;
}
result->drawBuffer = transformedExpanded;
result->drawIndexed = true;
result->pixelMapped = false;
} else if (prim == GE_PRIM_LINES) {
ExpandLines(vertexCount, maxIndex, inds, transformed, transformedExpanded, numTrans, throughmode);
result->pixelMapped = false;
if (!ExpandLines(vertexCount, numDecodedVerts, inds, indsSize, transformed, transformedExpanded, numTrans, throughmode)) {
result->drawIndexed = false;
result->drawNumTrans = 0;
return;
}
result->drawBuffer = transformedExpanded;
result->drawIndexed = true;
result->pixelMapped = false;
} else {
// We can simply draw the unexpanded buffer.
numTrans = vertexCount;
Expand Down Expand Up @@ -645,7 +657,13 @@ void SoftwareTransform::CalcCullParams(float &minZValue, float &maxZValue) {
std::swap(minZValue, maxZValue);
}

void SoftwareTransform::ExpandRectangles(int vertexCount, int &maxIndex, u16 *&inds, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode, bool *pixelMappedExactly) {
bool SoftwareTransform::ExpandRectangles(int vertexCount, int &numDecodedVerts, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode, bool *pixelMappedExactly) const {
// Before we start, do a sanity check - does the output fit?
if ((vertexCount / 2) * 6 > indsSize) {
// Won't fit, kill the draw.
return false;
}

// Rectangles always need 2 vertices, disregard the last one if there's an odd number.
vertexCount = vertexCount & ~1;
numTrans = 0;
Expand All @@ -655,7 +673,7 @@ void SoftwareTransform::ExpandRectangles(int vertexCount, int &maxIndex, u16 *&i
u16 *newInds = inds + vertexCount;
u16 *indsOut = newInds;

maxIndex = 4 * (vertexCount / 2);
numDecodedVerts = 4 * (vertexCount / 2);

float uscale = 1.0f;
float vscale = 1.0f;
Expand Down Expand Up @@ -733,9 +751,16 @@ void SoftwareTransform::ExpandRectangles(int vertexCount, int &maxIndex, u16 *&i
}
inds = newInds;
*pixelMappedExactly = pixelMapped;
return true;
}

void SoftwareTransform::ExpandLines(int vertexCount, int &maxIndex, u16 *&inds, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) {
bool SoftwareTransform::ExpandLines(int vertexCount, int &numDecodedVerts, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) const {
// Before we start, do a sanity check - does the output fit?
if ((vertexCount / 2) * 6 > indsSize) {
// Won't fit, kill the draw.
return false;
}

// Lines always need 2 vertices, disregard the last one if there's an odd number.
vertexCount = vertexCount & ~1;
numTrans = 0;
Expand All @@ -755,7 +780,7 @@ void SoftwareTransform::ExpandLines(int vertexCount, int &maxIndex, u16 *&inds,
dy = 1.0f;
}

maxIndex = 4 * (vertexCount / 2);
numDecodedVerts = 4 * (vertexCount / 2);

if (PSP_CoreParameter().compat.flags().CenteredLines) {
// Lines meant to be pretty in 3D like in Echochrome.
Expand Down Expand Up @@ -857,10 +882,16 @@ void SoftwareTransform::ExpandLines(int vertexCount, int &maxIndex, u16 *&inds,
}

inds = newInds;
return true;
}

bool SoftwareTransform::ExpandPoints(int vertexCount, int &maxIndex, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) const {
// Before we start, do a sanity check - does the output fit?
if (vertexCount * 6 > indsSize) {
// Won't fit, kill the draw.
return false;
}

void SoftwareTransform::ExpandPoints(int vertexCount, int &maxIndex, u16 *&inds, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) {
numTrans = 0;
TransformedVertex *trans = &transformedExpanded[0];

Expand Down Expand Up @@ -926,4 +957,5 @@ void SoftwareTransform::ExpandPoints(int vertexCount, int &maxIndex, u16 *&inds,
numTrans += 6;
}
inds = newInds;
return true;
}
13 changes: 8 additions & 5 deletions GPU/Common/SoftwareTransformCommon.h
View file
Expand Up @@ -67,14 +67,17 @@ class SoftwareTransform {
SoftwareTransform(SoftwareTransformParams &params) : params_(params) {}

void SetProjMatrix(const float mtx[14], bool invertedX, bool invertedY, const Lin::Vec3 &trans, const Lin::Vec3 &scale);
void Transform(int prim, u32 vertexType, const DecVtxFormat &decVtxFormat, int maxIndex, SoftwareTransformResult *result);
void BuildDrawingParams(int prim, int vertexCount, u32 vertType, u16 *&inds, int &maxIndex, SoftwareTransformResult *result);
void Transform(int prim, u32 vertexType, const DecVtxFormat &decVtxFormat, int numDecodedVerts, SoftwareTransformResult *result);

// NOTE: The viewport must be up to date!
// indsSize is in indices, not bytes.
void BuildDrawingParams(int prim, int vertexCount, u32 vertType, u16 *&inds, int indsSize, int &numDecodedVerts, SoftwareTransformResult *result);

protected:
void CalcCullParams(float &minZValue, float &maxZValue);
void ExpandRectangles(int vertexCount, int &maxIndex, u16 *&inds, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode, bool *pixelMappedExactly);
void ExpandLines(int vertexCount, int &maxIndex, u16 *&inds, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode);
void ExpandPoints(int vertexCount, int &maxIndex, u16 *&inds, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode);
bool ExpandRectangles(int vertexCount, int &numDecodedVerts, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode, bool *pixelMappedExactly) const;
bool ExpandLines(int vertexCount, int &numDecodedVerts, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) const;
bool ExpandPoints(int vertexCount, int &numDecodedVerts, u16 *&inds, int indsSize, const TransformedVertex *transformed, TransformedVertex *transformedExpanded, int &numTrans, bool throughmode) const;

const SoftwareTransformParams &params_;
Lin::Matrix4x4 projMatrix_;
Expand Down
2 changes: 1 addition & 1 deletion GPU/D3D11/DrawEngineD3D11.cpp
View file
Expand Up @@ -418,7 +418,7 @@ void DrawEngineD3D11::DoFlush() {
ApplyDrawState(prim);

if (result.action == SW_NOT_READY)
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, numDecodedVerts_, &result);
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, RemainingIndices(inds), numDecodedVerts_, &result);
if (result.setSafeSize)
framebufferManager_->SetSafeSize(result.safeWidth, result.safeHeight);

Expand Down
2 changes: 1 addition & 1 deletion GPU/Directx9/DrawEngineDX9.cpp
View file
Expand Up @@ -374,7 +374,7 @@ void DrawEngineDX9::DoFlush() {
ApplyDrawState(prim);

if (result.action == SW_NOT_READY)
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, numDecodedVerts_, &result);
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, RemainingIndices(inds), numDecodedVerts_, &result);
if (result.setSafeSize)
framebufferManager_->SetSafeSize(result.safeWidth, result.safeHeight);

Expand Down
2 changes: 1 addition & 1 deletion GPU/GLES/DrawEngineGLES.cpp
View file
Expand Up @@ -403,7 +403,7 @@ void DrawEngineGLES::DoFlush() {
ApplyDrawState(prim);

if (result.action == SW_NOT_READY)
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, numDecodedVerts_, &result);
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, RemainingIndices(inds), numDecodedVerts_, &result);
if (result.setSafeSize)
framebufferManager_->SetSafeSize(result.safeWidth, result.safeHeight);

Expand Down
2 changes: 1 addition & 1 deletion GPU/Software/TransformUnit.cpp
View file
Expand Up @@ -499,7 +499,7 @@ class SoftwareVertexReader {
// If we're only using a subset of verts, it's better to decode with random access (usually.)
// However, if we're reusing a lot of verts, we should read and cache them.
useCache_ = useIndices_ && vertex_count > (upperBound_ - lowerBound_ + 1);
if (useCache_ && cached_.size() < upperBound_ - lowerBound_ + 1)
if (useCache_ && (int)cached_.size() < upperBound_ - lowerBound_ + 1)
cached_.resize(std::max(128, upperBound_ - lowerBound_ + 1));
}

Expand Down
3 changes: 2 additions & 1 deletion GPU/Vulkan/DrawEngineVulkan.cpp
View file
Expand Up @@ -430,7 +430,8 @@ void DrawEngineVulkan::DoFlush() {
result.action = SW_NOT_READY;

if (result.action == SW_NOT_READY) {
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, numDecodedVerts_, &result);
// decIndex_ here is always equal to inds currently, but it may not be in the future.
swTransform.BuildDrawingParams(prim, vertexCount, dec_->VertexType(), inds, RemainingIndices(inds), numDecodedVerts_, &result);
}

if (result.setSafeSize)
Expand Down

0 comments on commit ddeb211

Please sign in to comment.