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

Workaround bug on logic op with float framebuffer #6858

Merged
merged 4 commits into from
May 24, 2024

Conversation

piplup55
Copy link
Contributor

@piplup55 piplup55 commented May 23, 2024

built on top of #6852 to add support for intel gpu's
NOTE: this won't fix the crashing on windows

fixes #6859

before:
image

after:
image

built on top of the amd workaround
@github-actions github-actions bot added gpu Related to Ryujinx.Graphics graphics-backend:vulkan Graphical bugs when using the Vulkan API labels May 23, 2024
@ryujinx-mako ryujinx-mako bot requested review from gdkchan, riperiperi and a team May 23, 2024 13:11
@piplup55 piplup55 added the graphics-vendor:intel Marks an issue affecting only Intel gpus label May 23, 2024
@gdkchan
Copy link
Member

gdkchan commented May 23, 2024

Maybe it's better to enable it for all GPUs that are not NVIDIA.

@piplup55
Copy link
Contributor Author

Maybe it's better to enable it for all GPUs that are not NVIDIA.

so we would enable it for nvidia only, i guess that makes more sense, i'm unware if it affects any other vendors

so something like this
bool logicOpEnable = LogicOpEnable && (gd.Vendor == Vendor.Nvidia || Internal.LogicOpsAllowed);
instead of
bool logicOpEnable = LogicOpEnable && (gd.Vendor != Vendor.Amd && gd.Vendor != Vendor.Intel || Internal.LogicOpsAllowed);

@gdkchan
Copy link
Member

gdkchan commented May 23, 2024

Maybe it's better to enable it for all GPUs that are not NVIDIA.

so we would enable it for nvidia only, i guess that makes more sense, i'm unware if it affects any other vendors

so something like this bool logicOpEnable = LogicOpEnable && (gd.Vendor == Vendor.Nvidia || Internal.LogicOpsAllowed); instead of bool logicOpEnable = LogicOpEnable && (gd.Vendor != Vendor.Amd && gd.Vendor != Vendor.Intel || Internal.LogicOpsAllowed);

Yes. Most of the other vendors are mobile vendors that don't even support logic ops at all. There's newer Adreno GPUs that supports it I think, and Samsung Xclipse GPUs, but I guess those would have the bug too since they are based on AMD RDNA architecture.

@piplup55
Copy link
Contributor Author

Yes. Most of the other vendors are mobile vendors that don't even support logic ops at all. There's newer Adreno GPUs that supports it I think, and Samsung Xclipse GPUs, but I guess those would have the bug too since they are based on AMD RDNA architecture.

i'll make the change and test it on amd and intel, it should work in theory but just in case

Enabled workaround for all vendors that aren't nvidia
@piplup55 piplup55 changed the title Workaround Intel bug on logic op with float framebuffer Workaround bug on logic op with float framebuffer May 23, 2024
@Ozzel05

This comment was marked as off-topic.

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, minus comment.

src/Ryujinx.Graphics.Vulkan/PipelineState.cs Outdated Show resolved Hide resolved
@ryujinx-mako ryujinx-mako bot requested a review from a team May 23, 2024 17:58
@gdkchan gdkchan merged commit c98b7fc into Ryujinx:master May 24, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gpu Related to Ryujinx.Graphics graphics-backend:vulkan Graphical bugs when using the Vulkan API graphics-vendor:intel Marks an issue affecting only Intel gpus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]Paper Mario TTYD black screen
4 participants