-
Notifications
You must be signed in to change notification settings - Fork 7
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
refactor: Screen API #51
Conversation
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.
LGTM other than a few things.
pub fn from_dimensions(origin: impl Into<Point2<i16>>, width: u16, height: u16) -> Self { | ||
let origin = origin.into(); | ||
Self { | ||
x0: start_x, | ||
y0: start_y + 0x20, | ||
x1: end_x, | ||
y1: end_y + 0x20, | ||
start: origin, | ||
end: Point2 { | ||
x: origin.x + (width as i16), | ||
y: origin.y + (height as i16), | ||
}, |
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.
It could be convenient to provide an API for creating a rectangle centered at a point.
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 that these changes may be an accident.
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.
LGTM
Describe the changes this PR makes. Why should it be merged?
line
positioning mode, since that has some inconsistent behavior.mint::Point2
, instead adding our ownPoint2
struct with an actually practical implementation.geometry
module.draw
pixel is now dead. Instead we blanket-implFill
for all types that can be converted into aPoint2
.Additional Context
Closes #44. Needs hardware testing.