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

(Graphics.Shader): Handle EmitSuatom constant dests and EmitSuld zero dest reg. #5504

Merged
merged 2 commits into from Jul 31, 2023

Conversation

LDj3SNuD
Copy link
Contributor

@LDj3SNuD LDj3SNuD commented Jul 30, 2023

Contributes to Ryujinx/Ryujinx-Games-List#2828.

Now the game should go ingame.

Reference: #5502 (comment)

@github-actions github-actions bot added the gpu Related to Ryujinx.Graphics label Jul 30, 2023
@gdkchan
Copy link
Member

gdkchan commented Jul 30, 2023

Can you fix EmitSuld too? It has the same issue. But in that case, dests array will be empty if srcB is equal to RegisterZeroIndex. There are places that accesses dests[0] so it is assuming that the size will be at least 1. Also I'm not sure what the effect of passing an empty array on the Operation would be in that case. For the EmitSuld case I think just doing if (srcB == RegisterConsts.RegisterZeroIndex) return; (properly formatted) would be fine since unlike SUATOM, SULD with a zero destination register should be a no-op.

@LDj3SNuD
Copy link
Contributor Author

surely, in a few hours

@LDj3SNuD
Copy link
Contributor Author

okay, I will correct, I have no experience in GPU

@LDj3SNuD
Copy link
Contributor Author

LDj3SNuD commented Jul 30, 2023

Let me know if this is okay; and if I need to increment any flags.
I confirm that the game works with those changes.

Copy link
Member

@gdkchan gdkchan left a comment

Choose a reason for hiding this comment

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

lgtm, thanks.

@gdkchan
Copy link
Member

gdkchan commented Jul 30, 2023

Let me know if this is okay; and if I need to increment any flags. I confirm that the game works with those changes.

It looks correct now (please test with Jurassic Park to be sure, with shader cache disabled or purged, otherwise it might be using the shaders from your previous test).

There is no need to increment the shader cache version since the bug makes it crash before the shader is generated, so the emulator wouldn't have a chance to save the broken shader in the cache.

@LDj3SNuD
Copy link
Contributor Author

LDj3SNuD commented Jul 31, 2023

Ok thanks, I always retest with the caches disabled or cleared.
About EmitSuld, the game would seem to call that method only with 2, 3 and 38 for srcB; until ingame at least.

@LDj3SNuD LDj3SNuD changed the title (Graphics.Shader): Handle EmitSuatom constant dests. (Graphics.Shader): Handle EmitSuatom constant dests and EmitSuld zero dest reg. Jul 31, 2023
Copy link
Member

@riperiperi riperiperi left a comment

Choose a reason for hiding this comment

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

LGTM

@gdkchan gdkchan merged commit 86931cc into Ryujinx:master Jul 31, 2023
10 checks passed
@LDj3SNuD LDj3SNuD deleted the emit_suatom_fix branch August 1, 2023 12:09
sunshineinabox pushed a commit to sunshineinabox/Ryujinx that referenced this pull request Aug 8, 2023
… dest reg. (Ryujinx#5504)

* (Graphics.Shader): Handle EmitSuatom constant dests.

* Proper fix for EmitSuatom; fix EmitSuld.
jcm93 pushed a commit to jcm93/Ryujinx that referenced this pull request Aug 15, 2023
… dest reg. (Ryujinx#5504)

* (Graphics.Shader): Handle EmitSuatom constant dests.

* Proper fix for EmitSuatom; fix EmitSuld.
@ryujinx-mako ryujinx-mako bot requested a review from a team March 4, 2024 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gpu Related to Ryujinx.Graphics
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants