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

Fix PostProcessing shader garbage on screen. #882

Merged
merged 1 commit into from Aug 28, 2014

Conversation

Sonicadvance1
Copy link
Contributor

This fixes issue 7574 where we are sampling from uninitialized texture data which can basically be anything from VRAM.

}
else
{
std::unique_ptr<unsigned char[]> clear_colour(new unsigned char[width * height * 4]());

This comment was marked as off-topic.

This comment was marked as off-topic.

@degasus
Copy link
Member

degasus commented Aug 26, 2014

Why do you check for gl4.4 for such an easy extension? You'll lock out mesa for years :/

Also, I don't see why we should depend on an optional extension for non-performance critical loads. imo it's fine here to just call glClear before the common drawings. We already have a bound framebuffer there. If you want to save some GPU cycles, add a flag if this texture was already cleared.

@Sonicadvance1 Sonicadvance1 force-pushed the fix-pp-blackness branch 4 times, most recently from 7f7ec8d to dd27608 Compare August 26, 2014 13:05
@i418c
Copy link
Contributor

i418c commented Aug 26, 2014

If this fixes 7574, it also fixes 7521 because 7574 is a duplicate of my bug report. I'm happy to have this fixed, though.

@delroth
Copy link
Member

delroth commented Aug 27, 2014

LGTM if it doesn't impact performance too badly (glClear can end up serializing stuff on some drivers?).

@JMC47 care to quickly test in a few games, at both low and high IR?


#include "VideoBackends/OGL/GLExtensions/gl_common.h"

extern PFNGLBINDBUFFERSBASEPROC glBindBuffersBase;

This comment was marked as off-topic.

@degasus
Copy link
Member

degasus commented Aug 27, 2014

I haven't got why you include two unused extension, but otherwise: LGTM

We were generating a texture without ever setting the data to a known value.
This happened on the old code as well, just that PP shaders are receiving some love and people are using it and noticing some of its issues.
@degasus
Copy link
Member

degasus commented Aug 28, 2014

LGTM

Sonicadvance1 added a commit that referenced this pull request Aug 28, 2014
Fix PostProcessing shader garbage on screen.
@Sonicadvance1 Sonicadvance1 merged commit c908a1e into dolphin-emu:master Aug 28, 2014
@Sonicadvance1 Sonicadvance1 deleted the fix-pp-blackness branch August 28, 2014 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants