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

Avoid copying element JSON in `ele.move()` #1459

Closed
maxkfranz opened this issue Jul 18, 2016 · 0 comments

Comments

Projects
None yet
1 participant
@maxkfranz
Copy link
Member

commented Jul 18, 2016

Issue type

Feature request

Description of new feature

When calling ele.move(), it would be nice if the original element were edited in-place instead of removing the original element and adding a modified copy.

Motivation for new feature

  • Performance: Avoid copying of element JSON.
  • Prior refs of ele are the same after a call to ele.move().
  • Because the element is not actually removed from the graph, it can be moved mid-gesture. For example, if you call ele.move() on a grabbed node, the user loses hold of the node.

@maxkfranz maxkfranz added this to the future milestone Jul 18, 2016

@maxkfranz maxkfranz self-assigned this Jan 24, 2019

@maxkfranz maxkfranz modified the milestones: future, 3.4.0 Jan 24, 2019

@maxkfranz maxkfranz changed the title Improve performance of eles.move() by avoiding copying Avoid copying element json in `ele.move()` Jan 24, 2019

@maxkfranz maxkfranz changed the title Avoid copying element json in `ele.move()` Avoid copying element JSON in `ele.move()` Jan 24, 2019

maxkfranz added a commit that referenced this issue Jan 25, 2019

Avoid copying element json in `ele.move()` #1459
- Add `addToPool` and `removeFromPool` options (for private use only) to `eles.restore()` and `eles.remove()`.  The signature is now like this: `ele.remove(notifyRenderer, removeFromPool)`, so calls with `(false, false)` arguments allow all of the remove and restore cleanup to be applied to elements that aren't actually removed from the graph --- and without notifying the renderer or emitting `remove` or `restore` events.
- This new internal feature allows for `eles.move()` to modify existing elements in-place rather than creating modified copies.
- Batching is used to avoid redundant style updates as a remove-restore pair would normally do.
- Add a `move` event for when `ele.move()` is called.
- Update the z-order sorting in the base renderer when a `move` notification is received.  This only need to happen in compound graphs. because moving edges does not influence the z-ordering.
- Update docs for `eles.move().
- Add docs for `move` event.
- Aside: Remove redundant collection creation within `remove()`.
- Aside: Clean up z-ordering code a bit in the base renderer.
- Aside: When removing parent=>child ref in `remove()`, the child=>parent ref should also be removed.

@maxkfranz maxkfranz closed this Jan 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.