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.

Sign up for GitHub

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

Jump to bottom

GPU: Remove unused dynamic state settings #6796

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

GPU: Remove unused dynamic state settings #6796

wants to merge 2 commits into from
+8 −30

Conversation

sunshineinabox
Copy link
Contributor

  • Removed dynamic state setting for Depth Bounds from PipelineDynamicStateCreateInfo as the command to set them is never used. At best this means no change. At worst this may have caused some validation errors?
  • Avoid passing pointers for viewport and scissor since these dynamic states are supported across devices that support Vulkan.
  • Removed references to unused DepthBias values and StencilOpState for both Front and Back.

This change likely has no user facing changes, but just need to check if there is anything broken. Just clean-up for future changes.

@github-actions github-actions bot added gpu Related to Ryujinx.Graphics graphics-backend:vulkan Graphical bugs when using the Vulkan API labels May 12, 2024
@ryujinx-mako ryujinx-mako bot requested review from gdkchan, riperiperi and a team May 12, 2024 05:16
@github-actions GitHub Actions
Copy link

github-actions bot commented May 12, 2024 •
edited

Download the artifacts for this pull request:

Old GUI (GTK3)
GUI-less (SDL2)
Only for Developers

@AcK77 AcK77 changed the title Remove unused dynamic state settings gpu: Remove unused dynamic state settings May 14, 2024
@AcK77 AcK77 changed the title gpu: Remove unused dynamic state settings GPU: Remove unused dynamic state settings May 14, 2024
gdkchan
gdkchan approved these changes May 16, 2024
View reviewed 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 minus comment. This is most likely a leftover from when we didn't use dynamic state at all.

@ryujinx-mako ryujinx-mako bot requested a review from a team May 16, 2024 18:16
@sunshineinabox
Dynamic state for Depth Bounds should not be passed to PipelineDynami…
c6c12c8 
…cStateCreateInfo as the command to set them is never called.

Do not pass pointer to viewport and scissor as those dynamic states should be supported on all devices.

Same as above for DepthBias values.
@sunshineinabox
Code Review Suggestion
882bb52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gdkchan gdkchan gdkchan approved these changes

@riperiperi riperiperi Awaiting requested review from riperiperi

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
gpu Related to Ryujinx.Graphics graphics-backend:vulkan Graphical bugs when using the Vulkan API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sunshineinabox @gdkchan