Always close fill paths, fix #68 #73
Conversation
|
Please do leave #68 open, updating the screenshot, or open a new issue so we don't forget. |
|
I opened #74. |
There was a problem hiding this comment.
This will certainly fix the svg examples, but a better place to fix it would be in render_ctx in the piet backend, as the implicit closepath is implemented by the other piet backends. I'm ok merging this and having an issue to fix it there, if you'd prefer to get a fix in quickly.
|
You're right, I was concerned as there was no method to mutably close a |
ishitatsuyuki
force-pushed
the
close-path
branch
from
74ff08c to
2957e16
Compare
|
I just pushed the alternative. Edit: rushed a commit message change as well. |
ishitatsuyuki
changed the title
ishitatsuyuki
force-pushed
the
close-path
branch
from
2957e16 to
8037b67
Compare
piet-gpu/src/render_ctx.rs
Outdated
| @@ -303,6 +303,14 @@ impl PietGpuRenderContext { | |||
| } | |||
|
|
|||
| fn encode_path(&mut self, path: impl Iterator<Item = PathEl>, is_fill: bool) { | |||
| if is_fill { | |||
| self.encode_path_inner(path.chain(std::iter::once(PathEl::ClosePath)), is_fill) | |||
There was a problem hiding this comment.
This is also not quite right (and makes me realize a similar issue in the original version) - if there are multiple open subpaths, each one needs an implicit close, not just at the end. Sorry for not communicating this earlier!
There was a problem hiding this comment.
Good catch! Working on this.
ishitatsuyuki
force-pushed
the
close-path
branch
from
8037b67 to
8a499bc
Compare
|
I went with a somewhat weird iterator trick so that it's allocation-free. Hope it's fine. Apparently the remaining cases of glitches are also fixed with the updated logic. I'll put close #74 in the PR description.
|
|
Sweet, I'm glad my desk-checking caught the additional artifacts without having to investigate. Thanks for iterating til we got it right. I was a bit surprised by the iterator trick, but it seems reasonable to me, especially as it doesn't allocate. (I probably would have done it using lower level logic, the way I would write it in C, but this looks clean and likely just as efficient) |
There are still a few minor glitches visible (even after applying #72), so we should revisit them at a later time. For now, I think this is enough for closing the issue. The glitches are primarily around the edge of the crossing-like structures.
Close #68
Close #74