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: Fixes #3664

Merged
merged 5 commits into from
Oct 28, 2017
Merged

rsx: Fixes #3664

merged 5 commits into from
Oct 28, 2017

Conversation

kd-11
Copy link
Contributor

@kd-11 kd-11 commented Oct 27, 2017

  1. Texture cache fixes
  • Tag framebuffer memory when strict mode is enabled to check if content has been written to. This way, framebuffer contents can be ignored if they are known to be incorrect.
  • Rework memory protection and ignore flush requests if the data has already been written to the CPU. Speedup when using WCB
  1. Stability fixes (See Regressions with #3638 #3649)
  • Vulkan: Flush command queue before attempting to perform texture writeback to guarantee draw order
  • Vulkan: Tag primary command buffers with a flush_only access hint to ensure they are always reopened if submitted outside flush_command_queue such as when dealing with access violations

@kd-11 kd-11 mentioned this pull request Oct 27, 2017
@Asinin3
Copy link
Contributor

Asinin3 commented Oct 27, 2017

Army of Two, Ni No Kuni and Young justice no longer crash RPCS3 with a nvglv64.dll error when using Vulkan+WCB. Young Justice does freeze without any specific error in log at the same point (loading screen) though. Which is a regression as it used to go ingame before #3638 (although with completely broken graphics).
YoungJusticeLogFix_PR.gz

@kd-11
Copy link
Contributor Author

kd-11 commented Oct 27, 2017

Freezing is expected and will be fixed in a later PR. As with everything else its about balancing performance and guaranteed accuracy.

@Asinin3
Copy link
Contributor

Asinin3 commented Oct 27, 2017

The unreleased texture issue seems to have come back.

Ni No Kuni - PR: 400-750 unreleased Master: 0-10
kdpr_2017-10-28_06-39-21
Persona 5 Unreleased Textures - PR:250-300 Master: 2-10
kdpr_2017-10-28_06-45-55
Gatling Gears Unreleased Textures: PR: over 10,000 Master: 14
image
Gatling Gears also renders at the wrong position when using resolution scaling now, UI elements (such as the menu seen here) are fine but the game doesn't scale properly to the window even when full-screened. It just shows the top left corner. I did not notice this in any other games though, it may be an edge case.

Graphics no longer flicker at least (regression from #3638 ).

@greentop
Copy link

No changes observed in ToCS2. Is currently in a great running state.
screenshot from 2017-10-27 16-01-46

@kd-11
Copy link
Contributor Author

kd-11 commented Oct 27, 2017

@Asinin3

  1. Scaling has never worked with write color buffers + vulkan. Vulkan does not like to be used on demand like that as it uses deferred processing of queues and would suffer significant slowdown if resizing of the data is attempted. A compromise is to crop the image read back from the gpu. Always use 100% scaling or openGL for that.
  2. The unreleased value can go up quite a bit, as long as memory consumption does not go very high (i.e small textures are occupying the space) its not a big deal. Verify with GPU monitoring tools that VRAM usage is excessive (e.g over 800M VRAM usage)
  3. I have pushed an update for the freezing issue with wcb that can happen in some games. Check if the situation has changed. The mechanism can still fail but I feel the majority of cases are properly handled now.

EDIT: Its possible that there is double counting of sections causing the reported value to grow excessively while the real number is quite small. Its also possible that there is excessive invalidation of sections going on. I'll look into what could be causing that to happen

@Asinin3
Copy link
Contributor

Asinin3 commented Oct 27, 2017

  1. But what about Demon's Souls? Many people have been playing that with Vulkan+WCB at high resolutions with no issues getting it to render at 4k for instance. This is the first time I'm hearing about scaling not working well with Vulkan + WCB it always worked okay for me.

  2. I checked VRAM usage on the Master vs PR for Gatling Gears and my VRAM usage went from around 850MB to 1.4GB, nothing major but fps went from around 10.5 down to 1.4. Ni No Kuni only saw a small 30mb increase in vram but also saw a significant performance loss. FPS went from 18.5 to 13.7. Keep in mind I had WCB disabled for Ni No Kuni in this test.

  3. I managed to get ingame in Young Justice with d9671bf so you did fix that hang. Then I tried Army of Two and it still had a random hang without any useful information in the log. But this happened before rsx: Fixes #3638 was merged so not a regression. Great work 👍

Edit: LBP Karting doesn't crash with d9671bf as well.

@kd-11
Copy link
Contributor Author

kd-11 commented Oct 28, 2017

@Asinin3 Demon's Souls uses cpu readback to only fetch a 16x16 luminance buffer, not actual color data. Thats why the threshold slider is there. At default settings, 16x16 is below the threshold and is not actually scaled. The same goes for any game that uses the readback for anything that is not actual color buffer data.
The cache thrashing issue should be fixed now with 427df64. Performance should be back to normal now and cache evictions should be minimal now, probably even less than master (unreleased textures counter)

@Asinin3
Copy link
Contributor

Asinin3 commented Oct 28, 2017

427df64 Fixed Performance loss with Gattling Gears VRAM usage is also normal now too 👍 I'll try a few other games but everything seems good so far. The scaling issue is still there but it really doesn't matter as it seems to be an edge case.

Edit: Ni No Kuni performance back to normal as well.

@kd-11
Copy link
Contributor Author

kd-11 commented Oct 28, 2017

Imo, wcb should not be used with scaling if color data is being written back to the cpu. It will downsample the image to fit in the cpu space and then the image will be stretched again to fit the screen. This usually gives very blurry result due to the upscale->downscale->upscale cycle where information is lost every time. I will however set up a mechanism to scale the output if conditions are right.

kd-11 added 5 commits October 28, 2017 12:46
- vk: Always reopen primary command buffers. They should only be closed in flush_command_queue
- If uploading a texture and there are collisions with protected buffers, do not rebuild the cache
- Perform writes via flush before reprotecting pages that were not trampled
- Only flush no pages once
…ing violations

- This allows for better handling of deferred flushes.
-- There's still no guarantee that cache contents will have changed between the set acquisition and following flush operation
-- Hopefully this is rare enough that it doesnt cause serious issues for now
@kd-11 kd-11 merged commit 7abf755 into RPCS3:master Oct 28, 2017
@Kravickas
Copy link
Contributor

Kravickas commented Oct 30, 2017

half fps (12->6) in midnight club LA after this merge

WCB / GPU scal. doesnt matter

@Psycho-A
Copy link

Please pay attention to this: #3681

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