-
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 #1246
Conversation
This will dagre.js for abnormal wide edges
this is to fix #864 issue |
@@ -1591,7 +1591,7 @@ dagre.layout = (graph, layout) => { | |||
} | |||
const label = g.node(v).label; | |||
if (min !== Number.POSITIVE_INFINITY && label.borderType !== borderType) { | |||
xs[v] = Math.max(xs[v], min); | |||
xs[v] = Math.min(xs[v], min); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will choose smallest width
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about the following code?
let minOut = Number.POSITIVE_INFINITY;
let minIn = Number.POSITIVE_INFINITY;
for (const e of blockG.node(v).out) {
minOut = Math.min(minOut, xs[e.w] - e.label);
}
for (const e of blockG.node(v).in) {
minIn = Math.min(minIn, xs[e.w] - e.label);
}
const label = g.node(v).label;
if (minOut !== Number.POSITIVE_INFINITY && minIn !== Number.NEGATIVE_INFINITY && label.borderType !== borderType) {
if (Math.abs(xs[v] - minOut) < Math.abs(xs[v] - minIn)) {
xs[v] = Math.max(xs[v], minOut);
}else {
xs[v] = Math.max(xs[v], minIn);
}
}
model | before | after |
---|---|---|
yolov5n | ||
sample.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about the following code?
This looks better !
@@ -1740,7 +1740,7 @@ dagre.layout = (graph, layout) => { | |||
for (const [v, x] of Object.entries(xs)) { | |||
const halfWidth = g.node(v).label.width / 2; | |||
max = Math.max(x + halfWidth, max); | |||
min = Math.min(x - halfWidth, min); | |||
min = Math.min(Math.max(0, x - halfWidth), min); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some x - halfWidth
goes to minus, which will make below const width = max - min;
larger.
@seanshpark this is changing how other models are rendered? Is there a good way to reason pros vs. cons? Before: After: |
@ lutzroeder, thanks for looking into this ! |
@lutzroeder , as not knowing much about algorithms in dagre, my wild guess is that the end node of the example graph tries to go to the center of the layout (the minimum diff from center?) I'll try to study what the algorithm does and find a way for this. Anyway, I'll try to find something better. |
66d40fe
to
19e4123
Compare
Close this for now and reopen when change is ready. |
I couldn't repoen this so posted another #1254 |
This will dagre.js for abnormal wide edges