-
Notifications
You must be signed in to change notification settings - Fork 71
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
Don't skip first dash in dash pattern #353
Conversation
2ee61dd
to
e0d7916
Compare
This change currently breaks for non-zero dash offsets. Edit: it is ready for review now. |
@dominikh Could you please rebase this on top of current |
Before this change, the dash iterator's initial state (before accounting for the dash offset) was "processing first dash, with remaining length of zero", which would cause the iterator to immediately jump to the next component in the dash pattern, effectively skipping the first dash and starting with a gap.
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.
Yes, the logic was wrong and this seems to be a valid fix. Thanks!
Related to this is the report in Dashed Line misbehaves when used with dash-offset. I think it's worth re-checking the behavior.
Reading GraphiteEditor/Graphite#1875, which is the tracking issue where the Graphite behavior was reported, I think this takes care of the sign negation, but it's possible there's another issue. Please file another bug against either kurbo or Vello if so.
Thanks for getting to this in your review queue! Once this lands and it's pulled in by the latest Vello master commit, we can try it out in Graphite to confirm the behavior matches the SVG spec. I'll report back in that Zulip thread if it's working, or file an issue here again if it's not. |
This updates Peniko to [0.2.0](https://github.com/linebender/peniko/releases/tag/v0.2.0). The main impact this would have on users is being able to set `alpha` on images, but that is currently broken because of #692. This also updates to Kurbo [0.11.1](https://github.com/linebender/kurbo/releases/tag/v0.11.1), which of note includes linebender/kurbo#353
Before this change, the dash iterator's initial state (before accounting
for the dash offset) was "processing first dash, with remaining length
of zero", which would cause the iterator to immediately jump to the next
component in the dash pattern, effectively skipping the first dash and
starting with a gap.