Skip to content
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

zoom sync with other nv instances #992

Closed
helghast79 opened this issue Jun 14, 2024 · 15 comments · Fixed by #1003 or #1021
Closed

zoom sync with other nv instances #992

helghast79 opened this issue Jun 14, 2024 · 15 comments · Fixed by #1003 or #1021
Assignees

Comments

@helghast79
Copy link

I have multiple nv instances working on the same page, since last version 0.43, zooming from one instance, syncs to the other nv's automatically. This would be acceptable if broadcastTo was being used but there should be a way to turn this off since in many cases zoom sync is not needed. In previous version 0.42.0 this didn't happen but did so in 0.43.0. Any ideia what can be causing this?

neurolabusc added a commit that referenced this issue Jun 17, 2024
@helghast79
Copy link
Author

Focusing on the canvas from which the event started (as showed in #996), seems to contain propagation to other canvas on the same page.

I'll just add a suggestion for zoom tool which is to position the crosshair on the mouse pointer when scrolling (if cursor is over the image), that way the zoom will be focused on that point which is normally what users expect. I achieve this by forcing a mouse click when user scrolls with zoom tool but that's an hacky way of doing it.

@neurolabusc
Copy link
Collaborator

@helghast79 the NiiVue team (including developers from FSL, AFNI, MRIcroGL) explored a lot of different ideas for the center of expansion when zooming (e.g. using the mouse scroll wheel or pinch gesture in this live demo): center of view port, center of crosshair, keep crosshair in same screen location, mouse position. Each method has its own use cases, but the current approach (which was adopted from FSLeyes) won the consensus vote as the best balance. As you note, the user can always mouse click if the cursor location is their preference. The project is open source with a permissive license, so each developer can use the zoom method they prefer.

@helghast79
Copy link
Author

got it, thanks

@helghast79
Copy link
Author

Just one question, is it possible to zoom/pan on mosaics?

@neurolabusc
Copy link
Collaborator

No, a mosaic string explicitly demands which slice is drawn. Consider the live demo where the first three slices are axial images at 0, 50, and 60mm coordinates: since panning is 3D allowing panning would make it hard to create reproducible images. Likewise, a mosaic is resized to show the whole image optimally. The demo does show that the measurement and contrast drag modes do work intuitively with mosaics.

@helghast79
Copy link
Author

thanks for the quick response. I have seen the demos and in code I haven't seen any option related to mosaic that pointed in zooming/panning. Just wondering if it might be in plans to be developed, something like a mosaic string option to set zoom center & zoom scale (e.g. "Zc 10 -20 Zs 2.3" to set zooming and center to that point with a 2.3 scale while maintaining the original image size as view window and cropping the rest). The ability to draw representations of images in mosaics way, present a very interesting approach, because it is very easy to save these settings and to load them latter exactly how they were. It would be great if zoom/pan were possible...

@helghast79
Copy link
Author

Please reopen this, since zoom still syncs with other nv instances after v0.42. The issue can be seen in sync demo, setting broadcast to "independent" and then changing the zoom on one of the viewports, the other will update on that zoom if we click on it (change pointer location). So only the pointer location is actually independent, zoom is not. This happens, even if we never set broadcast on either viewports

@hanayik hanayik reopened this Jul 2, 2024
@neurolabusc
Copy link
Collaborator

@helghast79 thanks for the explicit and easy to replicate description of your issue.

@cdrake I think your PR1003 fails to make a deep copy.

I think the solution is to change nvdocument.ts from:

 sceneData: { ...INITIAL_SCENE_DATA },

to read:

sceneData: JSON.parse(JSON.stringify(INITIAL_SCENE_DATA)),

this kludge uses JSON serialization/deserialization for deep copying. As you have mentioned previously, an alternative would be to use lodash, but that would add a dependency.

Can you test my suggested fix, and if you are happy with it please submit a new pull request.

@cdrake
Copy link
Collaborator

cdrake commented Jul 2, 2024

@helghast79 - Please write specific steps to reproduce. I'm happy to track this down for good. The following video shows the behavior I am seeing with version 0.43.5 using the bidirectional sync demo.

Screencast.from.07-02-2024.06.58.05.PM.webm

@neurolabusc
Copy link
Collaborator

neurolabusc commented Jul 3, 2024

@cdrake to replicate this:

  1. open the sync.mesh live demo.
  2. set the broadcast pull down to independent so gestures on the top panel should not influence the bottom.
  3. Set the Drag pull down to pan/zoom
  4. Drag your mouse over the top panel to pan the image (or pinch/scroll wheel to adjust the zoom). At this stage only the top panel changes, which seems correct.
  5. Click the mouse on the bottom panel so the lower panel image refreshes and notice that the pan has been applied.

In the example above, you need to change several defaults to experience the issue. Below is a minimal demo of the issue that sets the default to elicit the issue. Replace your \niivue\src\index.html with the code below and run npm run dev. Note that the web page will initiate with independent views, such that changes in one context should be independent of the other. Next, perform a drag gesture on the top panel (nv1) and notice when you release the mouse the pan effect of nv1 is duplicated on the bottom panel (nv2). Implementing my change will fix the issue.

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width,initial-scale=1.0" />
    <title>sync Mesh</title>
    <link rel="stylesheet" href="niivue.css" />
  </head>
  <body>
    <noscript>niivue requires JavaScript.</noscript>
    <header>
      <label for="dpiCheck">HighDPI</label>
      <input type="checkbox" id="dpiCheck" unchecked />
      <label for="zoomCheck"> 2D/3D Zoom Sync</label>
      <input type="checkbox" id="zoomCheck" unchecked />
      <label for="sliceType"> View</label>
      <select id="sliceType">
        <option value="0">Axial</option>
        <option value="1">Coronal</option>
        <option value="2">Sagittal</option>
        <option value="4">Render</option>
        <option value="3" selected>A+C+S+R</option>
      </select>
      <label for="controller"> Broadcast</label>
      <select id="controller">
        <option value="0"  selected>Independent</option>
        <option value="1" >Top Controls Bottom</option>
        <option value="2">Bottom Controls Top</option>
        <option value="3">Bidirectional</option>
      </select>
      <label for="dragMode"> Drag</label>
      <select id="dragMode">
        <option value="contrast" >contrast</option>
        <option value="measurement">measurement</option>
        <option value="pan" selected>pan/zoom</option>
        <option value="none">none</option>
      </select>
      <label for="dxSlider"> Mesh clipping</label>
      <input
        type="range"
        min="0"
        max="11"
        value="11"
        class="slider"
        id="dxSlider"
      />
    </header>
    <main>
      <canvas id="gl1"></canvas>
    </main>
    <main>
      <canvas id="gl2"></canvas>
    </main>
    <footer>
      <x id="intensity"> &nbsp;</x>
      <x id="intensity2"> &nbsp;</x>
    </footer>
    <script type="module" async>
      import { Niivue, NVImage, NVMesh, NVMeshLoaders } from "./niivue/index.ts"
      sliceType.onchange = function() {
        let st = parseInt(document.getElementById("sliceType").value)
        nv1.setSliceType(st)
        nv2.setSliceType(st)
      }
      controller.onchange = function() {
        let v = parseInt(controller.value)
        if ((v & 1) > 0)
            nv1.broadcastTo([nv2], { "3d": true, "2d": true })
        else
            nv1.broadcastTo()
        if ((v & 2) > 0)
            nv2.broadcastTo([nv1], { "3d": true, "2d": true })
        else
            nv2.broadcastTo()
      }
      dragMode.onchange = function () {
        switch (dragMode.value) {
          case "none":
            nv1.opts.dragMode = nv1.dragModes.none
            break
          case "contrast":
            nv1.opts.dragMode = nv1.dragModes.contrast
            break
          case "measurement":
            nv1.opts.dragMode = nv1.dragModes.measurement
            break
          case "pan":
            nv1.opts.dragMode = nv1.dragModes.pan
            break
        }
        nv2.opts.dragMode = nv1.opts.dragMode
      }
      dpiCheck.onchange = function() {
        nv1.setHighResolutionCapable(this.checked)
      }
      zoomCheck.onchange = function() {
        nv1.opts.yoke3Dto2DZoom = this.checked
        nv2.opts.yoke3Dto2DZoom = this.checked
        if (this.checked) {
          nv1.scene.volScaleMultiplier = nv1.scene.pan2Dxyzmm[3]
          nv2.scene.volScaleMultiplier = nv2.scene.pan2Dxyzmm[3]
          nv1.drawScene()
          nv2.drawScene()
        }
      }
      dxSlider.oninput = function () {
        let dx = parseFloat(this.value)
        if (dx > 10) dx = Infinity
        nv1.setMeshThicknessOn2D(dx)
      }
      function handleIntensityChange(data) {
        document.getElementById("intensity").innerHTML =
          "&nbsp;&nbsp;" + data.string
      }
      function handleIntensityChange2(data) {
        document.getElementById("intensity2").innerHTML =
          "&nbsp;&nbsp;" + data.string
      }
      var nv1 = new Niivue({
        show3Dcrosshair: true,
        onLocationChange: handleIntensityChange,
        backColor: [1, 1, 1, 1],
      })
      nv1.attachTo("gl1")
      nv1.setHighResolutionCapable(false)
      nv1.opts.isOrientCube = true
      var volumeList1 = [{ url: "../demos/images/mni152.nii.gz" }]
      await nv1.loadVolumes(volumeList1)
      await nv1.loadMeshes([{ url: "../demos/images/BrainMesh_ICBM152.lh.mz3" }])
      nv1.setMeshShader(0, "Outline")
      nv1.opts.multiplanarForceRender = true
      nv1.setSliceType(nv1.sliceTypeMultiplanar)
      nv1.setSliceMM(true)
      nv1.setClipPlane([0, 180, 40])
      //
      var nv2 = new Niivue({
        show3Dcrosshair: true,
        onLocationChange: handleIntensityChange2,
        backColor: [1, 1, 1, 1],
      })
      nv2.attachTo("gl2")
      nv2.setHighResolutionCapable(true)
      await nv2.loadVolumes(volumeList1)
      await nv2.loadMeshes([{ url: "../demos/images/BrainMesh_ICBM152.lh.mz3" }])
      nv2.opts.multiplanarForceRender = true
      nv2.setSliceType(nv2.sliceTypeMultiplanar)
      nv2.setSliceMM(true)
      nv2.setClipPlane([0, 180, 40])
      dragMode.onchange()
      nv1.onDragRelease  = function () {
        nv2.drawScene()
      }
    </script>
  </body>
</html>

@cdrake
Copy link
Collaborator

cdrake commented Jul 3, 2024

Thank you. I realized after I had made the comment there was pan zoom. The issue should be fixed in PR 1021.

@helghast79
Copy link
Author

Hello all, just to add the behaviour I explained before, in the 3 viewports displayed, the first 2 from left are synced while the last one on the right is independent. It can be seen that on 0.42, zooming is not affected at all by the other's zoom, even when they are synced (which is undesirable in my opinion). While in 0.43.5, zoom is always affected by the other zoom even if they are not synced, in that case they seem not to be affected but once the pointer changes, zoom syncs.

The best behaviour in my opinion, would be to sync zoom when viewports are synced, and leave them independent if they are not.

version 0.42.0

v0.42.mov

version 0.43.5

0.43.5.mov

@cdrake
Copy link
Collaborator

cdrake commented Jul 3, 2024

@helghast79 - Thank you for your patience. I forgot that pan/zoom existed for 2D. PR 1021 should fix the issue and restore the behavior from 0.42 as well as adding 3D zoom syncing in render view. I will make some changes suggested by @neurolabusc to the updated bidirectional sync demo and the PR should be ready to go.

@hanayik
Copy link
Member

hanayik commented Jul 3, 2024

@helghast79 , this should be fixed in version 0.43.6 which is now available on NPM. Let us know if you have any more issues.

@helghast79
Copy link
Author

helghast79 commented Jul 3, 2024

Seems perfect to me! Many thanks @hanayik, @cdrake and @neurolabusc

version 0.43.6

0.43.6.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
4 participants