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

ogl: use a sample-able texture as EFB instead of renderbuffer #331

Merged
merged 5 commits into from May 19, 2014

Conversation

degasus
Copy link
Member

@degasus degasus commented May 2, 2014

Renderbuffers are used to be faster for rendering, but on newer gpu generations, they internally doesn't care about renderbuffers vs textures any more. So there is no need for us to use renderbuffers which aren't as flexible as renderbuffers aren't usable as textures.
As example, interpretate format change emulation now directly sample the old msaa efb without resolving. This won't speed up (it's resolv vs sample shading), but we won't loose the MSAA sample information any more.
As CSAA is a legacy NV feature which is only compatible with renderbuffers, this feature is also removed. Also GLES3.0 doesn't support this kind of multisampled textures, but 3.1 does :D

@delroth
Copy link
Member

delroth commented May 2, 2014

Does that speed up EFB to Texture by not requiring a rendertarget switch?

@degasus
Copy link
Member Author

degasus commented May 2, 2014

@delroth: It will speed up EFB2Tex with MSAA when we avoid the resolving. But this isn't implemented now.

@neobrain
Copy link
Member

neobrain commented May 2, 2014

Please run a spell check over the comments.

if (m_msaaSamples <= 1)
{
// EFB targets will be textures in non-MSAA mode.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@degasus
Copy link
Member Author

degasus commented May 2, 2014

updated

@degasus
Copy link
Member Author

degasus commented May 2, 2014

@Sonicadvance1 do you know what's the issue with std::to_string on android?

@Sonicadvance1
Copy link
Contributor

As evident by this stackoverflow question http://stackoverflow.com/questions/17950814/how-to-use-stdstoul-and-stdstoull-in-android
The problem is that Android has broken C99 support, so the functions are explicitly disabled from bits/basic_string.h

@lioncash
Copy link
Member

lioncash commented May 2, 2014

Yep, I came across this problem a while ago when I tried to use newer C++11 string functions in the core as well. It's super fantastic that it's not documented anywhere (as far as I can tell), so people can actually know about this issue, and that the attitude of something not working was "lol lets just disable it" instead of actually fixing the underlying issue.

Sonicadvance1 added a commit to Sonicadvance1/dolphin that referenced this pull request May 2, 2014
Android has broken C99 support in it's NDK. This hasn't been too much of an issue since most uses of the disabled features have been in the WX GUI.
This has become a bit more of an issue since we can't use std::to_string in common code due to it being disabled in basic_string.h due to the
_GLIBCXX_USE_C99 define not being set.

This StdBasicString header adds the disabled code block back in with the few functions removed that we can't use without C99.
Once/If the Android NDK fully supports C99, this header can be removed.
This PR is required for PR dolphin-emu#331 as it is trying to use to_string in Source/Core/VideoBackends/OGL/FramebufferManager.cpp
Sonicadvance1 added a commit to Sonicadvance1/dolphin that referenced this pull request May 2, 2014
Android has broken C99 support in it's NDK. This hasn't been too much of an issue since most uses of the disabled features have been in the WX GUI.
This has become a bit more of an issue since we can't use std::to_string in common code due to it being disabled in basic_string.h due to the
_GLIBCXX_USE_C99 define not being set.

This StdBasicString header adds the disabled code block back in with the few functions removed that we can't use without C99.
Once/If the Android NDK fully supports C99, this header can be removed.
This PR is required for PR dolphin-emu#331 as it is trying to use to_string in Source/Core/VideoBackends/OGL/FramebufferManager.cpp
Sonicadvance1 added a commit to Sonicadvance1/dolphin that referenced this pull request May 2, 2014
Android has broken C99 support in it's NDK. This hasn't been too much of an issue since most uses of the disabled features have been in the WX GUI.
This has become a bit more of an issue since we can't use std::to_string in common code due to it being disabled in basic_string.h due to the
_GLIBCXX_USE_C99 define not being set.

This StdBasicString header adds the disabled code block back in with the few functions removed that we can't use without C99.
Once/If the Android NDK fully supports C99, this header can be removed.
This PR is required for PR dolphin-emu#331 as it is trying to use to_string in Source/Core/VideoBackends/OGL/FramebufferManager.cpp
Sonicadvance1 added a commit to Sonicadvance1/dolphin that referenced this pull request May 3, 2014
Android has broken C99 support in it's NDK. This hasn't been too much of an issue since most uses of the disabled features have been in the WX GUI.
This has become a bit more of an issue since we can't use std::to_string in common code due to it being disabled in basic_string.h due to the
_GLIBCXX_USE_C99 define not being set.

This StdBasicString header adds the disabled code block back in with the few functions removed that we can't use without C99.
Once/If the Android NDK fully supports C99, this header can be removed.
This PR is required for PR dolphin-emu#331 as it is trying to use to_string in Source/Core/VideoBackends/OGL/FramebufferManager.cpp
GLuint FramebufferManager::m_xfbFramebuffer;
GLuint FramebufferManager::m_efbColor;
GLuint FramebufferManager::m_efbDepth;
GLuint FramebufferManager::m_efbColorSwap; // for hot swap on reinterpret pixel format

This comment was marked as off-topic.

@neobrain
Copy link
Member

neobrain commented May 5, 2014

LGTM once that last comment is fixed. Also, either squash your the suggested fixes into the original commits or change the commit message to something more descriptive than "Fix neobrain's suggestions" (which really is sort of useless when looking through git logs).

@Sonicadvance1
Copy link
Contributor

Once the Android support gets merged in and this gets changed to handle it then it will LGTM.

{
// msaa without sample shading: calculate the mean value of the pixel
std::stringstream samples;
samples << m_msaaSamples;

This comment was marked as off-topic.

neobrain added a commit that referenced this pull request May 19, 2014
OpenGL: Use a sample-able texture as EFB instead of renderbuffer.
@neobrain neobrain merged commit ada6434 into dolphin-emu:master May 19, 2014
@degasus degasus deleted the kill_csaa branch May 20, 2014 03:53
@wisteso
Copy link

wisteso commented Mar 29, 2015

This change seems to cause severe slow down when users have 4x SSAA enabled. Maybe something to document in the GUI? https://forums.dolphin-emu.org/Thread-pr-331-causes-severe-slowdown-with-ssbb-4x-ssaa

@magumagu
Copy link
Contributor

Please file a bug if this caused a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
7 participants