d2renderer: #579 Circle Arrowhead

[Paracelsus-Rose]

Circles for arrowheads #579

./ci/test.sh Is failing. Not sure why, all else looks good.

[alixander]

./ci/test.sh Is failing. Not sure why, all else looks good.

what's the error? common issues: we're on go 1.18

[Paracelsus-Rose]

    --- FAIL: TestE2E/stable (0.00s)
        --- FAIL: TestE2E/stable/circle_arrowhead (0.13s)
            e2e_test.go:175: unexpected error: diff (rerun with $TESTDATA_ACCEPT=1 or $TA=1 to accept):
                --- /dev/null
                +++ b/testdata/stable/circle_arrowhead/dagre/board.got.json
                @@ -0,0 +1,264 @@
                +{

I assume if I do the test data accept it goes away, just was not sure.

[Paracelsus-Rose]

./ci/test.sh Is failing. Not sure why, all else looks good.

what's the error? common issues: we're on go 1.18

Did TA=1, not sure if this is correct or not.

[alixander]

awesome!

(feel free to update the docs https://github.com/terrastruct/d2-docs/blame/master/docs/tour/connections.md#L135)

[alixander]

yep, because it has no expected, for new tests you'll want to run with that accept env variable. i'll add that flow to the CONTRIBUTING.md.

[Paracelsus-Rose]

awesome!

(feel free to update the docs https://github.com/terrastruct/d2-docs/blame/master/docs/tour/connections.md#L135)

terrastruct/d2-docs#55

[alixander]

you'll need to run the tests with TESTDATA_ACCEPT and then check in the results

[ejulio-ts]

🚀 thanks @Paracelsus-Rose

Particularly, I found a bit hard to follow some constants in d2svg, not sure if we should change something in general so that we can include some ascii art about them. Most of them are just geometrical stuff for rendering, so having some drawings/examples could be useful in general.

Then, I believe this is related to offset, I wonder why the circles aren't touching the shapes borders.

Finally, I believe it is worth changing some of the test cases to use other shapes, besides squares, just to see how the arrowheads are rendered

[alixander]

Then, I believe this is related to offset, I wonder why the circles aren't touching the shapes borders.

i think that's right, we don't want it touching the border, but I think we should leave like 1px of padding or something, right now I did notice it's a little too much. i'd be curious to see if no padding looks weird though or if i'm imagining it

[Paracelsus-Rose]

I was following the diamond's for render. Here I show what I was looking at locally. I see the crows feet and arrow both touch, the diamond was not and I was not sure. Having the circles touch looks a little strange, the padding I thought made more sense visually from a distance and zoomed in.

This is them touching. Let me know which you prefer.

[Paracelsus-Rose]

Then, I believe this is related to offset, I wonder why the circles aren't touching the shapes borders.

i think that's right, we don't want it touching the border, but I think we should leave like 1px of padding or something, right now I did notice it's a little too much. i'd be curious to see if no padding looks weird though or if i'm imagining it

This is how it currently stands.

[alixander]

i think you have to account for stroke width

[Paracelsus-Rose]

i think you have to account for stroke width

Will need to look around and see what I can come up with. Have not worked with svg's prior to this. I see LineArrowhead is the only other one accounting for it hardcoded in, that and the offset on crows foot. Personally I liked the smallest gap thus far, and not touching the best, but it is not the same as everything else. Will play around and see. Just to make sure, you are saying that it is not pixel perfect on the edge connection there, due to not accounting for stroke width, correct?

[alixander]

Just to make sure, you are saying that it is not pixel perfect on the edge connection there, due to not accounting for stroke width, correct?

yes.

Personally I liked the smallest gap thus far

i do too, but maybe we'll like the third option best of perfectly touching (after accounting for stroke widths)

[ejulio-ts]

@Paracelsus-Rose

You might want to check this PR for arrowhead overlaps 😂
#48

It took me awhile to get it right (with the web app, d2, tala).
Particularly, you just need to account for edge and arrowhead width, the proper tracing to the shape border happens somewhere else.

What happens in that SVG borders grow in both directions ([in|out]ward) so you need to handle that when computing the overlap
https://stackoverflow.com/questions/7241393/can-you-control-how-an-svgs-stroke-width-is-drawn

[Paracelsus-Rose]

@Paracelsus-Rose

You might want to check this PR for arrowhead overlaps joy #48

It took me awhile to get it right (with the web app, d2, tala). Particularly, you just need to account for edge and arrowhead width, the proper tracing to the shape border happens somewhere else.

What happens in that SVG borders grow in both directions ([in|out]ward) so you need to handle that when computing the overlap https://stackoverflow.com/questions/7241393/can-you-control-how-an-svgs-stroke-width-is-drawn

Still not right, and it feels so magic number-y now. I wrote out a TS script quick to check my numbers and that just made me even more confused. At least in regards to the cx.

Will go back and attack it in a little bit. Svg's are interesting. Been good fun, apologize its been a crackpot job.

[ejulio-ts]

About the diamond offset. I believe this is an issue as it should touch the shape.
About the circle. I believe that should just account for edge stroke width when computing the edge offset, so that things are placed properly.

Consider the diagram below, where * represents the 1px edge/shape and the surround lines represent the offset of shape/edge stroke width

│ * │ note the edge and shape overlap │ * │
│ * │              ▼                  │ * │
│ * └────────────┬───┬────────────────┘ * │  ┬
│ ***************|***|******************* │  | shape stroke width
└────────────────┤ * ├────────────────────┘  ┴
                 │ * │
                 │ * │
                 │ * │
                 │ * │
                 │ * │
                 │ * │
                 │ * │
                 │ * │

                 |---|
            edge stroke width

The arrowhead adjustment I worked on was https://github.com/terrastruct/d2/blob/master/d2renderers/d2svg/d2svg.go#L300-L308

The idea is:

• By default, the offset is half shape + half edge stroke width (see the diagram above)
• If there's an arrowhead, then the arrowhead adds edge stroke width to offset because the arrowhead goes at the end of the edge adding 2*half edge stroke width overlap

[gavin-ts]

Consider the diagram below, where * represents the 1px edge/shape and the surround lines represent the offset of shape/edge stroke width

The idea is:

• By default, the offset is half shape + half edge stroke width (see the diagram above)
• If there's an arrowhead, then the arrowhead adds edge stroke width to offset because the arrowhead goes at the end of the edge adding 2*half edge stroke width overlap

To expand on that, the end coordinate of the edge will be exactly on the border of the shape, but we need to move it back so the end of where the edge's stroke is rendered matches up to where the end of the shape's stroke is rendered.

So if we have an edge ending at point O at (0,0), shape stroke width is 3, and edge stroke width is 5, it will need to move down (1.5+2.5). Otherwise the default rendering would have the overlap below:

│ *** │ note the edge and shape overlap │ *** │
│ *** │              ▼                  │ *** │
│ *** │           ┌─────┐               │ *** │
│ *** └───────────┼*****┼───────────────┘ *** │  ┬ y=-1.5
│ ****************|*****|******************** │  | 
│ ****************|**O**|******************** │  | shape stroke width = 3
│ ****************|*****|******************** │  | 
└─────────────────┼*****┼─────────────────────┘  ┴ y=1.5
                  │*****│                    
                  │*****│ O: @(0,0) is edge end point (y is equal to bottom coordinate of shape)
                  │*****│                    
                  │*****│                    
                  │*****│                    
                  │*****│                    
                  │*****│                    
                  │*****│                    
                                             
                  |-----|                    
              edge stroke width = 5          
              x=-2.5   x=2.5

after moving down with the offset of 4:

│ *** │ edge and shape ends now line up │ *** │
│ *** │              ▼                  │ *** │
│ *** └─────────────────────────────────┘ *** │  ┬ y=-1.5
│ ******************************************* │  | 
│ ******************************************* │  | shape stroke width = 3
│ ******************************************* │  | 
└─────────────────┬─────┬─────────────────────┘  ┴ y=1.5
                  │*****│                    
                  │*****│                    
                  │**O**│ O: @(0,4)
                  │*****│                    
                  │*****│                    
                  │*****│                    
                  │*****│                    
                  │*****│                    
                  │*****│                    
                                             
                  |-----|                    
              edge stroke width = 5          
              x=-2.5   x=2.5

[Paracelsus-Rose]

@gavin-ts @ejulio-ts
Wrote this today to learn more about SVG's, stroke widths etc. Not sure if more questions were created or answered by doing so. Realizing I am really not accounting for stroke width correct. Will think more on it.

https://stackblitz.com/github/Paracelsus-Rose/svg-circles?file=src%2Fviews%2FSvgCircleView.vue

https://github.com/Paracelsus-Rose/svg-circles

Did not think it would become this when I was like, oh that looks easy :-). Cheers yall, its been fun.

[Paracelsus-Rose]

The above linked stackblitz is where I have it currently. I am thinking there is some assumption I am making there which does not translate to d2. Most recent commit to this PR I tried to mimick the values over, but something is going wrong. Maybe need some new eyes on it, or a fresh head tomorrow.

Here stroke width is accounted for correctly, and all is aligning up on center.

But in d2svg.go it is not still.

Until next time.

[alixander]

i haven't looked into your code, but there's a few likely offenders:

1. cx and cy are middle of circle
2. r is radius (half of length)
3. svg coordinates are top-left is 0,0 and going right 1 unit and down 1 unit gets you to 1,1.
4. stroke-width grows from the middle, as demonstrated by my colleague's ascii art
5. both the circle path (connection arrowhead) and the main path (connection) have stroke-width

[Paracelsus-Rose]

Appreciate the help @gavin-ts. This seems right now. Do apologize with the confusion. You were also correct, been thinking the last 3 days while I settled in at the new house about what @alixander said, how the SVG's are rendered from 0,0 in the top left. I knew when he said that I was trying to solve a problem on a false pretense. This should be corrected now, anything else which needs to be done let me know. Did not expect to get stuck on this, but wow did I learn a lot about SVG rendering. Yall inspired me to make some fun stuff in the last week. Cheers everyone.

[gavin-ts]

looks like the test was added twice in a merge, other than that it looks good to go

[alixander]

sweet, let's try to get this into the release

not needed