Skip to content

Change sceGuViewport params from int to float #315

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

veka0
Copy link
Contributor

@veka0 veka0 commented Jul 3, 2025

This enables more flexibility in configuring viewport as you'd be able to specify it with subpixel precision.

sceGuViewport can still be used as before, with integer parameters and behavior of existing code will remain the same, however if someone for some reason is already using sceGuViewport with float parameters, by relying on conversion from float to int to truncate fractional part, with this change behavior of such implementations will be slightly different as viewport transform may be off by up to 1 pixel. To me it seems unlikely that someone was using it in this way, or that it will have a major impact and break something, but that's not up to me to decide.

Copy link
Member

@fjtrujy fjtrujy left a comment

Choose a reason for hiding this comment

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

I think this PR is a mistake...
If you want to offer sceGuViewport with float values, we can, but please let's keep the int one with the original name, as it follow same API as the official PSPSDK

so create if you want:

void sceGuViewportF(float cx, float cy, float width, float height)

@veka0
Copy link
Contributor Author

veka0 commented Jul 3, 2025

The idea behind this PR was to extend already existing functionality in a backwards compatible(-ish) way. But it's fair if you don't want to touch existing foundational functions like this one, I think in that case it might be a good idea to drop this PR & potentially consider adding that function if and when a real use case appears for it in the wild.

@fjtrujy
Copy link
Member

fjtrujy commented Jul 3, 2025

The idea behind this PR was to extend already existing functionality in a backwards compatible(-ish) way. But it's fair if you don't want to touch existing foundational functions like this one, I think in that case it might be a good idea to drop this PR & potentially consider adding that function if and when a real use case appears for it in the wild.

Don’t drop the PR, just add the floating function too, I think is a good idea to have it

@sharkwouter
Copy link
Member

I do think adding a sceGuViewportF function is a good idea. Feel free to use this PR to add it.

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.

3 participants