Skip to content

Faster trace visibility toggling - #2837

Closed
etpinard wants to merge 9 commits into
faster-axis-autorangefrom
faster-trace-visible-toggle
Closed

Faster trace visibility toggling#2837
etpinard wants to merge 9 commits into
faster-axis-autorangefrom
faster-trace-visible-toggle

Conversation

@etpinard

Copy link
Copy Markdown
Contributor

... by changing visible from a 'calc' to a 'plot' edit type and using a "visible batch" in the scattergl code, similar to how selections are currently handled.

Graphs will slow calc steps will benefit the most. For example, on a 1e6 scattergl graph,

Plotly.restyle(gd,'visible',false)setTimeout(()=>{console.time('1')Plotly.restyle(gd,'visible',true)console.timeEnd('1')},500)

clocks in at about 30ms which is about 50x faster than on master 🐎

Eventually, we could augment this PR by adding a new edit type that would bypass drawFramework and initInteractions (which are very 🐢 for sploms), but I'll leave that for later if ok.

Note that commits 3790734 and b816a8e might be a bit controversial and may have some side-effects during subplot/trace removal the spli through our tests, so I wouldn't mind a second opinion there.

I would appreciate if @dy could take a look at e85725a and cf65abd


To be merged in #2823

cc @alexcjohnson and @dy

- DRY scalar-to-array repeat option logic
- DRY get-viewport logic
- build scene viewport and range option once per scene (not per trace)
- so that gl-based trace can call their plot methods w/ an
empty array of traces and just work.
- this makes restyle(gd, 'visible', false) work properly for
scattergl traces
... to plot toggle scattergl 'visible' w/o having editType 'calc',
and hence w/o having to rebuild to scene options from scratch.
- to not have to guard against visible!==true traces in _module.style
- to shortcut full list of modules in other places downstream
}
}

Plotly.plot(gd, fig).then(function() {

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is broken on master, consider this a bug fix.

@alexcjohnson

Copy link
Copy Markdown
Collaborator

Love the concept! But it looks like autoranged axes aren't updated with the faster logic. I'll hold off reviewing until that's sorted out as I suspect it's going to be a bit involved - likely by maintaining separate _min and _max for each trace? We've been talking about doing that for a while anyhow.

autorange on visibility

@etpinard

Copy link
Copy Markdown
ContributorAuthor

But it looks like autoranged axes aren't updated with the faster logic

... and looks like we don't have any tests covering these cases 😑

@etpinard

Copy link
Copy Markdown
ContributorAuthor

Abandoned. See #2860

@etpinard
etpinard deleted the faster-trace-visible-toggle branch July 31, 2018 19:38
@etpinardetpinard mentioned this pull request Sep 25, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@etpinard@alexcjohnson