You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "neilcsmith-net (via GitHub)" <gi...@apache.org> on 2025/01/31 17:34:17 UTC

[PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

neilcsmith-net opened a new pull request, #8217:
URL: https://github.com/apache/netbeans/pull/8217

   Quick fix for #8209 - an infinite loop with large dependency tree because `isThereFreeSpaceNonFixedSpace()` never returns `true`.
   
   Only loop ~3 rotations.  Increased force constant seems to improve result with reduced search.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#discussion_r1938221721


##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -150,7 +150,7 @@ private void init() {
         bounds = new Rectangle(magicSizeConstant  + (magicSizeMultiplier * nds), 
                                magicSizeConstant  + (magicSizeMultiplier * nds)); //g.getMaximumBounds();
         temp = bounds.getWidth() / 10;
-        forceConstant = 0.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);
+        forceConstant = 1.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);

Review Comment:
   not sure about changing the weights (might be the riskier change), but the graph looks somewhat similar still so its probably fine



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#discussion_r1938258343


##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -150,7 +150,7 @@ private void init() {
         bounds = new Rectangle(magicSizeConstant  + (magicSizeMultiplier * nds), 
                                magicSizeConstant  + (magicSizeMultiplier * nds)); //g.getMaximumBounds();
         temp = bounds.getWidth() / 10;
-        forceConstant = 0.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);
+        forceConstant = 1.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);

Review Comment:
   yeah there is a lot which could be done UX wise. Drag on empty space should move the viewport like everyone will intuitively try doing after using maps for so long. I think scroll wheel should probably zoom by default without having to hold ctrl (scrollbars are off anyway, which would be fine if drag would work), etc.
   
   i would like to have the output of
   ```
    mvn eu.maveniverse.maven.plugins:toolbox:tree -DverboseTree=true 
   
   ```
   and
   ```
   mvn eu.maveniverse.maven.plugins:toolbox:plugin-tree -DverboseTree=true
   ```
   (possibly more) in swing trees. with colors, filters etc. Since i don't find graphs very useful for purposes like this.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#discussion_r1938267315


##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -150,7 +150,7 @@ private void init() {
         bounds = new Rectangle(magicSizeConstant  + (magicSizeMultiplier * nds), 
                                magicSizeConstant  + (magicSizeMultiplier * nds)); //g.getMaximumBounds();
         temp = bounds.getWidth() / 10;
-        forceConstant = 0.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);
+        forceConstant = 1.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);

Review Comment:
   I'll try and have a look at those in context of next release. Have used the visual library a lot. Obviously not for delivery though. 
   
   Drag behaviour is there, just using mouse button 2. Default makes sense if the rectangular select action is used, but it isn't here. Will look at changing.
   
   May also consider adding the embedded satellite view you can see in front page image on https://www.praxislive.org/
   
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#issuecomment-2629318823

   @neilcsmith-net those issues can be likely closed with this PR
   https://issues.apache.org/jira/browse/NETBEANS-1146
   https://issues.apache.org/jira/browse/NETBEANS-2414
   https://issues.apache.org/jira/browse/NETBEANS-3283
   https://issues.apache.org/jira/browse/NETBEANS-4314
   https://issues.apache.org/jira/browse/NETBEANS-5770
   
   although the complete fix would also include to not do this on EDT - but this goes beyond a hotix


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#issuecomment-2630601116

   > That's unfortunately akin to trying to run a Swing layout off of the EDT.
   
   yeah this shouldn't be done, the hope is that something in the computation can be detached from swing but I haven't looked into it so far. To get an idea I would:
    - generate an async profiler flame graph to see the code paths which spend most of the CPU time during init (I still haven't progressed with my nb plugin which would make this easier)
    - now check if the geometric data can be copied, computed asynchronously and then copied back into the swing data structure on EDT
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "ebarboni (via GitHub)" <gi...@apache.org>.
ebarboni merged PR #8217:
URL: https://github.com/apache/netbeans/pull/8217


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#discussion_r1938251292


##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -150,7 +150,7 @@ private void init() {
         bounds = new Rectangle(magicSizeConstant  + (magicSizeMultiplier * nds), 
                                magicSizeConstant  + (magicSizeMultiplier * nds)); //g.getMaximumBounds();
         temp = bounds.getWidth() / 10;
-        forceConstant = 0.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);
+        forceConstant = 1.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);

Review Comment:
   It seems to make little different with a smaller graph, but with the larger projects I tested (that had the infinite loop before) it's much better and makes them somewhat useful.  I suspect it's mitigating some of the problems with the reduced search for free space, but as that never completes to compare it with, it's a little hard to know for sure.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on code in PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#discussion_r1938255114


##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -382,7 +382,8 @@ private void relayoutNonFixed(NodeWidget w) {
         r = 30;
         theta = 0;
         w.setFixed(false);
-        while (true) {
+        // 48 - ~3 times round?
+        for (int i = 0; i < 48; i++) {

Review Comment:
   Yes. What I'm less sure about is why it doesn't come to an end given the increase in `r`. Increasing the increment of `r` does get the loop to end on my test project (with a result similar to that of increasing the forces), but takes longer, and is potentially still liable to get stuck with a different set of nodes.
   
   This whole thing could do with a proper look after NB25.  I find the horizontal tree (in popup menu -> layout) a potentially more useful display for smaller projects, and might be a better default given it uses a lot less CPU.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "neilcsmith-net (via GitHub)" <gi...@apache.org>.
neilcsmith-net commented on PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#issuecomment-2630510146

   > although the complete fix would also include to not do this on EDT
   
   That's unfortunately akin to trying to run a Swing layout off of the EDT.  Might be OK to wrap in something like https://bits.netbeans.org/24/javadoc/org-netbeans-api-progress/org/netbeans/api/progress/BaseProgressUtils.html#runOffEventDispatchThread-java.lang.Runnable-java.lang.String-java.util.concurrent.atomic.AtomicBoolean-boolean-int-int-


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


Re: [PR] Fix infinite loop in FruchtermanReingoldLayout. [netbeans]

Posted by "mbien (via GitHub)" <gi...@apache.org>.
mbien commented on code in PR #8217:
URL: https://github.com/apache/netbeans/pull/8217#discussion_r1938220760


##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -382,7 +382,8 @@ private void relayoutNonFixed(NodeWidget w) {
         r = 30;
         theta = 0;
         w.setFixed(false);
-        while (true) {
+        // 48 - ~3 times round?
+        for (int i = 0; i < 48; i++) {

Review Comment:
   If I read this right it will stop moving the node if it can't find a free spot after a few attempts. Makes sense.



##########
java/java.graph/src/org/netbeans/modules/java/graph/FruchtermanReingoldLayout.java:
##########
@@ -150,7 +150,7 @@ private void init() {
         bounds = new Rectangle(magicSizeConstant  + (magicSizeMultiplier * nds), 
                                magicSizeConstant  + (magicSizeMultiplier * nds)); //g.getMaximumBounds();
         temp = bounds.getWidth() / 10;
-        forceConstant = 0.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);
+        forceConstant = 1.75 * Math.sqrt(bounds.getHeight() * bounds.getWidth() / nds);

Review Comment:
   not sure about changing the weights, but the graph looks somewhat similar still so its probably fine



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists