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

Added missing setActive virtual method to sf::RenderTarget, added set… #1157

Merged
merged 1 commit into from Oct 13, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 20 additions & 0 deletions examples/opengl/OpenGL.cpp
Expand Up @@ -63,6 +63,9 @@ int main()
// mipmapping is purely optional in this example
texture.generateMipmap();

// Make the window the active window for OpenGL calls
window.setActive(true);

// Enable Z-buffer read and write
glEnable(GL_DEPTH_TEST);
glDepthMask(GL_TRUE);
Expand Down Expand Up @@ -141,6 +144,9 @@ int main()
glDisableClientState(GL_NORMAL_ARRAY);
glDisableClientState(GL_COLOR_ARRAY);

// Make the window no longer the active window for OpenGL calls
window.setActive(false);

// Create a clock for measuring the time elapsed
sf::Clock clock;

Expand Down Expand Up @@ -196,14 +202,25 @@ int main()

// Adjust the viewport when the window is resized
if (event.type == sf::Event::Resized)
{
// Make the window the active window for OpenGL calls
window.setActive(true);

glViewport(0, 0, event.size.width, event.size.height);

// Make the window no longer the active window for OpenGL calls
window.setActive(false);
}
}

// Draw the background
window.pushGLStates();
window.draw(background);
window.popGLStates();

// Make the window the active window for OpenGL calls
window.setActive(true);

// Clear the depth buffer
glClear(GL_DEPTH_BUFFER_BIT);

Expand All @@ -222,6 +239,9 @@ int main()
// Draw the cube
glDrawArrays(GL_TRIANGLES, 0, 36);

// Make the window no longer the active window for OpenGL calls
window.setActive(false);

// Draw some text on top of our OpenGL object
window.pushGLStates();
window.draw(text);
Expand Down
36 changes: 22 additions & 14 deletions include/SFML/Graphics/RenderTarget.hpp
Expand Up @@ -255,6 +255,28 @@ class SFML_GRAPHICS_API RenderTarget : NonCopyable
////////////////////////////////////////////////////////////
virtual Vector2u getSize() const = 0;

////////////////////////////////////////////////////////////
/// \brief Activate or deactivate the render target for rendering
///
/// This function makes the render target's context current for
/// future OpenGL rendering operations (so you shouldn't care
/// about it if you're not doing direct OpenGL stuff).
/// A render target's context is active only on the current thread,
/// if you want to make it active on another thread you have
/// to deactivate it on the previous thread first if it was active.
/// Only one context can be current in a thread, so if you
/// want to draw OpenGL geometry to another render target
/// don't forget to activate it again. Activating a render
/// target will automatically deactivate the previously active
/// context (if any).
///
Copy link
Member

Choose a reason for hiding this comment

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

For RenderWindow::setActive it is specified that the previously active window (if any) will get deactivated. Here no such statement being made leaves room for interpretation. The expected behaviour is probably something obvious for someone with good knowledge of OpenGL, but I'd still recommend adding a similar statement here or, if applicable, warn the user that (s)he should make sure the previous RT was deactivated (not sure if that last statement would make sense though).

Copy link
Member

Choose a reason for hiding this comment

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

Regarding multithreading, the documentation for RenderWindow::setActive seems more strong than this one. Should the user be expected to obey the following rule here as well?

if you want to make it active on another thread you have to deactivate it on the previous thread first if it was active.

/// \param active True to activate, false to deactivate
///
/// \return True if operation was successful, false otherwise
///
////////////////////////////////////////////////////////////
virtual bool setActive(bool active = true) = 0;

////////////////////////////////////////////////////////////
/// \brief Save the current OpenGL render states and matrices
///
Expand Down Expand Up @@ -380,20 +402,6 @@ class SFML_GRAPHICS_API RenderTarget : NonCopyable
////////////////////////////////////////////////////////////
void applyShader(const Shader* shader);

////////////////////////////////////////////////////////////
/// \brief Activate the target for rendering
///
/// This function must be implemented by derived classes to make
/// their OpenGL context current; it is called by the base class
/// everytime it's going to use OpenGL calls.
///
/// \param active True to make the target active, false to deactivate it
///
/// \return True if the function succeeded
///
////////////////////////////////////////////////////////////
virtual bool activate(bool active) = 0;

////////////////////////////////////////////////////////////
/// \brief Render states cache
///
Expand Down
13 changes: 0 additions & 13 deletions include/SFML/Graphics/RenderTexture.hpp
Expand Up @@ -204,19 +204,6 @@ class SFML_GRAPHICS_API RenderTexture : public RenderTarget

private:

////////////////////////////////////////////////////////////
/// \brief Activate the target for rendering
///
/// This function is called by the base class
/// everytime it's going to use OpenGL calls.
///
/// \param active True to make the target active, false to deactivate it
///
/// \return True if the function succeeded
///
////////////////////////////////////////////////////////////
virtual bool activate(bool active);

////////////////////////////////////////////////////////////
// Member data
////////////////////////////////////////////////////////////
Expand Down
30 changes: 18 additions & 12 deletions include/SFML/Graphics/RenderWindow.hpp
Expand Up @@ -112,6 +112,24 @@ class SFML_GRAPHICS_API RenderWindow : public Window, public RenderTarget
////////////////////////////////////////////////////////////
virtual Vector2u getSize() const;

////////////////////////////////////////////////////////////
/// \brief Activate or deactivate the window as the current target
/// for OpenGL rendering
///
/// A window is active only on the current thread, if you want to
/// make it active on another thread you have to deactivate it
/// on the previous thread first if it was active.
/// Only one window can be active on a thread at a time, thus
/// the window previously active (if any) automatically gets deactivated.
/// This is not to be confused with requestFocus().
///
/// \param active True to activate, false to deactivate
///
/// \return True if operation was successful, false otherwise
///
////////////////////////////////////////////////////////////
bool setActive(bool active = true);

////////////////////////////////////////////////////////////
/// \brief Copy the current contents of the window to an image
///
Expand Down Expand Up @@ -159,18 +177,6 @@ class SFML_GRAPHICS_API RenderWindow : public Window, public RenderTarget
///
////////////////////////////////////////////////////////////
virtual void onResize();

private:

////////////////////////////////////////////////////////////
/// \brief Activate the target for rendering
///
/// \param active True to make the target active, false to deactivate it
///
/// \return True if the function succeeded
///
////////////////////////////////////////////////////////////
virtual bool activate(bool active);
};

} // namespace sf
Expand Down
10 changes: 5 additions & 5 deletions src/SFML/Graphics/RenderTarget.cpp
Expand Up @@ -98,7 +98,7 @@ RenderTarget::~RenderTarget()
////////////////////////////////////////////////////////////
void RenderTarget::clear(const Color& color)
{
if (activate(true))
if (setActive(true))
{
// Unbind texture to fix RenderTexture preventing clear
applyTexture(NULL);
Expand Down Expand Up @@ -214,7 +214,7 @@ void RenderTarget::draw(const Vertex* vertices, std::size_t vertexCount,
#define GL_QUADS 0
#endif

if (activate(true))
if (setActive(true))
{
// First set the persistent OpenGL states if it's the very first call
if (!m_cache.glStatesSet)
Expand Down Expand Up @@ -304,7 +304,7 @@ void RenderTarget::draw(const Vertex* vertices, std::size_t vertexCount,
////////////////////////////////////////////////////////////
void RenderTarget::pushGLStates()
{
if (activate(true))
if (setActive(true))
{
#ifdef SFML_DEBUG
// make sure that the user didn't leave an unchecked OpenGL error
Expand Down Expand Up @@ -336,7 +336,7 @@ void RenderTarget::pushGLStates()
////////////////////////////////////////////////////////////
void RenderTarget::popGLStates()
{
if (activate(true))
if (setActive(true))
{
glCheck(glMatrixMode(GL_PROJECTION));
glCheck(glPopMatrix());
Expand All @@ -358,7 +358,7 @@ void RenderTarget::resetGLStates()
// Check here to make sure a context change does not happen after activate(true)
bool shaderAvailable = Shader::isAvailable();

if (activate(true))
if (setActive(true))
{
// Make sure that extensions are initialized
priv::ensureExtensionsInit();
Expand Down
7 changes: 0 additions & 7 deletions src/SFML/Graphics/RenderTexture.cpp
Expand Up @@ -156,11 +156,4 @@ const Texture& RenderTexture::getTexture() const
return m_texture;
}


////////////////////////////////////////////////////////////
bool RenderTexture::activate(bool active)
{
return setActive(active);
}

} // namespace sf
8 changes: 4 additions & 4 deletions src/SFML/Graphics/RenderWindow.cpp
Expand Up @@ -62,16 +62,16 @@ RenderWindow::~RenderWindow()


////////////////////////////////////////////////////////////
bool RenderWindow::activate(bool active)
Vector2u RenderWindow::getSize() const
{
return setActive(active);
return Window::getSize();
}


////////////////////////////////////////////////////////////
Vector2u RenderWindow::getSize() const
bool RenderWindow::setActive(bool active)
{
return Window::getSize();
return Window::setActive(active);
}


Expand Down