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

render: fix sketch arrowheads #656

Merged
merged 14 commits into from
Jan 14, 2023

Conversation

gavin-ts
Copy link
Contributor

@gavin-ts gavin-ts commented Jan 12, 2023

Summary

Fixes sketch arrowheads sometimes appearing broken, and render them in a sketched style.

Screen Shot 2023-01-12 at 8 46 23 PM

Details

  • the sketch path isn't reliable to use markers on, causing the arrowheads to sometimes appear pointed in different directions
  • this can be avoided by rendering the arrowheads separately
  • adds special sketch versions of the arrowheads
  • fixes sketch: Arrowheads #502
  • ci had slightly different floating point values from rough js so I added code to truncate floats from rough to 6 decimal values

new arrowheads test before

_Users_gavinnishizawa_github_repos_d2_d2renderers_d2sketch_testdata_arrowheads_sketch exp svg

after rendering arrowheads separately

_Users_gavinnishizawa_github_repos_d2_d2renderers_d2sketch_testdata_arrowheads_sketch exp svg (1)

after rendering sketch specific arrowheads (final)

_Users_gavinnishizawa_github_repos_d2_d2renderers_d2sketch_testdata_arrowheads_sketch exp svg (2)

@gavin-ts gavin-ts force-pushed the fix-sketch-arrowheads branch 2 times, most recently from b5b6f11 to 3feedda Compare January 13, 2023 03:57
@gavin-ts gavin-ts marked this pull request as ready for review January 13, 2023 04:48
@gavin-ts gavin-ts requested a review from a team January 13, 2023 04:48
@katwangy
Copy link

<-> and triangle arrow is the same style? i think the <-> can look more "arrow-y"

i think “crows foot one” is a lil weird. circle can be a tad bigger (to match crows foot many) and the line can be more in the middle of the circle and the edge

@gavin-ts

@alixander
Copy link
Collaborator

arrow is the more arrow-y one @katwangy

@alixander
Copy link
Collaborator

but yeah i think the example is confusing, since <-> is default triangle, it's covered twice

@gavin-ts
Copy link
Contributor Author

gavin-ts commented Jan 14, 2023

updated the test example to remove the duplicate triangle case (and make the cf one circle larger and its line further away)

_Users_gavinnishizawa_github_repos_d2_d2renderers_d2sketch_testdata_arrowheads_sketch exp svg (5)

@gavin-ts
Copy link
Contributor Author

gavin-ts commented Jan 14, 2023

moved them closer to the shape to match the positions of the non-sketched arrowheads

non-sketched arrowheads

_Users_gavinnishizawa_github_repos_d2_d2renderers_d2sketch_testdata_arrowheads_sketch exp svg (7)

sketched

_Users_gavinnishizawa_github_repos_d2_d2renderers_d2sketch_testdata_arrowheads_sketch exp svg (9)

@gavin-ts gavin-ts merged commit 1a08b5f into terrastruct:master Jan 14, 2023
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.

sketch: Arrowheads
3 participants