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 #3931

Merged
merged 17 commits into from Dec 18, 2017

Conversation

Projects
None yet
7 participants
@kd-11
Contributor

kd-11 commented Dec 11, 2017

  • Implement deferred swizzle remaps for twice-remapped resources
  • Fix blit engine operations on RGB565
  • Add a timeout on nv406e::semaphore_acquire. Requires further inevstigations why some semaphore addresses always lock up
  • Fix WCB causing hanging due to incorrect memory range approximation
  • Properly synchronize FIFO ctrl updates. Can improve stability in some cases. The issue still happens if the SPUs lag too far behind RSX
  • Implement variable point size #3740
  • Implement FP instructions BEM, TEXBEM, TXPBEM (untested, rarely used)
  • Fix for LG2 fp instruction processing invalid values. Thanks to @MaikelChan for assistance narrowing down the broken shader
  • Framebuffer setup fixes to avoid lost draw calls

#3913
#3740
#3829
#3768
#2194
#3905

//Redirect parameter 0 to the x2d temp register for TEXBEM
//TODO: Organize this a little better
std::pair<std::string, std::string> repl[] = { { "$0", "x2d" } };
std::string result = fmt::replace_all(code, repl);

This comment has been minimized.

@danilaml

danilaml Dec 11, 2017

Contributor

is this used anywhere?

This comment has been minimized.

@kd-11

kd-11 Dec 11, 2017

Contributor

copypasta, should have been used in the next line instead of code var

@@ -98,6 +87,11 @@ namespace rsx
image_type = type;
}
void set_sampler_status(const rsx::texture_sampler_status status)

This comment has been minimized.

@danilaml

danilaml Dec 11, 2017

Contributor

missing ref?

This comment has been minimized.

@kd-11

kd-11 Dec 11, 2017

Contributor

its an enum value so no. Its never used with allocated vars

This comment has been minimized.

@danilaml

danilaml Dec 11, 2017

Contributor

I see, const in signature confused me since it's not needed here (see my other comment) and usually considered bad style.

GLenum remap_values[4];
for (u8 channel = 0; channel < 4; ++channel)

This comment has been minimized.

@danilaml

danilaml Dec 11, 2017

Contributor

using u8 just because the loop variable fits in range isn't really beneficial and can have negative performance (especially if it's used in indexing). plain int /unsigned is usually faster. Also, you can use something like int_fast8_t.

std::vector<rsx_subresource_layout>& subresources_layout, std::pair<std::array<u8, 4>, std::array<u8, 4>>& decoded_remap, bool static_state);
std::vector<rsx_subresource_layout>& subresources_layout, const std::pair<std::array<u8, 4>, std::array<u8, 4>>& decoded_remap, bool static_state);
void apply_swizzle_remap(const GLenum target, const std::array<GLenum, 4>& swizzle_remap, const std::pair<std::array<u8, 4>, std::array<u8, 4>>& decoded_remap);

This comment has been minimized.

@danilaml

danilaml Dec 11, 2017

Contributor

const GLenum target

adding const to pass-by value in declaration is meaningless and can bit omitted.

@@ -457,6 +457,58 @@ namespace vk
m_texture_memory_in_use = 0;
m_discarded_memory_size = 0;
}
VkComponentMapping apply_swizzle_remap(const std::array<VkComponentSwizzle, 4> base_remap, const std::pair<std::array<u8, 4>, std::array<u8, 4>>& remap_vector)

This comment has been minimized.

@danilaml

danilaml Dec 11, 2017

Contributor

do you need to pass base_remap by value here?

@Kravickas

This comment has been minimized.

Contributor

Kravickas commented Dec 11, 2017

Winter star
When entering ingame

F {SPU[0x2000002] Thread (20 spu_runtime) [0x055e0]} class std::runtime_error thrown: Unknown STOP code: 0x100 (Out_MBox is empty)
(in file Emu\Cell\SPUThread.cpp:1772)
F {SPU[0x2000004] Thread (40 spu_runtime) [0x055e0]} class std::runtime_error thrown: Unknown STOP code: 0x100 (Out_MBox is empty)
(in file Emu\Cell\SPUThread.cpp:1772)
F {SPU[0x2000003] Thread (30 spu_runtime) [0x055e0]} class std::runtime_error thrown: Unknown STOP code: 0x100 (Out_MBox is empty)
(in file Emu\Cell\SPUThread.cpp:1772)
F {SPU[0x2000001] Thread (10 spu_runtime) [0x055e0]} class std::runtime_error thrown: Unknown STOP code: 0x100 (Out_MBox is empty)
(in file Emu\Cell\SPUThread.cpp:1772)
F {SPU[0x2000000] Thread (00 spu_runtime) [0x055e0]} class std::runtime_error thrown: Unknown STOP code: 0x100 (Out_MBox is empty)
(in file Emu\Cell\SPUThread.cpp:1772)

after unpause
unkaown
with few frames like this
unknown
and spam E {rsx::thread} RSX: nv406e::semaphore_acquire has timed out. semaphore_address=0x40300560

@MarioSonic2987

This comment has been minimized.

MarioSonic2987 commented Dec 12, 2017

Fixes car's reflection in MCLA with OpenGL:
mcla-2
mcla-1 gl

@kd-11 kd-11 changed the title from rsx: Fixes to [WIP] rsx: Fixes Dec 12, 2017

@stumts

This comment has been minimized.

stumts commented Dec 13, 2017

It fixes Virtua Tennis 4 menu:
captura de tela de 2017-12-13 00-30-53

captura de tela de 2017-12-13 00-23-41

But no change ingame:
captura de tela de 2017-12-13 00-31-16

captura de tela de 2017-12-13 00-24-21

@greentop

This comment has been minimized.

greentop commented Dec 13, 2017

Hyperdimension Neptunia is seeing much improvement. Fixing the blooming in Vulkan will really make this game more playable.
screenshot from 2017-12-13 00-03-00

@Kravickas

This comment has been minimized.

Contributor

Kravickas commented Dec 13, 2017

Winter Star is just worse , less time outs but even less fps ...cant get single ''normal'' render, but that game need spu recompiler for speed anyway more than anything.

@Illynir

This comment has been minimized.

Illynir commented Dec 13, 2017

the last commit on Semaphore timeout is critical on Nier/Nier Replicant. Game freeze 1 second every time the error appears now and Vulkan crash after a while with error:

F {rsx::thread} class std::runtime_error thrown: Assertion Failed! Vulkan API call failed with unrecoverable error: Device lost (Driver crashed with unspecified error or stopped responding and recovered) (VK_ERROR_DEVICE_LOST)
(in file c:\projects\rpcs3\rpcs3\emu\rsx\vk\VKHelpers.h:1130)

@kd-11 kd-11 force-pushed the kd-11:rsx_volatile branch from e55e7a6 to 4dabec0 Dec 18, 2017

kd-11 added some commits Dec 7, 2017

rsx: Implement delayed swizzle remap for blit engine resources
- Fixes remap vectors for memory copied via blit engine as it has no context
rsx: Texture cache fixes
- Handle blit resources in a more consistent way
- TODO: Handle some corner cases (piyotama)
rsx: Fix RGB565 blits. Data is byteswapped on input
- Fixes messed up BG on retroarch glyphs
rsx: Minor fixes
- Abort nv406e semaphore acquire if the rsx thread stalls/crashes
- Fix texture size approximation to take mipmaps into account. Fixes some games hanging with WCB
rsx: Framebuffer setup fixes
- Sometimes square renders are done to surfaces with pitch=64 and re-uploaded with swizzle scanning
-- This setup avoids discarding targets if they are square and pitch == 64
rsx: framebuffer textures do not have mipmaps!
- Force mipmap count to 1 if sampling from an RTV/DSV
- TODO: Better wcb flush detection, it should be better to re-upload the texture after it has been dwnloaded if expected mipmaps are > 1
rsx/fp: Stuff
- Implement BEM
- Add LG2 to special instructions

@kd-11 kd-11 force-pushed the kd-11:rsx_volatile branch from 4dabec0 to 33483b2 Dec 18, 2017

@kd-11 kd-11 changed the title from [WIP] rsx: Fixes to rsx: Fixes Dec 18, 2017

@kd-11

This comment has been minimized.

Contributor

kd-11 commented Dec 18, 2017

@Illynir The two errors seem to be unrelated, but I had to raise the hang detection timeout. 33ms is too short a duration to truly detect if something has gone wrong and attempt a recovery. A more comprehensive solution will be required as well as more tests.

@danilaml Critiques noted. The 'const' madness is borne of gcc paranoia (it's all over the texture code) and will need a more thorough set of changes that I will submit in a separate PR. I don't want to wrestle travis with all the other functional changes in the same PR.

@kd-11 kd-11 merged commit de5dab3 into RPCS3:master Dec 18, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Asinin3 Asinin3 referenced this pull request Dec 18, 2017

Closed

Regressions with #3931 #3947

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