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.

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

WIP: Virtual readbacks #11531

Merged
merged 3 commits into from Nov 12, 2018
Merged

WIP: Virtual readbacks #11531

merged 3 commits into from Nov 12, 2018

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Nov 4, 2018

As outlined in #11528

Fixes performance in Digimon Adventures and lets us remove the old MotoGP hack.

In addition there are multiple other games this could help but we'll add them later.

vfb->fb_address = dstBasePtr; // NOTE - not necessarily in VRAM!
vfb->fb_stride = dstStride;
vfb->z_address = 0;
vfb->z_stride =0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Spacing. Maybe we should verify that other code would fill these values in if the framebuf was drawn to later...

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yeah, that's a good point. This can be quite dangerous otherwise...

vfbs_.push_back(vfb);
dstBuffer = vfb;
}
dstBuffer->last_frame_used = gpuStats.numFlips;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be inside the if, or else be inside a nullcheck.

-[Unknown]

Copy link
Owner Author

@hrydgard hrydgard Nov 5, 2018

Choose a reason for hiding this comment

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

At that point, dstBuffer is guaranteed not to be null. Plus, these targets that we don't otherwise render to but texture from do need their last_frame_used updated, otherwise they'll quickly be decimated. It's kind of a bug that we didn't update this before on copy targets.

Copy link
Collaborator

Choose a reason for hiding this comment

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

When BlockTransferAllowCreateFB is false, ifdstBuffer cannot be null - why do we even need the !dstBuffer && check above, then?

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

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

Um.. right. Oops, sorry was too hasty :)

@hrydgard
Copy link
Owner Author

hrydgard commented Nov 8, 2018

Reportedly the speedup is massive in Digimon Adventures and the toon borders are sharp, with this.

So seems promising.

I really want to rethink and unify textures, framebuffers and CLUTs long term, but some ideas I have would require at least ES 3.0 level features and preferably higher. I really wish we could just forget about ES 2.0 and DX9 but we can't. So we're probably stuck with our current pile of hacks for some time longer, and this is just an addition to it. I will improve this further before merging of course, at least to fix the MotoGP case too so we can delete one hack while adding this one.

@hrydgard hrydgard added this to the v1.8.0 milestone Nov 8, 2018
@hrydgard hrydgard added Vulkan D3D9 Direct3D 9 D3D11 Direct3D 11 OpenGL labels Nov 8, 2018
hrydgard added a commit that referenced this pull request Nov 11, 2018
hrydgard added a commit that referenced this pull request Nov 11, 2018
@hrydgard
Copy link
Owner Author

hrydgard commented Nov 11, 2018

Rebased on #11553 and then master, added support in the detected-memcpy path as well and killed the old MotoGP hack.

This can actually also be used to avoid the sun flare readbacks in Burnout, the sun flare is working but it feels slightly different somehow, maybe because higher resolution. Not sure if I want to enable that - it does end up creating a ton of tiny framebuffers since it varies the size of its readback.

The only important thing left is to make it play nice if a game starts to render to one of these buffers. This can only happen if it's in VRAM, but Digimon is a case where that applies (though I don't know if it actually ever tries to render to one of these buffers).

hrydgard added a commit that referenced this pull request Nov 12, 2018
@hrydgard
Copy link
Owner Author

That last part is done, and it's re-rebased on framebuffers-outside-vram.

@unknownbrackets unknownbrackets changed the base branch from master to font-fixes November 12, 2018 14:08
@unknownbrackets unknownbrackets changed the base branch from font-fixes to master November 12, 2018 14:08
@unknownbrackets
Copy link
Collaborator

I wonder if we want to apply "lower resolution for effects" to this or anything...

-[Unknown]

@unknownbrackets unknownbrackets merged commit df5fbae into master Nov 12, 2018
@hrydgard hrydgard deleted the virtual-readbacks branch November 12, 2018 14:46
@hrydgard
Copy link
Owner Author

Hm, that's an interesting point.. Definitely not for Digimon Adventure outlines, but maybe for Burnout's sun.

@marosis
Copy link

marosis commented Nov 15, 2018

Burnout's sun is still same

@hrydgard
Copy link
Owner Author

I know, I didn't enable this for Burnout yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
D3D9 Direct3D 9 D3D11 Direct3D 11 OpenGL Vulkan
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants