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

Prevent sf::RectangleShape divisions-by-zero #499

Closed
SuperV1234 opened this Issue Nov 30, 2013 · 4 comments

Comments

Projects
None yet
3 participants
@SuperV1234

SuperV1234 commented Nov 30, 2013

This is sf::RectangleShape's default constructor.

explicit RectangleShape(const Vector2f& size = Vector2f(0, 0));

...and this is sf::Shape::updateTexCoords():

void Shape::updateTexCoords()
{
    for (unsigned int i = 0; i < m_vertices.getVertexCount(); ++i)
    {
        float xratio = (m_vertices[i].position.x - m_insideBounds.left) / m_insideBounds.width;
        float yratio = (m_vertices[i].position.y - m_insideBounds.top) / m_insideBounds.height;
        m_vertices[i].texCoords.x = m_textureRect.left + m_textureRect.width * xratio;
        m_vertices[i].texCoords.y = m_textureRect.top + m_textureRect.height * yratio;
    }
}

m_insideBounds.width and m_insideBounds.height will be 0, causing a division-by-zero error.

Usually, it's not a problem, but:

  • When using <cfenv>'s float bug-catching tools, SFML's updateTexCoords raises an exception. This is very annoying when trying to catch dangerous bugs and could lead to false positives.
  • Division by zero is undefined behavior: address sanitizers such as clang++'s will report errors and terminate program execution.

So, why not:

  • Default-construct sf::RectangleShape with sf::Vector2f(1, 1)?
  • Use assertions to check divisions by zero?
  • Use assertions/clamping to signal/prevent 0 size assignment in SFML shapes?
@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Nov 30, 2013

Member

Changing the default constructor won't fix this, so I guess it would be better to check the bounds or prevent them from becoming exactly 0?

Member

MarioLiebisch commented Nov 30, 2013

Changing the default constructor won't fix this, so I guess it would be better to check the bounds or prevent them from becoming exactly 0?

@SuperV1234

This comment has been minimized.

Show comment
Hide comment
@SuperV1234

SuperV1234 Nov 30, 2013

When I tried sf::RectangleShape rs{Vec2f{1.f, 1.f}}; I got no errors. I assume that changing the default-constructor to initialize the size with {1.f, 1.f} instead of {0.f, 0.f} and then add asserts in the setSize properties would fix the problem?

SuperV1234 commented Nov 30, 2013

When I tried sf::RectangleShape rs{Vec2f{1.f, 1.f}}; I got no errors. I assume that changing the default-constructor to initialize the size with {1.f, 1.f} instead of {0.f, 0.f} and then add asserts in the setSize properties would fix the problem?

@MarioLiebisch

This comment has been minimized.

Show comment
Hide comment
@MarioLiebisch

MarioLiebisch Nov 30, 2013

Member

Yes, but if I create a shape with no width or height (e.g. to fake a line), you're screwed once again. Would be up to debate whether that's something you should deem a no-go or not.

Member

MarioLiebisch commented Nov 30, 2013

Yes, but if I create a shape with no width or height (e.g. to fake a line), you're screwed once again. Would be up to debate whether that's something you should deem a no-go or not.

@ghost ghost assigned LaurentGomila Nov 30, 2013

@LaurentGomila

This comment has been minimized.

Show comment
Hide comment
@LaurentGomila

LaurentGomila Nov 30, 2013

Member

Thanks, it definitely needs to be fixed.

Member

LaurentGomila commented Nov 30, 2013

Thanks, it definitely needs to be fixed.

MarioLiebisch added a commit to MarioLiebisch/SFML that referenced this issue Feb 12, 2014

jcowgill added a commit to jcowgill/SFML that referenced this issue Sep 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment