Skip to content

Remove aaline width for 2.5.2 #3182

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Remove aaline width for 2.5.2 #3182

wants to merge 1 commit into from

Conversation

mzivic7
Copy link
Contributor

@mzivic7 mzivic7 commented Oct 17, 2024

Width argument for draw.aaline is not yet ready (does not support float values in coordinates) and will have different implementation in future versions.
This PR disables width argument for draw.aaline and removes it from docs/tests/examples.

@mzivic7 mzivic7 requested a review from a team as a code owner October 17, 2024 19:34
@mzivic7 mzivic7 changed the title Removed aaline width for 2.5.2 Remove aaline width for 2.5.2 Oct 17, 2024
@bilhox bilhox added this to the 2.5.2 milestone Oct 17, 2024
@yunline yunline added the draw pygame.draw label Oct 18, 2024
@ankith26
Copy link
Member

A couple of questions.

  1. With width=1 are you sure the issue you are talking about does not happen?
  2. If yes, what kinds of width are problematic?
  3. If no, then maybe the whole PR should be reverted instead?

@mzivic7
Copy link
Contributor Author

mzivic7 commented Oct 19, 2024 •
edited
Loading

With width=1 it always uses draw_aaline algorithm (Xiaolin Wu) which allows drawing at float coordinates.
My new implementation does not use Bresenham's algorithm at all (used by draw_line_width), so line_width_corners is unnecessary (used to get corners of draw_line_width) and I would remove it in new PR.
So almost all current code would be replaced in new PR anyway. That's why I think it would be better to revert whole #3140.

bilhox
bilhox approved these changes Oct 19, 2024
View reviewed changes
Copy link
Contributor

@bilhox bilhox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM !

@mzivic7 mzivic7 mentioned this pull request Oct 23, 2024
Merged
@ankith26 ankith26 mentioned this pull request Oct 23, 2024
Merged
@mzivic7
Copy link
Contributor Author

mzivic7 commented Oct 26, 2024

#3140 is completely reverted in #3192 instead.

@mzivic7 mzivic7 closed this Oct 26, 2024
@mzivic7 mzivic7 deleted the remove_aaline_width branch October 26, 2024 14:49
@ankith26 ankith26 removed this from the 2.5.2 milestone Oct 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bilhox bilhox bilhox approved these changes

Assignees
No one assigned
Labels
draw pygame.draw
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mzivic7 @ankith26 @bilhox @yunline