lutzroeder / netron

Visualizer for neural network, deep learning and machine learning models
https://netron.app
MIT License
28.05k stars 2.78k forks source link

New automatic scroll to selected node is disorienting #1352

Closed fdwr closed 1 month ago

fdwr commented 1 month ago

Steps to Reproduce:

  1. Open a graph.
  2. Click on a node to see its properties.
  3. Notice the whole graph now shifts around, and the node you just clicked on is several nodes away now. This is pretty jarring and a little confusing, especially when you have parts of the graph that repeat and look similar to each other. I don't remember it doing this before?? 😵‍💫 Is there a hidden option to disable this or a way to use a previous version? 🥲

Please attach or link model files to reproduce the issue.

Any.

fdwr commented 1 month ago

It might be a collateral side effect of attempting to address this issue: https://github.com/lutzroeder/netron/issues/1344

beru commented 1 month ago

@fdwr When a node is selected, the app scrolls the selected node to the center of the graph view (link) and I personally think it's a little excessive. By updating the code like this link, the scrolling occurs less often but it might be less exciting to scrolling addicts.

lutzroeder commented 1 month ago

@beru can you create a pull request, ideally addressing the scrollbar issue. @fdwr can you help test and review if the suggested change addresses the issue.

beru commented 1 month ago

@lutzroeder element.clientWidth doesn't include width of scrollbar so I'm thinking to use this. https://stackoverflow.com/a/40568748

Currently, the app scrolls a selected node to the center of the graph view. With the change I'm about to introduce with a new PR, the scrolling will not occur when a whole rectangle area of selected node is in the container client area. I'm not really sure if this change of behaviour is acceptable but I'll open a PR anyways.

fdwr commented 1 month ago

@fdwr can you help test and review if the suggested change addresses the issue.

Gladly - how? I just loaded ResNet into https://netron.app/, and the graph still shifts around on click 🤔. Might I need to explicitly refresh my browser cache?

beru commented 1 month ago

@fdwr the change for automatic scroll (f1ecfef71014cd4e100b1d4c273f966f1c724852) has landed to main branch very recently so it's not released yet. Given that Lutz has released new versions weekly, next version (Version 7.8.9) would be released on 21/09/2024.

The istructions to test developmenet code are written in https://github.com/lutzroeder/netron/blob/main/CONTRIBUTING.md#debugging You may still experience jumps of scroll with the updated code, though I hope they are reduced.