Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use sequence numbers instead of a tracking array for depth buffers #15854

Merged
merged 1 commit into from
Aug 17, 2022
+22 −81
Merged

Use sequence numbers instead of a tracking array for depth buffers #15854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 21 additions & 66 deletions GPU/Common/FramebufferManagerCommon.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,6 @@ FramebufferManagerCommon::~FramebufferManagerCommon() {
}
bvfbs_.clear();

// Shouldn't be anything left here in theory, but just in case...
for (auto trackedDepth : trackedDepthBuffers_) {
delete trackedDepth;
}
trackedDepthBuffers_.clear();

delete presentation_;
}

Expand Down Expand Up @@ -375,17 +369,11 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame
// Lookup in the depth tracking to find which VFB has the latest version of this Z buffer.
// Then bind it in color-to-depth mode.
//
// We are going to do this by having a special render mode where we take color and move to
// We do this by having a special render mode where we take color and move to
// depth in the fragment shader, and set color writes to off.
//
// We'll need a special fragment shader flag to convert color to depth.

for (auto &depth : this->trackedDepthBuffers_) {
if (depth->z_address == params.fb_address && depth->z_stride == params.fb_stride) {
// Found the matching depth buffer. Use this vfb.
vfb = depth->vfb;
}
}
// We use a special fragment shader flag to convert color to depth.
vfb = GetLatestDepthBufferAt(params.fb_address /* !!! */, params.fb_stride);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This looks as much like "important, fb was intentional here" as "TODO, gotta change something here."

-[Unknown]

}

gstate_c.SetFramebufferRenderMode(mode);
Expand Down Expand Up @@ -453,14 +441,13 @@ VirtualFramebuffer *FramebufferManagerCommon::DoSetRenderFrameBuffer(const Frame

// Looks up by z_address, so if one is found here and not have last pointers equal to this one,
// there is another one.
TrackedDepthBuffer *prevDepth = GetOrCreateTrackedDepthBuffer(vfb);
VirtualFramebuffer *prevDepth = GetLatestDepthBufferAt(vfb->z_address, vfb->z_stride);

// We might already want to copy depth, in case this is a temp buffer. See #7810.
if (prevDepth->vfb != vfb) {
if (!params.isClearingDepth && prevDepth->vfb) {
BlitFramebufferDepth(prevDepth->vfb, vfb);
if (prevDepth != vfb) {
if (!params.isClearingDepth && prevDepth) {
BlitFramebufferDepth(prevDepth, vfb);
}
prevDepth->vfb = vfb;
}

SetColorUpdated(vfb, skipDrawReason);
Expand Down Expand Up @@ -558,13 +545,6 @@ void FramebufferManagerCommon::DestroyFramebuf(VirtualFramebuffer *v) {
if (prevPrevDisplayFramebuf_ == v)
prevPrevDisplayFramebuf_ = nullptr;

// Remove any depth buffer tracking related to this vfb.
for (auto it = trackedDepthBuffers_.begin(); it != trackedDepthBuffers_.end(); it++) {
if ((*it)->vfb == v) {
(*it)->vfb = nullptr; // Mark for deletion in the next Decimate
}
}

delete v;
}

Expand Down Expand Up @@ -626,32 +606,16 @@ void FramebufferManagerCommon::BlitFramebufferDepth(VirtualFramebuffer *src, Vir
dst->last_frame_depth_updated = gpuStats.numFlips;
}

TrackedDepthBuffer *FramebufferManagerCommon::GetOrCreateTrackedDepthBuffer(VirtualFramebuffer *vfb) {
for (auto tracked : trackedDepthBuffers_) {
// Disable tracking if color of the new vfb is clashing with tracked depth.
if (vfb->fb_address == tracked->z_address) {
tracked->vfb = nullptr; // this is checked for. Cheaper than deleting.
continue;
}

if (vfb->z_address == tracked->z_address) {
if (vfb->z_stride == tracked->z_stride) {
return tracked;
} else {
// Stride has changed, mark as bad.
tracked->vfb = nullptr;
}
VirtualFramebuffer *FramebufferManagerCommon::GetLatestDepthBufferAt(u32 z_address, u16 z_stride) {
int maxSeq = -1;
VirtualFramebuffer *latestDepth = nullptr;
for (auto vfb : vfbs_) {
if (vfb->z_address == z_address && vfb->z_stride == z_stride && vfb->depthBindSeq > maxSeq) {
maxSeq = vfb->depthBindSeq;
latestDepth = vfb;
}
}

TrackedDepthBuffer *tracked = new TrackedDepthBuffer();
tracked->vfb = vfb;
tracked->z_address = vfb->z_address;
tracked->z_stride = vfb->z_stride;

trackedDepthBuffers_.push_back(tracked);

return tracked;
return latestDepth;
}

void FramebufferManagerCommon::NotifyRenderFramebufferCreated(VirtualFramebuffer *vfb) {
Expand Down Expand Up @@ -703,14 +667,14 @@ void FramebufferManagerCommon::NotifyRenderFramebufferSwitched(VirtualFramebuffe
shaderManager_->DirtyLastShader();

// Copy depth between the framebuffers, if the z_address is the same (checked inside.)
TrackedDepthBuffer *prevDepth = GetOrCreateTrackedDepthBuffer(vfb);
VirtualFramebuffer * prevDepth = GetLatestDepthBufferAt(vfb->z_address, vfb->z_stride);

// We might already want to copy depth, in case this is a temp buffer. See #7810.
if (prevDepth->vfb != vfb) {
if (!isClearingDepth && prevDepth->vfb) {
BlitFramebufferDepth(prevDepth->vfb, vfb);
if (prevDepth != vfb) {
if (!isClearingDepth && prevDepth) {
BlitFramebufferDepth(prevDepth, vfb);
}
prevDepth->vfb = vfb;
prevDepth = vfb;
}

if (vfb->drawnFormat != vfb->format) {
Expand Down Expand Up @@ -1234,16 +1198,6 @@ void FramebufferManagerCommon::DecimateFBOs() {
bvfbs_.erase(bvfbs_.begin() + i--);
}
}

// Also clean up the TrackedDepthBuffer array...
for (auto it = trackedDepthBuffers_.begin(); it != trackedDepthBuffers_.end();) {
if ((*it)->vfb == nullptr) {
delete *it;
it = trackedDepthBuffers_.erase(it);
} else {
it++;
}
}
}

// Requires width/height to be set already.
Expand Down Expand Up @@ -2220,6 +2174,7 @@ void FramebufferManagerCommon::ReadFramebufferToMemory(VirtualFramebuffer *vfb,
// We'll pseudo-blit framebuffers here to get a resized version of vfb.
if (gameUsesSequentialCopies_) {
// Ignore the x/y/etc., read the entire thing.
// TODO: What game did we need this for?
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, Grand Knights Historia was one of them but I think there were a few. It was because some games would memcpy regions in strips, or many smaller regions from a larger framebuffer.

-[Unknown]

x = 0;
y = 0;
w = vfb->width;
Expand Down
16 changes: 1 addition & 15 deletions GPU/Common/FramebufferManagerCommon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,18 +147,6 @@ struct VirtualFramebuffer {
int last_frame_depth_render;
};

struct TrackedDepthBuffer {
u32 z_address;
int z_stride;

// Really need to make sure we're killing these TrackedDepthBuffer's off when the VirtualFrameBuffers die.
VirtualFramebuffer *vfb;

// Could do full tracking of which framebuffers are used with this depth buffer,
// but probably not necessary.
// std::set<std::pair<u32, u32>> seen_fbs;
};

struct FramebufferHeuristicParams {
u32 fb_address;
u32 z_address;
Expand Down Expand Up @@ -300,7 +288,7 @@ class FramebufferManagerCommon {
void DownloadFramebufferForClut(u32 fb_address, u32 loadBytes);
void DrawFramebufferToOutput(const u8 *srcPixels, GEBufferFormat srcPixelFormat, int srcStride);

TrackedDepthBuffer *GetOrCreateTrackedDepthBuffer(VirtualFramebuffer *vfb);
VirtualFramebuffer *GetLatestDepthBufferAt(u32 z_address, u16 z_stride);

void DrawPixels(VirtualFramebuffer *vfb, int dstX, int dstY, const u8 *srcPixels, GEBufferFormat srcPixelFormat, int srcStride, int width, int height);

Expand Down Expand Up @@ -481,8 +469,6 @@ class FramebufferManagerCommon {
std::vector<VirtualFramebuffer *> vfbs_;
std::vector<VirtualFramebuffer *> bvfbs_; // blitting framebuffers (for download)

std::vector<TrackedDepthBuffer *> trackedDepthBuffers_;

bool gameUsesSequentialCopies_ = false;

// Sampled in BeginFrame/UpdateSize for safety.
Expand Down