Skip to content
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

Added border_radius keyword for draw.rect and drawing of one circle quadrant, fixed bounding rects #1494

Merged
merged 9 commits into from Nov 14, 2019

Conversation

MightyJosip
Copy link
Contributor

@MightyJosip MightyJosip commented Nov 9, 2019

This should fix #895, #1120, #1122, #1153, #1212, #1241, #1305 and #1306

@MightyJosip MightyJosip changed the title Added RoundRect, drawing of one circle quadrant fixed circle bounding rect Added draw.round_rect, drawing of one circle quadrant fixed circle bounding rect Nov 9, 2019
@illume
Copy link
Member

illume commented Nov 9, 2019

Thanks @MightyJosip :)

@MightyJosip MightyJosip changed the title Added draw.round_rect, drawing of one circle quadrant fixed circle bounding rect Added border_radius keyword for draw.rect and drawing of one circle quadrant, fixed circle bounding rect Nov 9, 2019
src_c/doc/draw_doc.h Outdated Show resolved Hide resolved
test/draw_test.py Outdated Show resolved Hide resolved
@MightyJosip
Copy link
Contributor Author

@charlesej since you did draw documentation, can you just check if I wrote it right, this is my first time I was writing docs and using rst files

Copy link
Contributor

@charlesej charlesej left a comment

Choose a reason for hiding this comment

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

Your documentation looks good. I made a few minor comments.

One final note. When you are done making your changes to draw.rst, remember to run setup on the docs (python setup.py docs) to generate/update the draw_doc.h file.

docs/reST/ref/draw.rst Outdated Show resolved Hide resolved
docs/reST/ref/draw.rst Outdated Show resolved Hide resolved
@charlesej charlesej added the draw pygame.draw label Nov 9, 2019
@illume
Copy link
Member

illume commented Nov 9, 2019

About quadrant=0-4... Is it confusing? I'm not sure if it's better than 0-3, and a default None for quadrant?

If kwargs were used (top_left, top_right, bottom_left, bottom_right) each defaulting to True, then it would make it possible from the API that different quadrants could be True and drawn at once. I'm not sure if that's desirable?

@illume
Copy link
Member

illume commented Nov 10, 2019

@MightyJosip here's an example of usage you asked for.

Edit: here is how it looks using the quadrant argument.

    pygame.draw.circle(screen, BLUE, [250, 250], 40, 0, quadrant=1)
    pygame.draw.circle(screen, RED, [250, 250], 40, 30, 2, quadrant=2)
    pygame.draw.circle(screen, GREEN, [250, 250], 40, 20, quadrant=3)
    pygame.draw.circle(screen, BLACK, [250, 250], 40, 10, quadrant=4)

    # draw top left, bottom left slices of the circle.
    pygame.draw.circle(screen, BLACK, [250, 250], 40, 10, quadrant=1)
    pygame.draw.circle(screen, BLACK, [250, 250], 40, 10, quadrant=4)

Here is how it looks using the top, left, bottom, right args.

    pygame.draw.circle(screen, BLUE, [250, 250], 40, 0, top_left=True)
    pygame.draw.circle(screen, RED, [250, 250], 40, 30, 2, top_right=True)
    pygame.draw.circle(screen, GREEN, [250, 250], 40, 20, bottom_right=True)
    pygame.draw.circle(screen, BLACK, [250, 250], 40, 10, bottom_left=True)

    # draw top left, bottom left slices of the circle.
    pygame.draw.circle(screen, BLACK, [250, 250], 40, 10, bottom_left=True, top_left=True)

They should probably default to None, so that it is not required to call each one with False?
top_left=None, top_right=None, bottom_left=None, bottom_right=None

The only problem is that bottom_left, top_left doesn't read very well. It does not saying anything about what top_left refers to (drawing only the top_left quarter of the circle). Here's some other possible names...

  • draw_top_left=True
  • quad_tl=True
  • quad_top_left=True
  • ...

@illume
Copy link
Member

illume commented Nov 10, 2019

Regarding:

it should be pretty trivial for me to make all corners different

Do you mean for the rounded corners?

In css they do let each corner be controlled with separate rules:

border-top-left-radius: 1px
border-top-right-radius: 2px
border-bottom-left-radius: 3px
border-bottom-right-radius: 4px

This way they have the default border-radius, and then each other rule can override that.
So it's possible to do (all corners 5px except top left at 10px):

border-radius: 5px
border-top-left-radius: 10px

@illume
Copy link
Member

illume commented Nov 11, 2019

I updated the example usage comment above with the quadrant=, and top_left= styles to make comparison easier.

@MightyJosip
Copy link
Contributor Author

With latest commit #1241, #1212, #1305, #1306, #1153 and #895 should also be fixed

@MightyJosip MightyJosip changed the title Added border_radius keyword for draw.rect and drawing of one circle quadrant, fixed circle bounding rect Added border_radius keyword for draw.rect and drawing of one circle quadrant, fixed bounding rects Nov 13, 2019
@illume illume changed the base branch from master to border_radius November 14, 2019 06:53
Copy link
Member

@illume illume left a comment

Choose a reason for hiding this comment

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

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draw pygame.draw
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants