terrastruct / TALA

A diagram layout engine designed specifically for software architecture diagrams
https://terrastruct.com/tala
Other
202 stars 4 forks source link

this layout solution seems unecessarily complex #15

Closed uliw closed 1 year ago

uliw commented 1 year ago

The code below produces a rather tangled layout (i.e., connectors cross, are lengthy and boxes are scattered). Can this be improved? And if this cannot be done automatically, would layout hints be an option to find a better solution?

S: Surface: {
A
I
P
}

I :Intermediate:{
A
I
P
}

D: Deep :{
A
I
P}

S.A <-> I.A
S.I <-> I.I
S.P <-> I.P

I.A <- D.A
I.I <- D.I
I.P <- D.P
alixander commented 1 year ago

@uliw hmm yes, some of the lengths are unnecessarily long. We'll look into this, ty for reporting. @ejulio-ts

https://play.d2lang.com/?script=dM0xCgJBDIXhPqf4L-AeIIgwkCbdwJxg0QgWiixjJd5djGksLH_eB28o47Gd12MoT4Em4AJd4CXwSdRvM7ZrnC7rDP3DTLGIO797jWNp7HcHfGlZXuVZvarXYWIsrafFknpS7CvfAQAA__8%3D&layout=tala

ejulio-ts commented 1 year ago

@uliw this one was fixed and should be available in TALA's next release image

uliw commented 1 year ago

Thanks! BTW, is there a way to affect the ordering (I.e., Surface on top)?

ejulio-ts commented 1 year ago

you might set

surface: {
   near: top
}

https://d2lang.com/tour/text#near-a-constant

uliw commented 1 year ago

hmmh, neither this nor the example https://d2lang.com/tour/text#near-a-constant will work. d2 version is v0.1.3, I updated to tala 0.2.10 but I am not sure how to verify the version.

running

title: |md
       # A winning strategy
       | { near: top-center }

poll the people -> results
results -> unfavorable -> poll the people
results -> favorable -> will of the people

will compile wo errors, but no title text is shown

Using the near: top complains that top is invalid. Replacing this with top-center results in the following error when I add it to the code above

err: failed to compile: exit status 2
err: stderr:
err: panic: runtime error: invalid memory address or nil pointer dereference
err: [signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0xdcafae]
err:
err: goroutine 35 [running]:
err: github.com/terrastruct/src/backend/tala.(*Node).addEdge(...)
err:    github.com/terrastruct/src/backend/tala/node.go:848
err: github.com/terrastruct/src/backend/tala.(*Graph).Connect(...)
err:    github.com/terrastruct/src/backend/tala/graph.go:397
err: github.com/terrastruct/src/backend/tala/transpilers/d2transpiler.PopulateNodes({0xdcfb92?, 0x1a46d40?}, 0xc00065c000, 0xc000013d40)
err:    github.com/terrastruct/src/backend/tala/transpilers/d2transpiler/d2transpiler.go:91 +0x62e
err: github.com/terrastruct/src/backend/tala/transpilers/d2transpiler.Layout({0x1a4d550, 0xc000d42000}, 0xc000117e50?)
err:    github.com/terrastruct/src/backend/tala/transpilers/d2transpiler/d2transpiler.go:128 +0x25d
err: main.talaPlugin.Layout({0xc000658000?}, {0x1a4d550, 0xc000d42000}, 0xc00065c000?)
err:    github.com/terrastruct/src/backend/tala/cmd/d2plugin-tala/plugin_tala.go:102 +0xda
err: oss.terrastruct.com/d2/d2plugin.layout({0x1a4d550, 0xc000d42000}, {0x1a4d340, 0xc00094fd70}, 0xc0001a1560)
err:    oss.terrastruct.com/d2@v0.1.4-0.20221226200138-1efc46d21766/d2plugin/serve.go:76 +0xb5
err: oss.terrastruct.com/d2/d2plugin.Serve.func1({0x1a4d550, 0xc000d42000}, 0xc0001a1560)
err:    oss.terrastruct.com/d2@v0.1.4-0.20221226200138-1efc46d21766/d2plugin/serve.go:42 +0x1f2
err: oss.terrastruct.com/util-go/xmain.(*State).Main.func1()
err:    oss.terrastruct.com/util-go@v0.0.0-20221226181616-c04ce7d1b79f/xmain/xmain.go:91 +0x71
err: created by oss.terrastruct.com/util-go/xmain.(*State).Main
err:    oss.terrastruct.com/util-go@v0.0.0-20221226181616-c04ce7d1b79f/xmain/xmain.go:89 +0x155
ejulio-ts commented 1 year ago

🤔 We just released TALA v.0.2.11, can you test with it? here is the playground version, so it should be working

https://play.d2lang.com/?script=XNDBCsJADATQe75iwHN_oAfBT6k6rQsxWbapRaz_Lose1p4C4c1hJlIoe2z3qwAHnLAms2QT6sTB6SkbXrVpj_DcXWjB8hbJroq4EZmeleiOKJwXjVl-t74WG4eHl-H8FbtQK__cmlThY0s_AQAA__8%3D&layout=tala

ejulio-ts commented 1 year ago

@uliw you can check TALA's version with d2plugin-tala --version

uliw commented 1 year ago

d2 version = 0.1.4 d2plugin-tala --version = v0.2.11 running

title: |md
       # A winning strategy
       | { near: top-center }

poll the people -> results
results -> unfavorable -> poll the people
results -> favorable -> will of the people

results in a figure that is missing the title. Using the same code on the playground site shows the title. I did a complete re-install but the result is the same

ejulio-ts commented 1 year ago

Here is the result of a fresh install

image

image

Here is the SVG/HTML image

and here is the PNG result d2 --layout=tala test.d2 test.png

image

uliw commented 1 year ago

ok, so probably has something to do with my local environment. I did the test again and can confirm that the SVG file is still missing the title. Exporting to PNG, however, renders the title as expected. So probably related to the SVG rendering. Opening the file with Inkscape results in the same result (i.e. no title). Using the XML editor in Inkscape, I am able to find the Title text, but it will not show (nor can you select it). I've attached ggg.svg which is the output as produced by d2, and ggg_2.svg which is the same file, but I deleted most elements.

ggg

ggg_2

ejulio-ts commented 1 year ago

interesting 🤔 What OS are you using? What browser?

Just in case, the SVG you attached rendered as expected here on GH for me image

uliw commented 1 year ago

I am on Linux (kernel 6.0) Just tested with firefox and chrome. It works indeed but fails with Inkscape, and applications that render svg with librsvg. Looking at the XML code, I notice that the Title text is implemented as svg:foreignObject rather than a standard text object https://developer.mozilla.org/en-US/docs/Web/SVG/Element/text

ejulio-ts commented 1 year ago

this is an issue on how D2 renders the SVG then. I opened an issue there to continue the discussion and find out why we ue foreignObject instead of text https://github.com/terrastruct/d2/issues/596

thanks for helping to debug this one :)

alixander commented 1 year ago

@uliw Markdown is compiled to HTML, which has to be in a foreignobject. if foreign objects are a nonstarter for the format you're viewing the SVGs in, you can use plain text.

https://play.d2lang.com/?script=PJGxCkIxDEX3fMX5gbe4mcFR_A_NUCit1AgV6b8_kqHjvQQOJ9eLV1MeVmvnPoq114e_QNgp3t_H05rbEEglxW16pIBr3sL-oXK5ZhMbKj6-lnFTd7dkyeS48ZMzAAD__w%3D%3D