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

rsx: Enable multisampling #6055

Merged
merged 22 commits into from
Jun 14, 2019
Merged

rsx: Enable multisampling #6055

merged 22 commits into from
Jun 14, 2019

Conversation

kd-11
Copy link
Contributor

@kd-11 kd-11 commented Jun 6, 2019

  • Adds MSAA support (vulkan only for now since almost all testers will run vulkan)
  • Completely refactors out D3D12 as it was holding back progress with shared code.
  • Minor improvement to fp decompiler input modifiers.

@Xcedf
Copy link

Xcedf commented Jun 6, 2019

Resistance 2
AA Auto
E {RSX [0x6c0e4b4]} RSX: Resolve shader:
#version 420
#extension GL_ARB_separate_shader_objects : enable
#extension GL_ARB_shader_stencil_export : enable

layout(std140, set=0, binding=0) uniform static_data{ ivec4 regs[8]; };
layout(set=0, binding=1) uniform sampler2DMS fs0;
layout(set=0, binding=2) uniform usampler2DMS fs1;
layout(pixel_center_integer) in vec4 gl_FragCoord;

void main()
{
ivec2 out_coord = ivec2(gl_FragCoord.xy);
ivec2 in_coord = (out_coord / regs[0].xy);
ivec2 sample_loc = out_coord % ivec2(regs[0].xy);
int sample_index = sample_loc.x + (sample_loc.y * regs[0].y);
float frag_depth = texelFetch(fs0, in_coord, sample_index).x;
uint frag_stencil = texelFetch(fs1, in_coord, sample_index).x;
gl_FragDepth = frag_depth;
gl_FragStencilRefARB = int(frag_stencil);
}

F {RSX [0x7075ab4]} RSX: class std::runtime_error thrown: Verification failed (e=0x7f):
(in file c:\projects\rpcs3\rpcs3\emu\rsx\vk\vkrendertargets.h:334)

AA Disabled
just

F {RSX [0x7075ab4]} RSX: class std::runtime_error thrown: Verification failed (e=0x7f):
(in file c:\projects\rpcs3\rpcs3\emu\rsx\vk\vkrendertargets.h:334)

Same for Killzone 2

@kd-11
Copy link
Contributor Author

kd-11 commented Jun 6, 2019

Resolve shader message is unrelated. I just left it as log_error in case of compilation errors.

@Xcedf
Copy link

Xcedf commented Jun 6, 2019

Well it's just verification fail then, and even if i disable it the game still ends with some error

@kd-11
Copy link
Contributor Author

kd-11 commented Jun 6, 2019

Yea, looks like some files on my end are corrupted; I'm reinstalling some demos and re-running more tests. The verification crash at least is reproducible.

@arabek
Copy link
Contributor

arabek commented Jun 6, 2019

Killzone 2 fails for me too, even after nuking all the caches.

@kd-11
Copy link
Contributor Author

kd-11 commented Jun 6, 2019

Fixed the VkRenderTargets.h crash caused by only testing MSAA x2 during development. MSAA x4 was completely broken which is what killzone 2 is using.

@kd-11
Copy link
Contributor Author

kd-11 commented Jun 6, 2019

Several other issues identified by testers and under investigation (avoid repeating these):

  • Performance slowdown in ni no kuni (even compared to early MSAA-enabled builds) Fixed.
  • Graphics corruption in ni no kuni when MSAA is disabled. Fixed.

@Xcedf
Copy link

Xcedf commented Jun 6, 2019

Now that's what i always expected
Resistance 2 (Native res)
master
61
60

@MarioSonic2987
Copy link
Contributor

MarioSonic2987 commented Jun 6, 2019

Comparisons:

God of War III (E3 2009 Demo) - Minor improvement

image
image

Midnight Club: Los Angeles (200% scaling)

image
image

Racedriver: Grid (200% scaling)

image
image

Red Dead Redemption (200% scaling)

image
image

Tekken Tag Tournament 2

image

Uncharted 2: Among Thieves

image

@Kravickas
Copy link
Contributor

Kravickas commented Jun 6, 2019

Crysis Approve 👍
pr
master

@stride21
Copy link

stride21 commented Jun 6, 2019

Master:
Master Ratchet
PR:
ratchet pr
Master:
resis master
PR:
resis 3 pr fixed

@@ -234,17 +218,12 @@ void GLGSRender::init_buffers(rsx::framebuffer_creation_context context, bool sk

const u8 color_bpp = get_format_block_size_in_bytes(layout.color_format);
const u8 depth_bpp = (layout.depth_format == rsx::surface_depth_format::z16 ? 2 : 4);
Copy link
Contributor

@scribam scribam Jun 6, 2019

Choose a reason for hiding this comment

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

I think the variable depth_bpp can be removed unless you prefer to replace:

m_depth_surface_info.bpp = (layout.depth_format == rsx::surface_depth_format::z16? 2 : 4);

by

m_depth_surface_info.bpp = depth_bpp;

@MsDarkLow
Copy link
Contributor

MSAA seems to work as intended in 4. However, I noticed a difference in colors in Yakuza 3, 4 and Ishin, they seem to show up incorrectly.

Yakuza 3 (Doesn't seem to support MSAA)

Master
Y3Master

PR
Y3

Yakuza 4

Master
Y4Master

PR MSAA
Y4Msaa

PR
Y4

Yakuza Ishin (Doesn't seem to support MSAA???)

Master
IshinMaster

PR MSAA
IshinMsaa

PR
Ishin

@e1489
Copy link

e1489 commented Jun 7, 2019

;=; how do I try this?

@Artins90
Copy link

Artins90 commented Jun 7, 2019

;=; how do I try this?

Click show all checks below.
Select details near appveryor
Open artifacts and dowload the 17MB file
The antialising option is in the GPU options section, select auto to activate it.

@dbz400
Copy link
Contributor

dbz400 commented Jun 7, 2019

D3D12 renderer selection box will be removed as well ?

@AniLeo
Copy link
Member

AniLeo commented Jun 7, 2019

d3d12 is not yet fully removed

@kd-11
Copy link
Contributor Author

kd-11 commented Jun 7, 2019

@MsDarkLow

However, I noticed a difference in colors in Yakuza 3, 4 and Ishin, they seem to show up incorrectly.

Maybe show the differences clearly? I cannot spot any differences since I'm not familiar with those games. There are some tweaks to the fragment decompiler based on hardware testing on a PS3, but I still have doubts as to the accuracy; if the commit is wrong compared to PS3 I can revert it from the PR and submit it separately at a later date.

@anseesaw
Copy link

anseesaw commented Jun 7, 2019

What does this mean for d3d12,Improve or delete?
1
2

@RainbowCookie32
Copy link
Contributor

RainbowCookie32 commented Jun 7, 2019

DX12 isn't being improved. I'd say that we are one step closer to removal with this PR (I think)

@MarioSonic2987
Copy link
Contributor

MarioSonic2987 commented Jun 8, 2019

TTT2 still crashes in Fallen Garden with MSAA enabled:
Log TTT2 Broken 2

Killzone 2 has flickering shadows and blurred graphics.

Master:
image

This PR:
image

@kd-11 kd-11 force-pushed the msaa branch 2 times, most recently from 13176cc to 12a0809 Compare June 8, 2019 09:27
@MsDarkLow
Copy link
Contributor

@kd-11

Maybe show the differences clearly? I cannot spot any differences since I'm not familiar with those games. There are some tweaks to the fragment decompiler based on hardware testing on a PS3, but I still have doubts as to the accuracy; if the commit is wrong compared to PS3 I can revert it from the PR and submit it separately at a later date.

I got a better scenario with Ishin which is way more clear. Took screenshots in the main menu including how it looks on a PS3.

Yakuza Ishin

PS3
PS3_Like_A_Dragon_Ishin

Master
IshinMaster

PR (both OpenGL & Vulkan look like this)
IshinPR

kd-11 added 21 commits June 14, 2019 15:56
- Remove deprecated functionality
- Refactor to share code between common routines
- Requires native_pitch value to take samples into account
- vk: Enable depth buffer resolve+unresolve
- vk: Add AMD stenciling extension support
- rsx: Temporarily disables MSAA-compatible hacks such as transparency AA
- TODO: Add paths to optionally disable MSAA
- Resolve image first before performing any transfer operations
- TODO: Option to completely skip clamping in some architectures as it is not needed in most games
- Mostly affects older GPUs that do not have access to native fp16
- Refactor overlays and resolve passes to support use of push constants instead of relying buffer map/unmap
- Add support for nvidia resolve (NV is the only vendor not supporting shader_stencil_export)
- Arguments to the transform function are xxyy not xyxy
…henever the parent is written to

- Fixes successive reads of an antialiased surface that is still bound between reads
- Missing ref increment when using framebuffer could lead to use-after-free.
  How master was not crashing is surprising
@kd-11
Copy link
Contributor Author

kd-11 commented Jun 14, 2019

TTT2 crash is now fixed.

@AizerMortenort
Copy link

Hello anyone able to fix Dynasty Warriors Gundam Reborn crash ?

@kd-11 kd-11 merged commit e515d9b into RPCS3:master Jun 14, 2019
@AizerMortenort
Copy link

Whats so wrong into bringing the crash from Dynasty Warriors Gundam Reborn to a developer's attention? Game crashes for years with same error so i dont see any update fixing it. Thanks

@RainbowCookie32
Copy link
Contributor

Because it's completely unrelated to this PR

@AniLeo
Copy link
Member

AniLeo commented Jun 14, 2019

You're wasting a developer's time with a false regression report, it's completely unrelated to this pull request

@AizerMortenort
Copy link

Still this is not a reason for hate.

@RPCS3 RPCS3 locked as off-topic and limited conversation to collaborators Jun 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.