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

fix: makes bidirectional animations correct #1939

Merged
merged 13 commits into from
May 19, 2024

Conversation

danielsuh05
Copy link
Contributor

@danielsuh05 danielsuh05 commented May 14, 2024

fixes #1936, #654

Screen.Recording.2024-05-14.at.16.20.43.mov

Makes <-> and -- have correct animation. I edit the path string instead of the points due to the rounded corners becoming sharp when using S curves (if you have a path from b -> a and b -> c, it is not trivial to round the corner b). Bezier curve segmentation is not perfect due to the parameterization of a bezier curve being inaccurate, but is good enough.

@danielsuh05 danielsuh05 changed the title fix: makes bidirectional animations correct (fixes #1936) fix: makes bidirectional animations correct May 14, 2024
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

the tests outputs look great! Can you please add to next.md as well?

Also I'm not sure if you want to implement this for the sketch renderer (d2renderers/d2sketch) as well. I don't know what it entails, you're welcome to take a look or just open another issue after this.

e2etests/testdata/txtar.txt Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
@danielsuh05
Copy link
Contributor Author

I'll create an issue for the sketch renderer soon, I'll take a look at how much work it is and decide if I want to tackle it. Thanks for all the comments!

d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
d2renderers/d2svg/d2svg.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

looks great, thank you! 🚀

@alixander alixander merged commit 0a54cb4 into terrastruct:master May 19, 2024
3 checks passed
@danielsuh05 danielsuh05 deleted the issue1936 branch May 19, 2024 18:53
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.

bidirectional connection animation
2 participants