mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-01 14:58:14 +00:00
GPU: Fix draw engine buffer overruns and stale vertex data
- Flush before the queued draws would decode more than VERTEX_BUFFER_MAX vertices. The batch was limited by index count, which doesn't bound a sparse index range, and DecodeVerts silently stopped while DecodeInds still emitted indices for the undecoded draws. - Give TestBoundingBox its own scratch buffer. It used offsets in decoded_, which can hold decoded vertices that aren't flushed yet. - Read 32-bit indices the way the PSP does, ignoring the upper 16 bits. IndexConverter and the fast bounding box test used all 32, so a game setting them indexed far past the decoded vertices. - D3D11: Flush in FinishDeferred like the other backends, since indices are still read from PSP memory at flush time (#10095). - Don't JIT new vertex decoders once the code space is full. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
1 parent
c40cce5173
commit
16ea2cbf88
9 files changed
+73
-25
No files matched your search
@@ -51,11 +51,13 @@ DrawEngineCommon::DrawEngineCommon() : decoderMap_(32) {
|
||||
transformedExpanded_ = (TransformedVertex *)AllocateMemoryPages(3 * TRANSFORMED_VERTEX_BUFFER_SIZE, MEM_PROT_READ | MEM_PROT_WRITE);
|
||||
decoded_ = (u8 *)AllocateMemoryPages(DECODED_VERTEX_BUFFER_SIZE, MEM_PROT_READ | MEM_PROT_WRITE);
|
||||
decIndex_ = (u16 *)AllocateMemoryPages(DECODED_INDEX_BUFFER_SIZE, MEM_PROT_READ | MEM_PROT_WRITE);
|
||||
bboxScratch_ = (u8 *)AllocateMemoryPages(BBOX_SCRATCH_SIZE, MEM_PROT_READ | MEM_PROT_WRITE);
|
||||
|
||||
_dbg_assert_(transformed_);
|
||||
_dbg_assert_(transformedExpanded_);
|
||||
_dbg_assert_(decoded_);
|
||||
_dbg_assert_(decIndex_);
|
||||
_dbg_assert_(bboxScratch_);
|
||||
|
||||
indexGen.Setup(decIndex_);
|
||||
|
||||
@@ -65,6 +67,7 @@ DrawEngineCommon::DrawEngineCommon() : decoderMap_(32) {
|
||||
DrawEngineCommon::~DrawEngineCommon() {
|
||||
FreeMemoryPages(decoded_, DECODED_VERTEX_BUFFER_SIZE);
|
||||
FreeMemoryPages(decIndex_, DECODED_INDEX_BUFFER_SIZE);
|
||||
FreeMemoryPages(bboxScratch_, BBOX_SCRATCH_SIZE);
|
||||
FreeMemoryPages(transformed_, TRANSFORMED_VERTEX_BUFFER_SIZE);
|
||||
FreeMemoryPages(transformedExpanded_, 3 * TRANSFORMED_VERTEX_BUFFER_SIZE);
|
||||
ShutdownDepthRaster();
|
||||
@@ -183,15 +186,14 @@ void DrawEngineCommon::DispatchSubmitImm(GEPrimitiveType prim, TransformedVertex
|
||||
// - Less accurate, but..
|
||||
// - Only requires six plane evaluations then.
|
||||
bool DrawEngineCommon::TestBoundingBox(const void *vdata, const void *inds, int vertexCount, const VertexDecoder *dec, u32 vertType) {
|
||||
// Grab temp buffer space from large offsets in decoded_. Not exactly safe for large draws.
|
||||
// Although this may lead to drawing that shouldn't happen, the viewport is more complex on VR.
|
||||
// Let's always say objects are within bounds.
|
||||
// The scratch buffer is sized for 1024 vertices. Although this may lead to drawing that shouldn't happen,
|
||||
// the viewport is more complex on VR. Let's always say objects are within bounds.
|
||||
if (vertexCount > 1024 || gstate_c.Use(GPU_USE_VIRTUAL_REALITY)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
SimpleVertex *corners = (SimpleVertex *)(decoded_ + 65536 * 12);
|
||||
float *verts = (float *)(decoded_ + 65536 * 18);
|
||||
SimpleVertex *corners = (SimpleVertex *)(bboxScratch_ + BBOX_SCRATCH_CORNERS_OFFSET);
|
||||
float *verts = (float *)(bboxScratch_ + BBOX_SCRATCH_VERTS_OFFSET);
|
||||
|
||||
// Try to skip NormalizeVertices if it's pure positions. No need to bother with a vertex decoder
|
||||
// and a large vertex format.
|
||||
@@ -218,7 +220,7 @@ bool DrawEngineCommon::TestBoundingBox(const void *vdata, const void *inds, int
|
||||
}
|
||||
} else {
|
||||
// Simplify away indices, bones, and morph before proceeding.
|
||||
u8 *temp_buffer = decoded_ + 65536 * 24;
|
||||
u8 *temp_buffer = bboxScratch_ + BBOX_SCRATCH_TEMP_OFFSET;
|
||||
|
||||
if ((inds || (vertType & (GE_VTYPE_WEIGHT_MASK | GE_VTYPE_MORPHCOUNT_MASK)))) {
|
||||
// Need for Speed Carbon ends up on this path! With a single bone weight.
|
||||
@@ -392,7 +394,8 @@ static bool TestBoundingBoxFast(const float *cullMatrix, const void *vdata, cons
|
||||
}
|
||||
case GE_VTYPE_IDX_32BIT:
|
||||
{
|
||||
u32 idx = ((u32 *)idata)[i];
|
||||
// The PSP ignores the upper 16 bits.
|
||||
u16 idx = (u16)((u32 *)idata)[i];
|
||||
data = (const s8 *)srcdata + idx * stride;
|
||||
break;
|
||||
}
|
||||
@@ -759,7 +762,7 @@ int DrawEngineCommon::ExtendNonIndexedPrim(const uint32_t *cmd, const uint32_t *
|
||||
if (IsTrianglePrim(newPrim) != isTriangle)
|
||||
break;
|
||||
int vertexCount = data & 0xFFFF;
|
||||
if (numDrawInds >= MAX_DEFERRED_DRAW_INDS || vertexCountInDrawCalls_ + offset + vertexCount > VERTEX_BUFFER_MAX) {
|
||||
if (numDrawInds >= MAX_DEFERRED_DRAW_INDS || vertexCountInDrawCalls_ + offset + vertexCount > VERTEX_BUFFER_MAX || numVertsToDecode_ + (offset - dv.vertexCount) + vertexCount > VERTEX_BUFFER_MAX) {
|
||||
break;
|
||||
}
|
||||
DeferredInds &di = drawInds_[numDrawInds++];
|
||||
@@ -780,6 +783,7 @@ int DrawEngineCommon::ExtendNonIndexedPrim(const uint32_t *cmd, const uint32_t *
|
||||
dv.vertexCount = offset;
|
||||
dv.indexUpperBound = dv.vertexCount - 1;
|
||||
vertexCountInDrawCalls_ += totalCount;
|
||||
numVertsToDecode_ += totalCount;
|
||||
*bytesRead = totalCount * dec->VertexSize();
|
||||
return cmd - start;
|
||||
}
|
||||
@@ -805,7 +809,21 @@ void DrawEngineCommon::SkipPrim(GEPrimitiveType prim, int vertexCount, const Ver
|
||||
|
||||
// vertTypeID is the vertex type but with the UVGen mode smashed into the top bits.
|
||||
bool DrawEngineCommon::SubmitPrim(const void *verts, const void *inds, GEPrimitiveType prim, int vertexCount, const VertexDecoder *dec, u32 vertTypeID, bool clockwise, int *bytesRead, ClipInfoFlags clipInfoFlags) {
|
||||
if (!indexGen.PrimCompatible(prevPrim_, prim) || numDrawVerts_ >= MAX_DEFERRED_DRAW_VERTS || numDrawInds_ >= MAX_DEFERRED_DRAW_INDS || vertexCountInDrawCalls_ + vertexCount > VERTEX_BUFFER_MAX) {
|
||||
// The index count doesn't bound how many vertices DecodeVerts will produce (the index range can be
|
||||
// sparse), so track that separately, as the growth of the range to decode.
|
||||
u16 lowerBound = 0;
|
||||
u16 upperBound = 0;
|
||||
int decodeGrowth = 0;
|
||||
if (vertexCount > 0) {
|
||||
GetIndexBounds(inds, vertexCount, vertTypeID, &lowerBound, &upperBound);
|
||||
decodeGrowth = upperBound - lowerBound + 1;
|
||||
if (CanExtendDecode(verts, inds, dec)) {
|
||||
const DeferredVerts &last = drawVerts_[numDrawVerts_ - 1];
|
||||
decodeGrowth = std::max(upperBound, last.indexUpperBound) - std::min(lowerBound, last.indexLowerBound) - (last.indexUpperBound - last.indexLowerBound);
|
||||
}
|
||||
}
|
||||
|
||||
if (!indexGen.PrimCompatible(prevPrim_, prim) || numDrawVerts_ >= MAX_DEFERRED_DRAW_VERTS || numDrawInds_ >= MAX_DEFERRED_DRAW_INDS || vertexCountInDrawCalls_ + vertexCount > VERTEX_BUFFER_MAX || numVertsToDecode_ + decodeGrowth > VERTEX_BUFFER_MAX) {
|
||||
Flush();
|
||||
}
|
||||
|
||||
@@ -855,6 +873,7 @@ bool DrawEngineCommon::SubmitPrim(const void *verts, const void *inds, GEPrimiti
|
||||
const int rem = vertexCount % 3;
|
||||
if (rem != 0) {
|
||||
vertexCount -= rem;
|
||||
GetIndexBounds(inds, vertexCount, vertTypeID, &lowerBound, &upperBound);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -881,17 +900,16 @@ bool DrawEngineCommon::SubmitPrim(const void *verts, const void *inds, GEPrimiti
|
||||
|
||||
_dbg_assert_(numDrawVerts <= MAX_DEFERRED_DRAW_VERTS);
|
||||
|
||||
if (inds && numDrawVerts > decodeVertsCounter_ && drawVerts_[numDrawVerts - 1].verts == verts && !applySkin) {
|
||||
if (CanExtendDecode(verts, inds, dec_)) {
|
||||
// Same vertex pointer as a previous un-decoded draw call - let's just extend the decode!
|
||||
di.vertDecodeIndex = numDrawVerts - 1;
|
||||
u16 lb;
|
||||
u16 ub;
|
||||
GetIndexBounds(inds, vertexCount, vertTypeID, &lb, &ub);
|
||||
DeferredVerts &dv = drawVerts_[numDrawVerts - 1];
|
||||
if (lb < dv.indexLowerBound)
|
||||
dv.indexLowerBound = lb;
|
||||
if (ub > dv.indexUpperBound)
|
||||
dv.indexUpperBound = ub;
|
||||
const int oldCount = dv.indexUpperBound - dv.indexLowerBound + 1;
|
||||
if (lowerBound < dv.indexLowerBound)
|
||||
dv.indexLowerBound = lowerBound;
|
||||
if (upperBound > dv.indexUpperBound)
|
||||
dv.indexUpperBound = upperBound;
|
||||
numVertsToDecode_ += dv.indexUpperBound - dv.indexLowerBound + 1 - oldCount;
|
||||
} else {
|
||||
// Record a new draw, and a new index gen.
|
||||
DeferredVerts &dv = drawVerts_[numDrawVerts];
|
||||
@@ -899,8 +917,9 @@ bool DrawEngineCommon::SubmitPrim(const void *verts, const void *inds, GEPrimiti
|
||||
dv.verts = verts;
|
||||
dv.vertexCount = vertexCount;
|
||||
dv.uvScale = LoadUVScaleOffset(gstate);
|
||||
// Does handle the unindexed case.
|
||||
GetIndexBounds(inds, vertexCount, vertTypeID, &dv.indexLowerBound, &dv.indexUpperBound);
|
||||
dv.indexLowerBound = lowerBound;
|
||||
dv.indexUpperBound = upperBound;
|
||||
numVertsToDecode_ += upperBound - lowerBound + 1;
|
||||
}
|
||||
|
||||
vertexCountInDrawCalls_ += vertexCount;
|
||||
@@ -930,8 +949,9 @@ void DrawEngineCommon::DecodeVerts(const VertexDecoder *dec, u8 *dest) {
|
||||
drawVertexOffsets_[i] = numDecodedVerts - indexLowerBound;
|
||||
const int indexUpperBound = dv.indexUpperBound;
|
||||
const int count = indexUpperBound - indexLowerBound + 1;
|
||||
if (count + numDecodedVerts >= VERTEX_BUFFER_MAX) {
|
||||
// Hit our limit! Stop decoding in this draw.
|
||||
if (count + numDecodedVerts > VERTEX_BUFFER_MAX) {
|
||||
// SubmitPrim flushes before this can happen.
|
||||
_dbg_assert_(false);
|
||||
break;
|
||||
}
|
||||
|
||||
|
||||
@@ -38,6 +38,11 @@ enum {
|
||||
VERTEX_BUFFER_MAX = 65536,
|
||||
DECODED_VERTEX_BUFFER_SIZE = VERTEX_BUFFER_MAX * 2 * 36, // 36 == sizeof(SimpleVertex)
|
||||
DECODED_INDEX_BUFFER_SIZE = VERTEX_BUFFER_MAX * 6 * 6 * 2, // * 6 for spline tessellation, then * 6 again for converting into points/lines, and * 2 for 2 bytes per index
|
||||
// TestBoundingBox handles up to 1025 vertices: corners (SimpleVertex), then positions, then decoded vertices.
|
||||
BBOX_SCRATCH_CORNERS_OFFSET = 0,
|
||||
BBOX_SCRATCH_VERTS_OFFSET = 64 * 1024,
|
||||
BBOX_SCRATCH_TEMP_OFFSET = 128 * 1024,
|
||||
BBOX_SCRATCH_SIZE = 256 * 1024,
|
||||
};
|
||||
|
||||
enum {
|
||||
@@ -161,6 +166,11 @@ protected:
|
||||
void DecodeVerts(const VertexDecoder *dec, u8 *dest);
|
||||
int DecodeInds();
|
||||
|
||||
// Whether an indexed draw can share the previous draw's vertex decode, by widening its index range.
|
||||
bool CanExtendDecode(const void *verts, const void *inds, const VertexDecoder *dec) const {
|
||||
return inds && numDrawVerts_ > decodeVertsCounter_ && drawVerts_[numDrawVerts_ - 1].verts == verts && !dec->skinInDecode;
|
||||
}
|
||||
|
||||
int ComputeNumVertsToDecode() const;
|
||||
|
||||
void ApplyFramebufferRead(FBOTexState *fboTexState);
|
||||
@@ -211,6 +221,7 @@ protected:
|
||||
numDrawVerts_ = 0;
|
||||
numDrawInds_ = 0;
|
||||
vertexCountInDrawCalls_ = 0;
|
||||
numVertsToDecode_ = 0;
|
||||
decodeIndsCounter_ = 0;
|
||||
decodeVertsCounter_ = 0;
|
||||
seenPrims_ = 0;
|
||||
@@ -273,6 +284,8 @@ protected:
|
||||
// Vertex collector buffers
|
||||
u8 *decoded_ = nullptr;
|
||||
u16 *decIndex_ = nullptr;
|
||||
// Separate from decoded_, which can hold decoded vertices that haven't been flushed yet.
|
||||
u8 *bboxScratch_ = nullptr;
|
||||
|
||||
// Cached vertex decoders
|
||||
DenseHashMap<u32, VertexDecoder *> decoderMap_;
|
||||
@@ -312,6 +325,8 @@ protected:
|
||||
int numDrawVerts_ = 0;
|
||||
int numDrawInds_ = 0;
|
||||
int vertexCountInDrawCalls_ = 0;
|
||||
// How many vertices DecodeVerts will produce for the queued draws. Must stay <= VERTEX_BUFFER_MAX.
|
||||
int numVertsToDecode_ = 0;
|
||||
|
||||
int decodeVertsCounter_ = 0;
|
||||
int decodeIndsCounter_ = 0;
|
||||
|
||||
@@ -342,6 +342,7 @@ void IndexGenerator::TranslatePrim(int prim, int numInds, const u16_le *inds, in
|
||||
}
|
||||
}
|
||||
|
||||
// The PSP ignores the upper 16 bits of 32-bit indices. The u16 output drops them the same way.
|
||||
void IndexGenerator::TranslatePrim(int prim, int numInds, const u32_le *inds, int indexOffset, bool clockwise) {
|
||||
switch (prim) {
|
||||
case GE_PRIM_POINTS: TranslatePoints<u32_le>(numInds, inds, indexOffset); break;
|
||||
|
||||
@@ -170,8 +170,9 @@ void GetIndexBounds(const void *inds, int count, u32 vertType, u16 *indexLowerBo
|
||||
bool oob = false;
|
||||
const u32_le *ind32 = (const u32_le *)inds;
|
||||
for (int i = 0; i < count; i++) {
|
||||
// The PSP ignores the upper 16 bits, so only the low ones count.
|
||||
const u16 value = (u16)ind32[i];
|
||||
// These aren't documented and should be rare. Let's bounds check each one.
|
||||
// Games setting the upper bits should be rare, so report them.
|
||||
if (ind32[i] != value) {
|
||||
oob = true;
|
||||
}
|
||||
@@ -1373,6 +1374,12 @@ void VertexDecoder::SetVertexType(u32 fmt, const VertexDecoderOptions &options,
|
||||
// Attempt to JIT as well. But only do that if the main CPU JIT is enabled, in order to aid
|
||||
// debugging attempts - if the main JIT doesn't work, this one won't do any better, probably.
|
||||
if (jitCache) {
|
||||
// Compile doesn't check for space. We can't clear the cache here since other decoders point into it,
|
||||
// so when it's full (only seen with garbage display lists), new decoders use the interpreter.
|
||||
if (jitCache->GetSpaceLeft() < 4096) {
|
||||
WARN_LOG(Log::G3D, "Vertex decoder JIT cache full, using the interpreter for %08x", fmt_);
|
||||
return;
|
||||
}
|
||||
jitted_ = jitCache->Compile(*this, &jittedSize_);
|
||||
if (!jitted_) {
|
||||
WARN_LOG(Log::G3D, "Vertex decoder JIT failed! fmt = %08x (%s)", fmt_, GetString(SHADER_STRING_SHORT_DESC).c_str());
|
||||
|
||||
@@ -115,7 +115,8 @@ public:
|
||||
case GE_VTYPE_IDX_16BIT:
|
||||
return indices16[index];
|
||||
case GE_VTYPE_IDX_32BIT:
|
||||
return indices32[index];
|
||||
// The PSP supports 32-bit indices in name only: it ignores the upper 16 bits.
|
||||
return indices32[index] & 0xFFFF;
|
||||
default:
|
||||
return index;
|
||||
}
|
||||
|
||||
@@ -63,7 +63,9 @@ public:
|
||||
void Flush() override;
|
||||
|
||||
void FinishDeferred() {
|
||||
DecodeVerts(dec_, decoded_);
|
||||
// Decoding only the vertices isn't enough: the indices are still read from PSP memory at flush
|
||||
// time, and the game may change them once it regains control (#10095).
|
||||
Flush();
|
||||
}
|
||||
|
||||
void NotifyConfigChanged() override;
|
||||
|
||||
@@ -229,6 +229,7 @@ void DrawEngineGLES::Flush() {
|
||||
numDrawVerts_ = 0;
|
||||
numDrawInds_ = 0;
|
||||
vertexCountInDrawCalls_ = 0;
|
||||
numVertsToDecode_ = 0;
|
||||
decodeVertsCounter_ = 0;
|
||||
decodeIndsCounter_ = 0;
|
||||
return;
|
||||
|
||||
@@ -550,6 +550,7 @@ void DrawEngineVulkan::ResetAfterSkippedDraw() {
|
||||
numDrawVerts_ = 0;
|
||||
numDrawInds_ = 0;
|
||||
vertexCountInDrawCalls_ = 0;
|
||||
numVertsToDecode_ = 0;
|
||||
decodeIndsCounter_ = 0;
|
||||
decodeVertsCounter_ = 0;
|
||||
gstate_c.vertexFullAlpha = true;
|
||||
|
||||
+1
-1
@@ -330,7 +330,7 @@ enum GEVertexType : uint32_t {
|
||||
GE_VTYPE_IDX_NONE = (0<<11),
|
||||
GE_VTYPE_IDX_8BIT = (1<<11),
|
||||
GE_VTYPE_IDX_16BIT = (2<<11),
|
||||
GE_VTYPE_IDX_32BIT = (3<<11),
|
||||
GE_VTYPE_IDX_32BIT = (3<<11), // In name only: the hardware ignores the upper 16 bits of each index.
|
||||
GE_VTYPE_IDX_MASK = (3<<11),
|
||||
#define GE_VTYPE_IDX_SHIFT 11
|
||||
};
|
||||
|
||||
Reference in new issue
Block a user