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

Transition interruption not behaving as expected #201

Open
dbuezas opened this issue Sep 5, 2021 · 5 comments
Open

Transition interruption not behaving as expected #201

dbuezas opened this issue Sep 5, 2021 · 5 comments
Labels

Comments

@dbuezas
Copy link

dbuezas commented Sep 5, 2021

Steps to reproduce:

  1. Render a dot file (first)
  2. Transition into a different dot file (second) rendering
  3. Interrupt transition before it finishes
  4. Transition again into a third dot file

Expected behaviour

  1. The third transition should begin right away and from the state the svg is at the moment of the interruption

Current behaviour

  1. The second transition is interrupted but there is a delay (as big as the remaining transition time)
  2. Then the third transition takes over (choppy)

Kapture 2021-09-05 at 11 17 29

My rendering code

      selectAll([container]).selectAll("*").interrupt("reload");
      const t = transition("reload").ease(easeCubic).duration(3000);
      graphviz(container)
        .zoomScaleExtent([0, Infinity])
        .zoom(true)
        .tweenShapes(true)
        .convertEqualSidedPolygons(false)
        .growEnteringEdges(true)
        .tweenPaths(true)
        .fade(true)
        .tweenPrecision("25")
        .transition(t as any) // @TODO: no clue why there is a type problem there
       .renderDot(dot);
@magjac
Copy link
Owner

magjac commented Sep 5, 2021

Thanks for the report.

In order to understand what the problem is I would need to see the code for the full sequence including the first rendering. That said; interrupting a transition is not something I've tested or even considered so you may well have found a real bug.

@dbuezas
Copy link
Author

dbuezas commented Sep 5, 2021

Got it.

I suspect something is waiting for the end event in the transition to happen. Calling element.__transition[group].timer.call(duration) on all elements does make the next transition start right away (but does jump to the end of the previous transition).

I'll make a test repo soon then :).

Thanks for this awesome module btw, I've been using it for years, great work!

@dbuezas
Copy link
Author

dbuezas commented Sep 6, 2021

Demo

Ok, here is the minimalistic demo:
https://bl.ocks.org/dbuezas/cc9e5ec4df29b9f436ce2755b6a54fbf

@dbuezas
Copy link
Author

dbuezas commented Sep 6, 2021

Hack-fix

For what is worth, I played a bit with the internals and found a way to "fix-hack-it". I hope it helps finding out what the problem is:

what it does

  1. on interrupt, go through all paths and put their current d attribute in their data. This ensures the next transition starts interpolating from the current path shape instead of what it would have become after the transition finishes.
  2. do something analogous with the transform of the svg container element.
  3. call the "end" event on the container. This removes the delay between transitions (the delay is as big as the remaining time of the transition if it wouldn't have been interrupted)

this way, when one calls interrupt on the selection, it is actually interrupted mid way as it should.

const t = transition("reload")
        .ease(easeCubic)
        .duration(3000)
        .on("interrupt", () => {
          document.querySelectorAll("#graph *").forEach((node: any) => {
            if (node.__data__?.attributes?.d && node.getAttribute("d"))
              node.__data__.attributes.d = node.getAttribute("d");
            if (node.__zoom) {
             
              const match = node.firstElementChild
                .getAttribute("transform")
                .match(/(-?[\d.]+)/g);
              if (match) {
                const transform = match.map(Number)
                if (transform[0]!==undefined)node.__zoom.x = transform[0];
                if (transform[1]!==undefined)node.__zoom.y = transform[1];
                if (transform[2]!==undefined)node.__zoom.k = transform[2];
              }
            }
          });
          const node = container as any;

          var slots = node.__transition;
          for (const self of Object.values(slots) as any) {
            self.on.call("end", node, node.__data__, self.index, self.group);
          }
        });

interrupt call

selectAll([container]).selectAll("*").interrupt("reload");

@magjac
Copy link
Owner

magjac commented Sep 7, 2021

Thank you so much for this. I'll look into it as soon as I can carve out some time and mental space for it.

@magjac magjac added the bug label Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants