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: Fbo fixes #5427

Merged
merged 20 commits into from
Jan 6, 2019
Merged

rsx: Fbo fixes #5427

merged 20 commits into from
Jan 6, 2019

Conversation

kd-11
Copy link
Contributor

@kd-11 kd-11 commented Dec 16, 2018

Reimplements some parts of framebuffer management including:

  • Avoiding generating framebuffer attachments for attachments that are likely unused. This prevents memory corruption when WCB/WDB is not in use since we rely on attachment memory tags to watch for events. Fixes tag_framebuffer causes FIFO corruption in a few games #5406.
  • Rewrites memory inheritance to implicity guarantee correct results even when strict mode is not enabled. This both simplifies the design and potentially moves some more games from strict mode requirement.
  • Fix a texture cache deadlock due to incorrect internal size computations
  • Bump max number of compute invocations on vulkan to 8 times the old value. Real-world testing has shown that it is completely possible to trigger a very high number of readback operations in loading screens. Fixes Soul Calibur [NPEB01363] with WCB crashes on VKCompute.h:137 #5193 and other similar issues.

This is the first in a series of pull requests, so gt_fixes will be made available after this one is merged as they touch some of the same common code.

Copy link
Contributor

@Megamouse Megamouse left a comment

Choose a reason for hiding this comment

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

best review

rpcs3/Emu/RSX/Common/TextureUtils.h Outdated Show resolved Hide resolved
rpcs3/Emu/RSX/GL/GLRenderTargets.cpp Outdated Show resolved Hide resolved
Copy link

@HerrHulaHoop HerrHulaHoop left a comment

Choose a reason for hiding this comment

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

best review so far

rpcs3/Emu/RSX/Common/texture_cache.h Outdated Show resolved Hide resolved
rpcs3/Emu/RSX/GL/GLHelpers.cpp Outdated Show resolved Hide resolved
@MSuih
Copy link
Member

MSuih commented Dec 16, 2018

X-Men Origins: Wolverine no longer needs SRM to fix flickering models. Sadly it's still needed to fix rainbow-colored effects, but that might be AMD-specific.

Skate 3 is very close to playable with this PR, I think the only issues that remain are performance, very rare crashes and teammate naming.

@Xcedf
Copy link

Xcedf commented Dec 16, 2018

#4429 now fixed
Games like Killzone 2 or TeKken Tag 2 work without Strict rendering now, and charcters texturing is also fixed now in Tekken Tag 2 no more black areas with WCB+CPU Blit, scalig works, graphics is perfect.

@Xcedf
Copy link

Xcedf commented Dec 16, 2018

With this PR i have this error
F {RSX [0x02f2ca0]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cf600000, 0x400000, addr=0x3cf900000, error=0x1e7)
in Sonic & Sega All Stras Racing Demo just before ingame
F {RSX [0x1356430]} RSX: class std::runtime_error thrown: VirtualProtect failed (000000034e506000, 0x3f5000, addr=0x34e800000, error=0x1e7)
This one i have in Gran Turismo 5 almost after game booting

@kd-11
Copy link
Contributor Author

kd-11 commented Dec 16, 2018

The virtualprotect failures hint that I may be over-estimating the size of some textures, something which was actually much more broken before. Let me know if you encounter any more applications that trigger this, maybe one of them will be easy to debug.

@Xcedf
Copy link

Xcedf commented Dec 16, 2018

F {RSX [0x037ad40]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cd834000, 0x4001000, addr=0x3cf900000, error=0x1e7) God of War Ascension "Prison of the Damned" Demo, right before the menu.

F {RSX [0x00508f8]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cd85d000, 0x4001000, addr=0x3cf900000, error=0x1e7) Rage Demo, just before ingame

@darkdragonz12
Copy link

soul

Soul calibur V

the error has been corrected with active WCB, to have disabled strict render mode I get this, activating the strict render mode the graphics look good is my observation.

@HenningJW
Copy link

HenningJW commented Dec 16, 2018

I noticed a regression in Catherine, flickering in main menu and fps cut in half, I haven't had time to test it and compare it to the latest master (maybe another PR caused this) and test if WCB fix this. I can test on Friday again if no one else does.

Sorry for this not very helpful report, I'll try to post logs on Friday.

@legend800
Copy link

legend800 commented Dec 16, 2018

Tested about 15 games (see list below) w/w/o WCB and only found 2 regressions from today's mainline:

Backbreaker Vengeance - more gfx corruption (WCB/Blit on - has other existing issues if those are off):
Mainline:
2
1

PR:
3
4

Split Second - vulkan driver error / emu crash(no log) (similar to the Conan regression from a couple months back):
2018-12-16 09_44_33-window

Here's all the games tested (you can disregard the playability notes):
• Backbreaker Vengeance(v1.0-eu-c00)[WCB(field cache), Blit(fixes textures on VK)] Note: Broken text (3 minor areas-WCB), broken field logo (WCB), missing crowd (partially shows with blit) – Black endzone on new pr which fixes glow/only blocker remaining
• (d)Captain America(3D)[Stretch, Strict (Cap shading)] – Slow(~15 fps on curr. lvl)
• (d)Conan [WCB+Blit – stability on vulkan] – missing textures, (hangs during loads sometimes now-logged)
• NFL Blitz (v1.0) – little slow, some gfx issues (logged), bad player physics(logged)
• Ridge Racer 7(3D)(25-special events) – [SPU LLVM+xfloat off(curr. broken 11/14)] – Tree flickering on x stages
• (d)Splatterhouse (1-survival)(eur) PPU mem crashes (needs PPU interpreter for certain areas like start of chapters-but 15 fps)
• Wipeout HD (1-Fury)(v2.51)[WCB for bloom] Slow (20 at start, 40 avg.)
• Force Unleashed 2 – Hangs randomly, Long loads/HDD reading, blue screen vids/gameplay
• Split Second – Slow, Bad gfx
• Double Dragon Neon – Broken gfx (background)
• *(d)TC 4 [PS Move or Mouse(Y button, can’t skip cinematics)] Settings: WCB | Note: Little slow w/o TSX
• *(d)Deadstorm Pirates(+install data) [PS Move (Mouse(Y button) can’t press Start to Continue, could alternative P1 & P2)] Settings: WCB | Note: Minor transparent textures, Must enter config when Move is full red

@Aurez
Copy link

Aurez commented Dec 16, 2018

Just tested and no: Tekken Tag Tournament 2 still needs Strict Rendering Mode for the 2 last stages.

@elhaya
Copy link

elhaya commented Dec 16, 2018

fixes heavy rain with WCB :)
image

@Xcedf
Copy link

Xcedf commented Dec 16, 2018

Confirm Heavy Rain fixed
found Another error
F {RSX [0x1481f90]} RSX: class std::runtime_error thrown: VirtualProtect failed (000000033d608000, 0x3f5000, addr=0x33d900000, error=0x1e7)
Gran Turismo HD Concept, right after booting the game

@Dani1977
Copy link

Hi,
I do not know if this PR looking for to fix the problem of Demon's Souls with CPU 4/4 core (like my i7-7700k).

Unfortunately, the game continues to crash and also emulator crash too.

RPCS3.zip

event viewer:

Nome dell'applicazione che ha generato l'errore: rpcs3.exe, versione: 0.0.0.0, timestamp: 0x5c164d3a
Nome del modulo che ha generato l'errore: ucrtbase.dll, versione: 10.0.17763.1, timestamp: 0x309241e0
Codice eccezione: 0xc0000409
Offset errore 0x000000000006f08e
ID processo che ha generato l'errore: 0x3470
Ora di avvio dell'applicazione che ha generato l'errore: 0x01d4956ff409508d
Percorso dell'applicazione che ha generato l'errore: G:\aa\rpcs3.exe
Percorso del modulo che ha generato l'errore: C:\WINDOWS\System32\ucrtbase.dll
ID segnalazione: e32fac44-0c67-4f10-a42a-88c67762ee2f
Nome completo pacchetto che ha generato l'errore:
ID applicazione relativo al pacchetto che ha generato l'errore:

Regards

@MsDarkLow
Copy link
Contributor

More VirtualProtect Fatals, Yakuza 5, Dead Souls, Ishin and Kiwami now crash after boot.
Yakuza 5:

F {RSX [0x07a7ff0]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cf8bf000, 0x100000, addr=0x3cf900000, error=0x1e7)

Yakuza Dead Souls:

F {RSX [0x03005f0]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cf8f7000, 0x20000, addr=0x3cf900000, error=0x1e7)

Yakuza Ishin:

F {RSX [0x01008d0]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cf8f7000, 0x20000, addr=0x3cf900000, error=0x1e7)

Yakuza Kiwami:

F {RSX [0x01008f0]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cf8ef000, 0x40000, addr=0x3cf900000, error=0x1e7)

@Xcedf
Copy link

Xcedf commented Dec 16, 2018

Motorstorm speed regression, commit 01f7ad4
Before
12
After
6
Reverting fixes it , and it seems fixes VirtualProtect failed errors, but undo Heavy Rain fix.

@stride21
Copy link

Killzone 3:
F {RSX [0x02d6534]} RSX: class std::runtime_error thrown: VirtualProtect failed (00000003cf7c5000, 0x201000, addr=0x3cf900000, error=0x1e7)

Battlefield: Bad Company:
F {RSX [0x0024f08]} RSX: class std::runtime_error thrown: VirtualProtect failed (0000000370c55000, 0x101000, addr=0x370d00000, error=0x1e7)

@stride21
Copy link

In Motorstorm the 2nd and 3rd races go ingame again with this PR but I can also confirm that I'm seeing the same performance regression.

@kd-11
Copy link
Contributor Author

kd-11 commented Dec 17, 2018

A lot of the regressions reported are due to incorrect texture size approximation, or rather, it was so bad before (seriously under-estimated size) that I do not know if its a real bug or if fixing the problem correctly just uncovered other issues. A solution will be made available in the next few hours though.

@kd-11
Copy link
Contributor Author

kd-11 commented Dec 17, 2018

The regressions should be fixed now, I forgot to account for number-of-texels decoded per single row of bytes which caused overestimated size of S3TC data. Should be resolved now.

@kd-11
Copy link
Contributor Author

kd-11 commented Dec 17, 2018

Found another small error, crash may still happen.

@Xcedf
Copy link

Xcedf commented Dec 17, 2018

On new build all of my VirtualProtect failed errors still happen, and become
13
Motorstorm even slower

kd-11 added 16 commits January 5, 2019 22:33
- Implicitly invoke a memory barrier if actively reading from an unsynchronized texture
- Simplify memory transfer operations
- Should allow more games to work without strict mode
- Also account for variable pitch textures (swizzled scan)
…alidation

- If draw call resources consume memory that intersects with NA parts of the texture cache, we get a framebuffer test mismatch.
  This mismatch is false and happens because the thread has not yet reached the point of relocking the pages
- Remove the required_xxx_pitch constraint as it makes no sense. The pitch controls what can be written per line.
- It is possible to have a huge surface width but only render to a small region at the beginning and have a smaller pitch than can fit the surface (NFS carbon)
- gl: Include an execution state wrapper to ensure state changes are consistent. Also removes a lot of required 'cleanup' for helper methods
- texture_cache: Make execition context a mandatory field as it is required for all operations. Also removes a lot of situations where duplicate argument is added in for both fixed and vararg fields
- Explicit read/write barrier for framebuffer resources depending on
  usage. Allows for operations like optional memory initialization before
  reading
- This silly typo broke the flip improvements in the GT fixes PR
- D24S8 targets have 2 aspects that are dealt with separately; Forcefully initialize the remaining data if a partial init is done. Its 'free' anyway
- It seems that the stencil mask matters when clearing unlike the depth mask and color mask
- Set up partial transfers
- Force clear of target before starting the transfer
- Pitch 0 makes sense if width == 1 and height == 1
- Avoid tagging and rely on read/write barriers and the dirty flag mechanism. Testing is done with a weak 8-byte memory test
- Introducing new data when tagging breaks applications with race conditions where tags can overwrite flushed data
@legend800
Copy link

This PR reintroduced this issue which was actually fixed a few weeks prior:
#4949

@Josivan88
Copy link

affect resident evil 5
before
rpcs3 before fbo
After
rpcs3 after fbo
RPCS3.log.gz

@stride21
Copy link

stride21 commented Jan 16, 2019

This PR fixed some of the issues with black lines going horizontally across the screen when aiming down sight.

Before (OpenGL + SRM):
before fib

After (OpenGL + SRM):
fio opengl

Before (Vulkan + SRM):
vulkan before 5

After (Vulkan + SRM):
fio vulkan

Before (WCB + SRM):
original wcb

After (WCB + SRM):
wcb

Edit: When using Vulkan on this PR the auger shows enemies through the wall on the left half of the screen.

Before (Vulkan + SRM):
resistance prev

After (Vulkan + SRM):
resistance 3 vulkan master

After (OpenGL + SRM)
resistance 3 opengl master

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