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

Rounded rectangle #81

Closed

Conversation

dennisorlando
Copy link
Contributor

  • created a RoundedRectangle struct, pretty much copy-pasted from Rectangle
    • it doesn't have an "intersect" function
    • it doesn't implement PrimitiveZero
    • it has some additional functions like "as_rectangle" or "from_rectangle" or "radius()"
  • added some quality-of-life getters to Rectangle and RoundedRectangle
    • top(), left(), right(), bottom(),
  • created a draw_rounded_rectangle() function for Graphics2D which uses an 8-vertex polygon and 4 arcs

I don't think I know how to complete the various missing "draw_rounded_*" functions: it would be lovely if someone else knows how to write them :3 (perhaps a new "rounded_rectangle" branch?)

@dennisorlando dennisorlando mentioned this pull request Apr 6, 2023
src/lib.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
+ std::ops::Div<f32, Output = T>
{
/// Returns true if the specified point is inside this rounded rectangle.
/// Note: this is always inclusive, in contrast to the `contains` method
Copy link
Owner

Choose a reason for hiding this comment

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

We should keep the behaviour consistent (this should also be exclusive)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure about how to handle the curves: there isn't a real "bottom" if the corner is a curve. By "exclusive" do you mean that instead of always inclusive it should be always exclusive? In which case it would be different from the Rectangle's contains() anyways

src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
src/shape.rs Outdated Show resolved Hide resolved
@dennisorlando
Copy link
Contributor Author

on it

src/shape.rs Outdated Show resolved Hide resolved
@QuantumBadger
Copy link
Owner

on it

Perfect, thanks again for this! If you have any questions please let me know.

@QuantumBadger
Copy link
Owner

Hey @dennisorlando, thanks for making the changes! Just checking if this is ready for re-review? It looks like there's a merge conflict in web.rs.

@dennisorlando
Copy link
Contributor Author

Hey @dennisorlando, thanks for making the changes! Just checking if this is ready for re-review? It looks like there's a merge conflict in web.rs.

Hey :) I think so (?) there's the exclusivity / inclusivity problem of the "contains" method that is left unsolved; I left it as it is, therefore always inclusive

@dennisorlando
Copy link
Contributor Author

dennisorlando commented Jun 27, 2023

(I made some mess with branches and merges because of inexperience, I made a new branch and created a new pull request)

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

Successfully merging this pull request may close these issues.

2 participants