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
bugfix for dirty sprite when using a source rect #899
Conversation
src_py/sprite.py
Outdated
_spr_rect_clip = _spr_rect.clip | ||
for idx in _spr_rect.collidelistall(_update): | ||
# clip | ||
clip = _spr_rect_clip(_update[idx]) | ||
_surf_blit(spr.image, | ||
clip, | ||
(clip[0] - _spr_rect[0], | ||
clip[1] - _spr_rect[1], | ||
(clip[0] - _spr_rect[0] + spr.source_rect[0], |
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.
The default value for source_rect
is None
, so this will crash when a source_rect
is not used.
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.
Thanks for the review and feedback.
You are right, the source_rect can be None. I have considered it now in the second commit.
About the different sizes... it does not make much sense because its the source area of the blit method (not sure what will happen if you have a bigger source rect...... for a smaller source rect I guess you only the that part).
I still wonder if the clipping will work in all cases.
@@ -1099,17 +1099,14 @@ def draw(self, surface, bgd=None): | |||
if spr._visible: | |||
# sprite not dirty; blit only the intersecting part | |||
_spr_rect = spr.rect | |||
if spr.source_rect is not None: |
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.
I think a DirtySprite
's rect
and source_rect
can be different sizes so this code is probably still required to ensure the proper image is drawn.
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.
My understanding is that source_rect
is used to get the correct image. If you look above at lines 1063-1067 you will see that a similar thing is being done there.
The test in #900 still seems to fail |
The test is still failing because this update is removing code (lines 1102-1104 in the original code) that is unrelated to fixing this issue. The removal of those lines breaks current usage and is inconsistent with other parts of the code (lines 1063-1067: The originator did a good job of fixing the original issue, but making an unrelated functionality breaking change (granted this is a minor one) would be best done via a separate update. It would also be useful to include some related tests and update the documentation stating that this functionality has been changed and may break existing code. |
Continuing this PR in a pygame repo branch so we can all collaborate more easily... |
fixes the bug reported by Inigo González inigoini@gmail.com:
[pygame] BUG: sprite refresh when using source_rect
from 2019.03.10