Revert change to Drawable::draw function signature

This change was made in 359fe90 due to recommendations from tooling.
On its face this change makes sense since it removes a copy that
isn't always necessary. In practice it caused ergonomic issues due
to now being forced to make a copy of the render states when needed.

The performance gains of eliding this copy are unsubstantiated. We
have not done any profiling to measure its impact. For lack of such
measurements I'd rather err on the side of improved user experience.
If future benchmarks prove this copy is rather expensive then we
can reconsider removing it with that evidence in mind.
This commit is contained in:
Chris Thrasher 2024-04-16 10:41:43 -06:00 committed by Lukas Dürrenberger
parent d6e1961112
commit 4315c3d290
15 changed files with 54 additions and 68 deletions

View File

@ -38,7 +38,7 @@ public:
onUpdate(time, x, y);
}
void draw(sf::RenderTarget& target, const sf::RenderStates& states) const override
void draw(sf::RenderTarget& target, sf::RenderStates states) const override
{
if (m_isLoaded)
{
@ -66,9 +66,9 @@ protected:
private:
// Virtual functions to be implemented in derived effects
virtual bool onLoad() = 0;
virtual void onUpdate(float time, float x, float y) = 0;
virtual void onDraw(sf::RenderTarget& target, const sf::RenderStates& states) const = 0;
virtual bool onLoad() = 0;
virtual void onUpdate(float time, float x, float y) = 0;
virtual void onDraw(sf::RenderTarget& target, sf::RenderStates states) const = 0;
std::string m_name;
bool m_isLoaded{};

View File

@ -51,11 +51,10 @@ public:
m_shader.setUniform("pixel_threshold", (x + y) / 30);
}
void onDraw(sf::RenderTarget& target, const sf::RenderStates& states) const override
void onDraw(sf::RenderTarget& target, sf::RenderStates states) const override
{
sf::RenderStates statesCopy(states);
statesCopy.shader = &m_shader;
target.draw(*m_sprite, statesCopy);
states.shader = &m_shader;
target.draw(*m_sprite, states);
}
private:
@ -111,11 +110,10 @@ public:
m_shader.setUniform("blur_radius", (x + y) * 0.008f);
}
void onDraw(sf::RenderTarget& target, const sf::RenderStates& states) const override
void onDraw(sf::RenderTarget& target, sf::RenderStates states) const override
{
sf::RenderStates statesCopy(states);
statesCopy.shader = &m_shader;
target.draw(m_text, statesCopy);
states.shader = &m_shader;
target.draw(m_text, states);
}
private:
@ -165,11 +163,10 @@ public:
m_shader.setUniform("blink_alpha", 0.5f + std::cos(time * 3) * 0.25f);
}
void onDraw(sf::RenderTarget& target, const sf::RenderStates& states) const override
void onDraw(sf::RenderTarget& target, sf::RenderStates states) const override
{
sf::RenderStates statesCopy(states);
statesCopy.shader = &m_shader;
target.draw(m_points, statesCopy);
states.shader = &m_shader;
target.draw(m_points, states);
}
private:
@ -245,11 +242,10 @@ public:
m_surface.display();
}
void onDraw(sf::RenderTarget& target, const sf::RenderStates& states) const override
void onDraw(sf::RenderTarget& target, sf::RenderStates states) const override
{
sf::RenderStates statesCopy(states);
statesCopy.shader = &m_shader;
target.draw(sf::Sprite(m_surface.getTexture()), statesCopy);
states.shader = &m_shader;
target.draw(sf::Sprite(m_surface.getTexture()), states);
}
private:
@ -318,17 +314,15 @@ public:
m_shader.setUniform("size", sf::Vector2f(size, size));
}
void onDraw(sf::RenderTarget& target, const sf::RenderStates& states) const override
void onDraw(sf::RenderTarget& target, sf::RenderStates states) const override
{
sf::RenderStates statesCopy(states);
// Prepare the render state
statesCopy.shader = &m_shader;
statesCopy.texture = &m_logoTexture;
statesCopy.transform = m_transform;
states.shader = &m_shader;
states.texture = &m_logoTexture;
states.transform = m_transform;
// Draw the point cloud
target.draw(m_pointCloud, statesCopy);
target.draw(m_pointCloud, states);
}
private:

View File

@ -63,7 +63,7 @@ protected:
/// \param states Current render states
///
////////////////////////////////////////////////////////////
virtual void draw(RenderTarget& target, const RenderStates& states) const = 0;
virtual void draw(RenderTarget& target, RenderStates states) const = 0;
};
} // namespace sf
@ -94,15 +94,14 @@ protected:
///
/// private:
///
/// void draw(sf::RenderTarget& target, const sf::RenderStates& states) const override
/// void draw(sf::RenderTarget& target, sf::RenderStates states) const override
/// {
/// // You can draw other high-level objects
/// target.draw(m_sprite, states);
///
/// // ... or use the low-level API
/// sf::RenderStates statesCopy(states);
/// statesCopy.texture = &m_texture;
/// target.draw(m_vertices, statesCopy);
/// states.texture = &m_texture;
/// target.draw(m_vertices, states);
///
/// // ... or draw with OpenGL directly
/// glBegin(GL_TRIANGLES);

View File

@ -281,7 +281,7 @@ private:
/// \param states Current render states
///
////////////////////////////////////////////////////////////
void draw(RenderTarget& target, const RenderStates& states) const override;
void draw(RenderTarget& target, RenderStates states) const override;
////////////////////////////////////////////////////////////
/// \brief Update the fill vertices' color

View File

@ -208,7 +208,7 @@ private:
/// \param states Current render states
///
////////////////////////////////////////////////////////////
void draw(RenderTarget& target, const RenderStates& states) const override;
void draw(RenderTarget& target, RenderStates states) const override;
////////////////////////////////////////////////////////////
/// \brief Update the vertices' positions

View File

@ -398,7 +398,7 @@ private:
/// \param states Current render states
///
////////////////////////////////////////////////////////////
void draw(RenderTarget& target, const RenderStates& states) const override;
void draw(RenderTarget& target, RenderStates states) const override;
////////////////////////////////////////////////////////////
/// \brief Make sure the text's geometry is updated

View File

@ -288,11 +288,10 @@ private:
/// \code
/// class MyEntity : public sf::Transformable, public sf::Drawable
/// {
/// void draw(sf::RenderTarget& target, const sf::RenderStates& states) const override
/// void draw(sf::RenderTarget& target, sf::RenderStates states) const override
/// {
/// sf::RenderStates statesCopy(states);
/// statesCopy.transform *= getTransform();
/// target.draw(..., statesCopy);
/// states.transform *= getTransform();
/// target.draw(..., states);
/// }
/// };
///

View File

@ -183,7 +183,7 @@ private:
/// \param states Current render states
///
////////////////////////////////////////////////////////////
void draw(RenderTarget& target, const RenderStates& states) const override;
void draw(RenderTarget& target, RenderStates states) const override;
////////////////////////////////////////////////////////////
// Member data

View File

@ -329,7 +329,7 @@ private:
/// \param states Current render states
///
////////////////////////////////////////////////////////////
void draw(RenderTarget& target, const RenderStates& states) const override;
void draw(RenderTarget& target, RenderStates states) const override;
////////////////////////////////////////////////////////////
// Member data

View File

@ -234,22 +234,20 @@ void Shape::update()
////////////////////////////////////////////////////////////
void Shape::draw(RenderTarget& target, const RenderStates& states) const
void Shape::draw(RenderTarget& target, RenderStates states) const
{
RenderStates statesCopy(states);
statesCopy.transform *= getTransform();
statesCopy.coordinateType = CoordinateType::Pixels;
states.transform *= getTransform();
states.coordinateType = CoordinateType::Pixels;
// Render the inside
statesCopy.texture = m_texture;
target.draw(m_vertices, statesCopy);
states.texture = m_texture;
target.draw(m_vertices, states);
// Render the outline
if (m_outlineThickness != 0)
{
statesCopy.texture = nullptr;
target.draw(m_outlineVertices, statesCopy);
states.texture = nullptr;
target.draw(m_outlineVertices, states);
}
}

View File

@ -126,14 +126,12 @@ FloatRect Sprite::getGlobalBounds() const
////////////////////////////////////////////////////////////
void Sprite::draw(RenderTarget& target, const RenderStates& states) const
void Sprite::draw(RenderTarget& target, RenderStates states) const
{
RenderStates statesCopy(states);
statesCopy.transform *= getTransform();
statesCopy.texture = m_texture;
statesCopy.coordinateType = CoordinateType::Pixels;
target.draw(m_vertices.data(), m_vertices.size(), PrimitiveType::TriangleStrip, statesCopy);
states.transform *= getTransform();
states.texture = m_texture;
states.coordinateType = CoordinateType::Pixels;
target.draw(m_vertices.data(), m_vertices.size(), PrimitiveType::TriangleStrip, states);
}

View File

@ -339,21 +339,19 @@ FloatRect Text::getGlobalBounds() const
////////////////////////////////////////////////////////////
void Text::draw(RenderTarget& target, const RenderStates& states) const
void Text::draw(RenderTarget& target, RenderStates states) const
{
ensureGeometryUpdate();
RenderStates statesCopy(states);
statesCopy.transform *= getTransform();
statesCopy.texture = &m_font->getTexture(m_characterSize);
statesCopy.coordinateType = CoordinateType::Pixels;
states.transform *= getTransform();
states.texture = &m_font->getTexture(m_characterSize);
states.coordinateType = CoordinateType::Pixels;
// Only draw the outline if there is something to draw
if (m_outlineThickness != 0)
target.draw(m_outlineVertices, statesCopy);
target.draw(m_outlineVertices, states);
target.draw(m_vertices, statesCopy);
target.draw(m_vertices, states);
}

View File

@ -135,7 +135,7 @@ FloatRect VertexArray::getBounds() const
////////////////////////////////////////////////////////////
void VertexArray::draw(RenderTarget& target, const RenderStates& states) const
void VertexArray::draw(RenderTarget& target, RenderStates states) const
{
if (!m_vertices.empty())
target.draw(m_vertices.data(), m_vertices.size(), m_primitiveType, states);

View File

@ -348,7 +348,7 @@ bool VertexBuffer::isAvailable()
////////////////////////////////////////////////////////////
void VertexBuffer::draw(RenderTarget& target, const RenderStates& states) const
void VertexBuffer::draw(RenderTarget& target, RenderStates states) const
{
if (m_buffer && m_size)
target.draw(*this, 0, m_size, states);

View File

@ -16,7 +16,7 @@ public:
}
private:
void draw(sf::RenderTarget&, const sf::RenderStates&) const override
void draw(sf::RenderTarget&, sf::RenderStates) const override
{
++m_callCount;
}