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

UI on all platforms(including NewUI!): Add options to change FramebufferCPUConvert #2851

Merged
merged 4 commits into from
Jul 20, 2013

Conversation

thedax
Copy link
Collaborator

@thedax thedax commented Jul 19, 2013

Needed for non-NVidia GPU PCs and mobile devices in games like Danganronpa(for whatever reason, it crashes for Intel/Amd/Android if CPU Convert isn't on, and it shouldn't). Not everyone roots or wants to root, nor edit an INI by hand often.

…ferCPUConvert without manual editing of INI file.
@thedax
Copy link
Collaborator Author

thedax commented Jul 19, 2013

I had to move Display raw FB to a different page so it could fit the convert CPU option..

@dbz400
Copy link
Contributor

dbz400 commented Jul 20, 2013

BTW , it would nice to grey these two framebuffer options when buffered rendering OFF (but i'm busy other stuff so cannot work on this )

@thedax
Copy link
Collaborator Author

thedax commented Jul 20, 2013

@raven02 : This is my first time poking around in NewUI; does it have functionality to enable/disable specific items after being added? If it does, I certainly missed it..

@hrydgard
Copy link
Owner

@thedax , not easily right now, I'll have to think about how we do that best.

@hrydgard
Copy link
Owner

By the way is the CPU conversion a big speed hit on nvidia? We should probably default it to true as it appears to be required on most non-nvidia chips...

@thedax
Copy link
Collaborator Author

thedax commented Jul 20, 2013

@hrydgard : Um, I haven't noticed any real speed issues with it on, but I don't really think it does anything on NVidia anyway, so I don't -think- it would hurt to have it on if FramebuffersToMem is also enabled.

Edit: I did a quick test with Danganronpa 1 with both Framebuffer options on and then only FramebuffersToMem on and there was zero speed/VPS difference(tested by looking at the Speed when holding down tab), though we do have to remember my PC is pretty beefy..

@hrydgard
Copy link
Owner

So do this then: Set to true by default, and remove the option on mobile platforms using #ifndef USING_GLES2 . Then I guess slow nvidia users can try to turn it off, and it "just works" for everyone else.

@thedax
Copy link
Collaborator Author

thedax commented Jul 20, 2013

That should do it. Let's hope it doesn't slow down mobile too badly..or is it time to consider making the framebuffer options an all in one solution(e.g. one button rules them all)?

@hrydgard
Copy link
Owner

Yeah, I think so, unless we can get the conversion that doesn't involve the CPU working on mobile.

hrydgard added a commit that referenced this pull request Jul 20, 2013
UI on all platforms(including NewUI!): Add options to change FramebufferCPUConvert
@hrydgard hrydgard merged commit 18d0d72 into hrydgard:master Jul 20, 2013
@dbz400
Copy link
Contributor

dbz400 commented Jul 21, 2013

We now have vendor flag setup in framebuffer , may be we can somehow enable or disable this option based on vendor flag however i don't pretty sure what would be the matching . As lots of users in other forums keep asking WHEN should this option be ON or OFF .....

@dbz400
Copy link
Contributor

dbz400 commented Jul 21, 2013

On my Nvidia system with GTX690 , CPU ON is around 1/3 faster than OFF . Therefore i suggest to be always ON

  • YS 7

With CPU convert ON , 696 FPS
With CPU convert OFF , 445 FPS

  • God Eat Burst

With CPU convert ON , 876 FPS
With CPU convert OFF , 619 FPS

  • Danganronpa

With CPU convert ON , 1456 FPS
With CPU convert OFF , 1388 FPS

@hrydgard
Copy link
Owner

Thanks for testing. Then we have the right default now :) And can probably get rid of the other path. Although it is a bit surprising.

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