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

"brushing on ordinal bar chart" example row chart animation is very slow #1860

Open
gordonwoodhull opened this issue Feb 18, 2022 · 4 comments

Comments

@gordonwoodhull
Copy link
Contributor

After brushing on the ordinal bar chart in the example, the row chart takes more than two minutes to stop moving.

There is nothing obvious in the code that would cause this. The row chart initialization is as simple as can be:

        var row = new dc.RowChart('#first-letters');
        row
            .width(1000)
            .height(350)
            .gap(1)
            .dimension(letterDimension)
            .group(letterGroup);

There must have been a change to D3 that caused this.

@kum-deepak
Copy link
Collaborator

The same chart when done with the alternate approach does not show the lag. I checked

https://dc-js.github.io/dc.js/examples/brush-ordinal.html - does not show lag
https://dc-js.github.io/dc.js/examples/brush-ordinal-dynamic.html - shows lag

@gordonwoodhull
Copy link
Contributor Author

Wild!

@Spacarar
Copy link

I thought I would take a look out of curiosity and I did notice that this line seems to severely affect performance, and when taking it out, I'm not immediately seeing any errors or issues.

https://github.com/dc-js/dc.js/blob/develop/web-src/examples/brush-ordinal-dynamic.html#L120

bar.on('preRedraw', chart =>  chart.rescale());

What is this line for and can it be removed?

@gordonwoodhull
Copy link
Contributor Author

Thanks @Spacarar! Hmm, this line is indeed the culprit, but it was put in to solve #1770.

The bar chart needs to be rescaled when it is redrawn in order to get the tick labels right when the row chart is filtered. But this somehow causes a large number of brush events which in turn cause more redraws when the bar chart is filtered. Unintended recursion of events, perhaps.

An extremely ugly workaround is to use

        row.on('filtered', () => bar.rescale());

instead. But having to add this to every other chart is a drag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants