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

renderer_vulkan: Implement rectlist emulation with tessellation #1857

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

raphaelthegreat
Copy link
Collaborator

@raphaelthegreat raphaelthegreat commented Dec 23, 2024

RectLists are a primitive types that renders a quad, given only 3 vertices, by interpolating the fourth one. There is no similar primitive provided by the vulkan API, so we have to emulate it. On main this is a bit of a hack, where we change it to either TriangleStrip/TriangleFan and adjust the number of drawn vertices.

For this to work however, several assumptions must be true. The shader must auto-generate the vertices (no vertex buffer bound), it must also generate a proper fourth vertex to form a quad and only draw 1 quad. These requirements often are true, but not always. And even if they are, sometimes it still doesn't render properly due to changes in ordering.

In this PR we remove the hacks and implement RectList primitive using auxiliary tessellation shaders. The interpolation method is referenced from GRVK: https://github.com/libcg/grvk/blob/03d43a5a/src/amdilc/amdilc_rect_gs_compiler.c#L141

Most existing implementations use a geometry shader for this, but we opted not to for several reasons. Firstly macos doesn't support geoshaders and thus would not be able to benefit, while tessellation shaders should be supported. Also tessellation shaders will work in all cases of RectList usage without interfering with pipeline. It is possible for guest to configure RectLists together with a guest geometry shaders, which would break if we had to insert our own geometry shader. Using tessellation requires patch primitive type instead, so this problem doesn't exist.

Should fix cases of poking triangles in the screen.

@Linear524
Copy link

Is this case related to spiky polygon syndrome in BloodBorne ? (face vertices are blown away sometimes)

@raphaelthegreat
Copy link
Collaborator Author

This shouldn't solve vertex explosions no

@GHU7924
Copy link

GHU7924 commented Dec 23, 2024

Build 059678с

With this PR, I was able to launch [CUSA00005] RESOGUN. (This doesn't happen in "Main" for me.)
I couldn't get into the game, but you can navigate through the menu.
111

Also, this does not apply to this PR, but when running Gravity Rush 2 on this build, an error appears:

[Debug] ajm_at9.cpp:operator():76: Assertion Failed!
Atrac9Decode failed ret = -0x7e000000

shad_log.txt

@Emulator-Team-2
Copy link
Contributor

Emulator-Team-2 commented Dec 23, 2024

This PR improves the graphics in Fist of the North Star: Lost Paradise.

Main Build:
3
PR:
6

(More Screenshots)

Main Build:
1
2

PR:
4
5

Log:
shad_log.zip

@@ -238,7 +238,7 @@ void Rasterizer::Draw(bool is_indexed, u32 index_offset) {
instance_offset);
} else {
const u32 num_vertices =
regs.primitive_type == AmdGpu::PrimitiveType::RectList ? 4 : regs.num_indices;
regs.primitive_type == AmdGpu::PrimitiveType::RectList ? 3 : regs.num_indices;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this supposed to still be changing the num_vertices? Thought this was supposed to support proper number of vertices e.g. multiple rectangles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is debugging leftover yes, can just use regs.num_indices

@@ -1,6 +1,6 @@
// SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project
// SPDX-License-Identifier: GPL-2.0-or-later

#pragma clang optimize off
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this for here?

using Sirit::Id;

struct QuadRectListEmitter : public Sirit::Module {
explicit QuadRectListEmitter(size_t num_attribs_)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should pass supported SPIR-V version to Sirit::Module like so:

: Sirit::Module(profile_.supported_spirv), info{info_}, runtime_info{runtime_info_},

Otherwise emits a validation error on Vulkan 1.2.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can also pass a fixed, lower version (<= 1.5 I think) as I suppose newer isn't needed.

@squidbus
Copy link
Contributor

Seems to be working on Mac from my testing so far, other than the SPIR-V version validation error I mentioned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants