Improve rendering performance when pxPerSec is high #909
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Adds an optional 'partialRender' parameter to enable * Calculates and renders peaks only for current visible waveform * Keeps track of currently calculated/rendered peaks to avoid duplicate calculation and only incremental scroll changes are rendered Tested all combinations of Canvas/MultiCanvas and Wave/Bars rendering at various zoom levels.
katspaugh
approved these changes
Jan 16, 2017
|
Thanks a lot! Sorry it took so long to merge! |
|
Awesome work! Thanks for adding this. Is this gonna make its way into the npm version? Working in nodejs and have to work using PC and Mac so need to be able to update via npm. |
|
I'm waiting for feedback in the last open PR and will release afterwards. |
|
ok, thanks. |
|
Question on this. Should this resolve an issue I have where the waveform disappears when the pxsPerSec value is above 260? Using zoom(270) |
|
(wrote this before your edit) I think the waveform disappearing with zooming is a limitation of single canvas. Probably the internal representation of your canvas is > 4000px or whatever the size limitation of canvas is on your particular browser's/gpu's implementation. This PR will not fix that, because the canvas allocation will still fail. You should try multi-canvas to address this, but now this PR will mean that you won't over-render to the whole multi-canvas when the option is set. I've used zoom levels well above 270 in my test app, I think into the thousands. IMO multi-canvas should be the default and you should consider removing single-canvas because it's strictly a subset of the functionality of multi-canvas. |
|
I've made MultiCanvas the default renderer and published version 1.3.0 on npm. Thanks everyone! |
mspae
pushed a commit
to mspae/wavesurfer.js
that referenced
this pull request
Jan 20, 2017
* Adds an optional 'partialRender' parameter to enable * Calculates and renders peaks only for current visible waveform * Keeps track of currently calculated/rendered peaks to avoid duplicate calculation and only incremental scroll changes are rendered Tested all combinations of Canvas/MultiCanvas and Wave/Bars rendering at various zoom levels.
thijstriemstra
pushed a commit
that referenced
this pull request
Jan 27, 2017
…is high (#909) (#924) * Improve rendering performance when pxPerSec is high (#909) * Adds an optional 'partialRender' parameter to enable * Calculates and renders peaks only for current visible waveform * Keeps track of currently calculated/rendered peaks to avoid duplicate calculation and only incremental scroll changes are rendered * Tested all combinations of Canvas/MultiCanvas and Wave/Bars rendering at various zoom levels. * travis with prune for more accurate dependency resolution * updated karma-webpack to work with webpack 2
tf
added a commit
to tf/wavesurfer.js
that referenced
this pull request
Jun 19, 2017
refs katspaugh#1127 Before katspaugh#909, calling `drawBuffer` to redraw the wave invoked `drawer.drawPeaks` which in turn invoked `drawer.setWidth` which always caused the canvas to be cleared as a side effect of `drawer.updateSize`. In katspaugh#909 `setWidth` was changed to short circuit if the width did not change. This now causes the waveform to be redrawn on top of the previous rendition, making the edges of the wave appear less crisp. This change makes `setWidth`/`setHeight` return a boolean to indicate whether changes were needed. If not, `drawer.clearWave` is called manually. So we make sure that the previous wave is always cleared, but do not perform the possibly performance intensive task of clearing the canvas twice if it already happened as a side effect of `setWidth`.
katspaugh
pushed a commit
that referenced
this pull request
Jul 2, 2017
refs #1127 Before #909, calling `drawBuffer` to redraw the wave invoked `drawer.drawPeaks` which in turn invoked `drawer.setWidth` which always caused the canvas to be cleared as a side effect of `drawer.updateSize`. In #909 `setWidth` was changed to short circuit if the width did not change. This now causes the waveform to be redrawn on top of the previous rendition, making the edges of the wave appear less crisp. This change makes `setWidth`/`setHeight` return a boolean to indicate whether changes were needed. If not, `drawer.clearWave` is called manually. So we make sure that the previous wave is always cleared, but do not perform the possibly performance intensive task of clearing the canvas twice if it already happened as a side effect of `setWidth`.
mspae
pushed a commit
to mspae/wavesurfer.js
that referenced
this pull request
Aug 18, 2017
refs katspaugh#1127 Before katspaugh#909, calling `drawBuffer` to redraw the wave invoked `drawer.drawPeaks` which in turn invoked `drawer.setWidth` which always caused the canvas to be cleared as a side effect of `drawer.updateSize`. In katspaugh#909 `setWidth` was changed to short circuit if the width did not change. This now causes the waveform to be redrawn on top of the previous rendition, making the edges of the wave appear less crisp. This change makes `setWidth`/`setHeight` return a boolean to indicate whether changes were needed. If not, `drawer.clearWave` is called manually. So we make sure that the previous wave is always cleared, but do not perform the possibly performance intensive task of clearing the canvas twice if it already happened as a side effect of `setWidth`.
mspae
pushed a commit
that referenced
this pull request
Aug 19, 2017
refs #1127 Before #909, calling `drawBuffer` to redraw the wave invoked `drawer.drawPeaks` which in turn invoked `drawer.setWidth` which always caused the canvas to be cleared as a side effect of `drawer.updateSize`. In #909 `setWidth` was changed to short circuit if the width did not change. This now causes the waveform to be redrawn on top of the previous rendition, making the edges of the wave appear less crisp. This change makes `setWidth`/`setHeight` return a boolean to indicate whether changes were needed. If not, `drawer.clearWave` is called manually. So we make sure that the previous wave is always cleared, but do not perform the possibly performance intensive task of clearing the canvas twice if it already happened as a side effect of `setWidth`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
duplicate calculation and only incremental scroll changes are rendered
Tested all combinations of Canvas/MultiCanvas and Wave/Bars rendering
at various zoom levels.