-
Notifications
You must be signed in to change notification settings - Fork 10
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
Bugs/fix pygame smoothscale crash #59
Conversation
As reported on irc. Smoothscale handled the non-transform case incorrectly when the pitch of the destination surface was not the same as the the original surface (which happens when dealing with subsurfaces as the source, for example). This changes the logic to copy the image a row at a time to avoid that issue (which matches pygame's logic).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left two minor comments, but the code looks good.
@@ -13,13 +13,37 @@ def _make_object(): | |||
return obj | |||
|
|||
|
|||
def test_subsurface_smoothscale(surface): | |||
"""Simple integer scaling tests""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this have a different docstring to test_int_smoothscale?
@@ -241,11 +241,16 @@ def smoothscale(surface, size, dest_surface=None): | |||
with locked(new_surf): | |||
with locked(c_surf): | |||
if c_surf.w == width and c_surf.h == height: | |||
pitch = c_surf.pitch | |||
c_pitch = c_surf.pitch | |||
n_pitch = new_surf.pitch | |||
# Trivial case |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should rename this case since it's no longer that trivial?
As reported on irc, smoothscale can cause a crash in certain circumstances.
This fixes the obvious error and ads a conformance test for that case to ensure we're matching pygame's behaviour.
Similiar issues probably lurk in other places in pygame.transform - we should examine that closely in the not too distant future.