pygame-community / pygame-ce

🐍🎮 pygame - Community Edition is a FOSS Python library for multimedia applications (like games). Built on top of the excellent SDL library.
https://pyga.me
929 stars 154 forks source link

FRect - incorrect positioning for negative values #2409

Open rethanon opened 1 year ago

rethanon commented 1 year ago

Environment:

Current behavior:

When FRects are positioned off screen using negative values (i.e. off the top of the screen (-y), or left of the screen(-x)) they are 1 pixel out from where they should be. I think this is because pygame-ce is either dealing with floats in the FRect as ints, or is calculating floor for positive values and ceil for negative values (I assume it's int conversion but might be wrong).

Expected behavior:

FRects should use the same method of calculation for both negative and positive values, I would expect calculating floor values would be the correct way.

Steps to reproduce:

  1. create a program where a surface uses FRect for positioning
  2. animate/move the surface off screen (either the top or the left) in small amounts i.e. not integers
  3. observe the positioning which stutters or pauses while it transitions from positive to negative values (caused by values ranging from 0.9 to -0.9 recurring all being truncated/converted to 0)

Test code

You should be able to notice the circle appears to pause when it reaches the top of the screen, and this is because there it spends twice as long at position 0 due to the int conversion for values between 0.9999 and -0.9999.

import time

import pygame

pygame.init()

timer = pygame.time.Clock()

WIDTH = 800
HEIGHT = 600

screen = pygame.display.set_mode((300, 200), pygame.SCALED)
main_font = pygame.font.SysFont(None, 24)

circle_surf = pygame.Surface((102, 102), pygame.SRCALPHA)
pygame.draw.circle(circle_surf, "red", (51, 51), 51, 1)
circle_surf_frect = pygame.FRect(circle_surf.get_rect(topleft=(9, 9)))

frame_start_time = time.time()
running = True
while running:
    delta_time = time.time() - frame_start_time
    frame_start_time = time.time()
    for event in pygame.event.get():
        if event.type == pygame.QUIT:
            running = False

    frect_y_render = main_font.render(
        f"FRect.x:  {round(circle_surf_frect.y, 4)}", False, "yellow"
    )
    y_coord_render = main_font.render(
        f"Pixel x:  {int(circle_surf_frect.y)}", False, "yellow"
    )

    screen.fill("midnightblue")
    screen.blit(circle_surf, circle_surf_frect)
    screen.blit(frect_y_render, (120, 20))
    screen.blit(y_coord_render, (120, 40))

    circle_surf_frect.y -= 2 * delta_time
    pygame.display.flip()
    timer.tick(60)

Other information In the rect_test.py file I think the test_clipline_floats test would fail if a test was performed that included negative x and y values as it uses floor to check the values.

Starbuck5 commented 1 year ago

We were talking about this a lot on discord yesterday since it was brought up by Clear.

FRects should use the same method of calculation for both negative and positive values

Just to add some clarification @rethanon, they are actually doing the same operation-- the problem is that it's a truncate operation. If you're curious, this is the code: https://github.com/pygame-community/pygame-ce/blob/681f5b5e4e91f813f806c13b1183a7a1e18450d6/src_c/base.c#L483-L489

The problem is that the function between blit and that is pg_TwoIntsFromObj, which is used in many places in the codebase where we haven't thought about rounding behavior. So maybe we need a pg_TwoIntsFromObj_Floor for a few select places we really want to think through the reasoning for.

rethanon commented 1 year ago

Thanks, my comment about using the same calculation could have been worded better, I assumed it was int conversion that was causing the issue which as you rightly point out is the same operation.

It becomes noticeable when you have a game where sprites or objects move off screen - in my case it was a scrolling credits screen like you see after a movie has ended, and I could clearly notice a brief pause just before objects moved past the top of the screen. I resolved it by manually calculating the y position using math.floor so wasn't much of a problem to resolve but it kind of defeated the purpose of using an FRect instead of a Rect.