Skip to content

Conversation

@weichweich
Copy link
Contributor

I added an iterator which can draw triangles. It uses the line iterator, to draw the edges. To fill the triangle I start to draw the edges at the top left corner and fill the space between the points on the edge.

Because the current way of drawing lines not always starts drawing at the start point, I had to reimplement the line drawing algorithm.

There are some issues with my triangle implementation. First issue is that triangles have no stroke width. The second issue is that the triangle iterator returns some points multiple times. The corner point e.g. are return at least twice because they are part of two edges which are drawn separately.

Copy link
Member

@jamwaffles jamwaffles left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I've left a preliminary review, and I'll test the simulator examples when I'm next at a Linux machine. Can you add a Triangle bullet to the Primitives list on the docs homepage please?

If a line has zero length, surely it shouldn't be drawn at all? What's the current behaviour in Embedded Graphics for this? Is there an established/expected behaviour from the wider graphics community?

Don't worry about stroke width - that's something that needs to be fixed for the line primitive as well!

d,
s,
err: d[0] + d[1],
e2: 0,
Copy link
Member

Choose a reason for hiding this comment

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

I think these single letters are from Bresenham's algorithm, no? Can you give d, s and e2 more descriptive names please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be fixed now

pub fn new() -> Self {
Self {
width: 256,
width: 320,
Copy link
Member

Choose a reason for hiding this comment

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

Why does this need to change?

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 added triangles to the stroke example and wanted to make them fit into the window. I didn't realize that this should be done in the example itself. Should be fixed with my last commit.

@weichweich
Copy link
Contributor Author

Thanks for the PR! I've left a preliminary review, and I'll test the simulator examples when I'm next at a Linux machine. Can you add a Triangle bullet to the Primitives list on the docs homepage please?

Done

If a line has zero length, surely it shouldn't be drawn at all? What's the current behaviour in Embedded Graphics for this? Is there an established/expected behaviour from the wider graphics community?

I didn't put much thought in that. I think the behaviour should not be changed. So zero length lines should no longer be drawn.

Don't worry about stroke width - that's something that needs to be fixed for the line primitive as well!

Lines with a stroke width are rectangles. If the triangle solution is stable enough, it could be reused for lines and polygons.

Copy link
Member

@jamwaffles jamwaffles left a comment

Choose a reason for hiding this comment

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

Tested in the simulator. This looks awesome! Thanks for spending the time implementing this feature for embedded_graphics 🙂

@jamwaffles jamwaffles merged commit f2e105d into embedded-graphics:master May 5, 2019
@jamwaffles
Copy link
Member

Released in 0.4.8.

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