biocore / biocore/empress

Only call drawTree() once when changing the layout

Open
#367 0 comments 0 reactions 0 assignees View on GitHub
performance
Dominant language
JavaScript
Stars
56
Forks
32
PR merge metrics
No merged PRs in 30d

Description

Brought up in https://github.com/biocore/empress/pull/363, although this has been a TODO in the code for a while now:

`Empress.reLayout()` calls `Empress.drawTree()`. After a call to `reLayout()`, in `Empress.updateLayout()`, `Empress.centerLayoutAvgPoint()` is called -- which calls `drawTree()` again. This represents a slight inefficiency.

There are cases where `reLayout()` is called by itself where we want to draw the tree afterwards without changing the camera position (e.g. when toggling ignore lengths vs. don't ignore lengths), and there are cases where `centerLayoutAvgPoint()` is called by itself (e.g. when clicking the "Reset camera" button).

Solutions to this include:

- remove `drawTree()` calls from both of these functions and just make the caller call `drawTree()` themselves if needed
- add boolean parameters to these functions that control whether or not to call `drawTree()`
- probs other things

Contributor guide

Open the contributing guide

Research direction

Trace Empress.reLayout(), Empress.updateLayout(), and Empress.centerLayoutAvgPoint() to understand which callers need rendering and which only adjust the camera. Choose an approach that preserves standalone reLayout() and Reset camera behavior while ensuring a layout change calls drawTree() only once; verify the relevant interaction paths manually.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend, performance
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.