-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 dagre.js for abnormal wide edges #1254
Conversation
this change has suggestion from @beru at #1246 (comment) but still this part goes bad
|
I'm still reading the papers of Dagre ... and looks like will take a while. |
The code I wrote in #1246 (comment) didn't make much sense so I did some experiment. https://github.com/beru/netron/commits/horizontalCompaction/ It was difficult for me to understand |
@beru, Wow, you've got the sample working !
Algorithms are described in the papers in https://github.com/dagrejs/dagre/wiki#recommended-reading (I think you may know the links) and still reading these hard-to-understand algorithms in my spare time ... |
Thank you for the info. I didn't know the wiki of dagre project or maybe I just forgot it? Anyway, I'll try to understand what's written on the paper.
I wish to do so if I'll be able to come up with more polished code. The code I wrote doesn't use the second pass at all (as I'm not capable of understanding it) but that's probablly not a good idea. |
90d1518
to
c153259
Compare
514284f
to
0250d3e
Compare
952b6ed
to
2781faa
Compare
b448189
to
1d342d8
Compare
d9a23ec
to
5028ef6
Compare
@lutzroeder , I've update the patch with what I understand as of now. But still there exist a problem with the model you've provided and the result is same as in #1254 (comment) capture. If possible can you please land current change? |
This will update dagre.js for abnormal wide edges
@lutzroeder , I've updated for CI build fail. |
This will update dagre.js for abnormal wide edges