Skip to content

Commit

Permalink
Make sure texture unit 0 is active when reseting RenderTarget states (#…
Browse files Browse the repository at this point in the history 
…523), fix RenderTarget not clearing when a texture used as a RenderTexture color attachment is left bound in a different context (http://en.sfml-dev.org/forums/index.php?topic=9350.0).
  • Loading branch information
@binary1248
binary1248 committed Apr 28, 2014
1 parent cdf32a7 commit 24c364e
Showing 1 changed file with 9 additions and 0 deletions.
9 changes: 9 additions & 0 deletions src/SFML/Graphics/RenderTarget.cpp
View file
Expand Up @@ -93,6 +93,9 @@ void RenderTarget::clear(const Color& color)
{
if (activate(true))
{
// Unbind texture to fix RenderTexture preventing clear
applyTexture(NULL);

glCheck(glClearColor(color.r / 255.f, color.g / 255.f, color.b / 255.f, color.a / 255.f));
glCheck(glClear(GL_COLOR_BUFFER_BIT));
}
Expand Down Expand Up @@ -363,7 +366,13 @@ void RenderTarget::resetGLStates()
applyTransform(Transform::Identity);
applyTexture(NULL);
if (Shader::isAvailable())
{
applyShader(NULL);

// Make sure that the texture unit which is active is the number 0
glCheck(glClientActiveTextureARB(GL_TEXTURE0_ARB));
glCheck(glActiveTextureARB(GL_TEXTURE0_ARB));
}
m_cache.useVertexCache = false;

// Set the default view
Expand Down

9 comments on commit 24c364e

@LaurentGomila
Copy link
Member

Choose a reason for hiding this comment

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

This will not compile on GL ES 1.1 platforms, you need to use an alias for extensions and map it to the correct symbol for GL ES (hope that it's available). This was not a problem before because the implementation of sf::Shader is empty on these platforms ;)

@eXpl0it3r
Copy link
Member

Choose a reason for hiding this comment

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

I guess what Laurent said. I can only confirm that it fixes the issue on my Windows machine with the AMD driver 14.1.

@LaurentGomila
Copy link
Member

Choose a reason for hiding this comment

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

In case there's no equivalent in GL ES 1.1, you can #ifdef the corresponding code. It will never be executed anyway, because Shader::isAvailable returns false.

@binary1248
Copy link
Member Author

Choose a reason for hiding this comment

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

In the case that I disable the code with an #ifdef, user code that switches texture units in GL ES 1.1 will break SFML as the reporter described. I think the better solution is to add it to GLExtensions. Is there any convention that should be followed? Or can I simply add an entry to the end of both blocks?

@LaurentGomila
Copy link
Member

Choose a reason for hiding this comment

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

In the case that I disable the code with an #ifdef, user code that switches texture units in GL ES 1.1 will break SFML as the reporter described.

As I said, this code will never be called in GL ES implementations. Shader::isAvailable returns false. But why are these two calls tied to shaders anyway?

Is there any convention that should be followed? Or can I simply add an entry to the end of both blocks?

Yes, you can just add an entry to the end. Just make it look like the other entries ;)

@binary1248
Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to add: it would have to be moved outside of the Shader::isAvailable() check as well, since GL ES 1.1 doesn't support shaders but does support multitexturing.

@binary1248
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in ad01d1d. This might be a good time to squash the commits ;). I have no idea what would happen if I pushed an amended commit. Maybe these comments would be lost? :P

@MarioLiebisch
Copy link
Member

Choose a reason for hiding this comment

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

No, they'd stay as they are. Only comments added to removed changes as line comments will be sorted as "previous commits". Check pull request #580 by me. I amended the commit, so four (line) comments got "dropped".

@binary1248
Copy link
Member Author

Choose a reason for hiding this comment

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

The squashed commit can be found at 132f476.

Please sign in to comment.