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

core: Rewrite videoout library and bringup new vulkan backend #111

Merged
merged 17 commits into from
Apr 29, 2024

Conversation

raphaelthegreat
Copy link
Collaborator

On master the video/graphics is relatively hard to understand as it's split into multiple folders and directories without much cohesion on what every does. In addition the texture cache is extremely basic and works based on hashing: it will track changes to memory regions by computing its hash. This is fine for simple demos but when real games are put to the test, hashing large blocks of memory every draw call isn't going to be fun. The vulkan code was also a bit fragile and broken under wayland, needing a hack to make it synchronize properly.

So this PR does 3 things, it reworks video_out to be more accurate based on my reverse engineering, fully reworks the vulkan backend side of things to have better abstractions that will make the 3d engine implementation easier and fully reworks the texture caching system to be based on fault tracking.

The first part is mostly self explanatory, the implementation has been split into a separate class for easier state management and some additional error codes have been added, but the result isn't all the different from before. Presentation now occurs in the game thread instead of the window thread, which makes things a bit easier. In the future there should be a separate gpu thread to handle all the extra work but that isn't needed here.

The new vulkan backend is based on the Citra one and uses vulkan-hpp instead of raw vulkan as it's a little bit less verbose and solves the previous license problem, as the C headers are licensed under Apache which is incompatible with GPLv2. Vulkan-Hpp on the other hand is licensed with MIT as well which is ok. Like Citra, initialization is handled by the Instance class where all extensions are also loaded, the Scheduler has been ported as well as it will prove useful for parallel shader building in the future and makes validation layer performance a bit less miserable.

The main change here however is the texture cache. When a new image is stored in the cache, the region it owns is marked as protected using an mprotect call. This means that any reads or writes from the guest will go through the texture cache's exception handler, which will allow it to decide on the appropriate action (either invalidation or flush). When the image is requested again, it is validated with an upload and reprotected. In general that's a relatively clean way to handle readbacks or related accesses emulating a UMA system entails. In the future this can be tuned to be better suited for the PS4s memory model, but it's a good base

CMakeLists.txt Outdated
endif()

if (WIN32)
add_custom_command(TARGET shadps4 POST_BUILD
COMMAND ${CMAKE_COMMAND} -E copy_if_different
add_custom_command(TARGET shadps4 POST_BUILD

Choose a reason for hiding this comment

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

Is there still a need to copy the shared object when you linked it as static?

@@ -24,11 +24,11 @@ bool isNeoMode() {
return isNeo;
}

u32 getScreenWidth() {
u32 GetScreenWidth() {

Choose a reason for hiding this comment

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

Nit, the rest of the project uses camel case, it would be preferable to be consistent

uintptr_t offset;
int protection;
int memory_type;
u32 is_flexible_memory : 1;
Copy link

@SachinVin SachinVin Apr 9, 2024

Choose a reason for hiding this comment

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

Can we steal bitfield.h from dolphin

u32 is_stack : 1;
u32 is_pooled_memory : 1;
u32 is_committed : 1;
char name[32];

Choose a reason for hiding this comment

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

Nit std::array

src/core/libraries/kernel/memory_management.h Outdated Show resolved Hide resolved
enum class MemoryTypes : u32 {
WriteBackOnion = 0,
WriteCombOnion = 3,
WriteBackGarlic = 10,

Choose a reason for hiding this comment

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

Q) Is this not hex?

src/sdl_window.cpp Outdated Show resolved Hide resolved
src/sdl_window.cpp Outdated Show resolved Hide resolved
using ImageId = SlotId;
using ImageViewId = SlotId;

struct Offset2D {

Choose a reason for hiding this comment

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

Nit, maybe it would be better to use something generic like Vec2 and Vec3 like in Citra

src/video_core/renderer_vulkan/vk_platform.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_vulkan/vk_platform.cpp Outdated Show resolved Hide resolved
src/video_core/renderer_vulkan/vk_instance.cpp Outdated Show resolved Hide resolved
src/video_core/texture_cache/image.cpp Outdated Show resolved Hide resolved
* Also fixed build for clang-cl with libc
@georgemoralis georgemoralis merged commit d496fab into shadps4-emu:main Apr 29, 2024
5 of 6 checks passed
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.

None yet

4 participants