search

lutzroeder / netron

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

Update view.View.scrollTo method #1354

Closed beru closed 1 week ago

beru commented 2 weeks ago

This PR is opened to address the issue reported in #1352

lutzroeder commented 2 weeks ago
  1. Would it help to have a border zone for elements which are close to corners of the visible area? For example, shrink the test rectangle by 10% so elements close to the corners also get scrolled into the visible area?
  2. Should elements scroll to center or only move into the visible area?
  3. Long edges larger than the visible area cause scrolling when clicked. For example og_285 in gpt2-lm-head-bs-12.onnx.
beru commented 2 weeks ago
  1. Would it help to have a border zone for elements which are close to corners of the visible area? For example, shrink the test rectangle by 10% so elements close to the corners also get scrolled into the visible area?

I'm not sure about that. Shall I create a commit to check UX?

  1. Should elements scroll to center or only move into the visible area?

Centering a clicked node is a bit too much I think. I suppose moving it inside a safe-area will suffice. The size of the safe area can be around 80% or 90%.

If any of the attemps won't satisfy @fdwr, reverting c5868fe92cf2347749238a7bf8326da4bf270679 would be needed.

  1. Long edges larger than the visible area cause scrolling when clicked. For example og_285 in gpt2-lm-head-bs-12.onnx.

Firstly, I'm not really sure if scrolling is necessary when edges are selected. Does it really help? In some cases when lines are very long, clicking them triggers a long scroll and connected nodes that were previously shown tend to go out of the visible area due to the scrolling.

beru commented 1 week ago

@lutzroeder @fdwr done updating the code so can you please check new auto scroll behavior is OK?

lutzroeder commented 1 week ago
  1. Open candy.onnx
  2. Open Find sidebar
  3. Scroll to bottom and select scale_B
  4. Scroll to bottom and select scalePreprocessor

Actual: does not scroll to new location.

beru commented 1 week ago

@lutzroeder

  1. Scroll to bottom and select scalePreprocessor Actual: does not scroll to new location.

Fixed the bug with 19d547ffeaf29f1d127b1851f010b02bee9b0f05 by using Number.NEGATIVE_INFINITY instead of 0. Currently, the change of this PR disregards your recent commit 7b993c63d1a04c806c45aed2eefbb0368b63e753 so a further adjustment might be needed...

This is an off-topic but I've noticed that outputImage connection of candy.onnx isn't shown in the FIND sidebar list albeit the fact that inputImage connection is shown.

lutzroeder commented 1 week ago

Regressed #1356

Open alexnet.zip.pth.

Actual: initial positioning shows center of node Expected: initial positioning should show top of node

beru commented 1 week ago

Regressed https://github.com/lutzroeder/netron/issues/1356

Honestly, it's difficult to change auto scroll behavior while keeping #1356.