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

If the wavesurfer container has a very large width, only one canvas will be rendered. #3682

Closed
Johnrobmiller opened this issue May 1, 2024 · 5 comments · Fixed by #3685
Closed
Labels

Comments

@Johnrobmiller
Copy link
Contributor

Johnrobmiller commented May 1, 2024

I replicated this on a forked version of wavesurfer where I ran npm run start to start the local examples.

If the body, i.e., the wavesurfer's container, is given a very large width (e.g., 70000px), then the wavesurfer will only create one single canvas that is 70000px large. The expected behavior is that is creates lots of smaller canvases, such as 70 1000px canvases.

It also seems that this bug replicates when fillParent is both true and false.

I am currently fixing this on my fork and could contribute to wavesurfer by providing some code when I figure out what the fix might be.

@Johnrobmiller
Copy link
Contributor Author

EDIT: I noticed that this also replicated when I passed in a very large width in the wavesurfer options. The conditions in which the renderer will make only one canvas are a bit tricky to pinpoint.

@katspaugh
Copy link
Owner

Looking at the code it should always cap a single canvas width to 4K px max.

https://github.com/katspaugh/wavesurfer.js/blob/main/src/renderer.ts#L544

So if the total visible width is 7k, it should create 2 canvases. Not sure why it’s not doing that.

@Johnrobmiller
Copy link
Contributor Author

Johnrobmiller commented May 1, 2024

Thankfully, there is only a one line change needed to replicate.

In renderer.ts, add options.width = 100000 on line 39. Then, test on the zoom example.

Here is the code for more context:

  constructor(options: WaveSurferOptions, audioElement?: HTMLElement) {
    super()

    this.subscriptions = []
    this.options = options
    options.width = 100000

EDIT: I notice that it is marked as not scrollable when it probably should be, as shown in the console.logs in the screenshot. I also see that renderSingleCanvas only fires once if isScrollable is false. This seems like it's probably the issue here?

image



EDIT 2: Yes, that seems to be correct. If I comment out the highlighted code in the screenshot, then the issue is fixed and the canvas widths are the max canvas width.

image

@Johnrobmiller
Copy link
Contributor Author

@katspaugh PR incoming: #3685

I think this fixes the issue. However, I don't know how to run the e2e tests, and I'm not familiar with the code so there might be a better solution.

@katspaugh
Copy link
Owner

Thank you! 🙏
I’ve released v7.7.12 with your fix.

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

Successfully merging a pull request may close this issue.

2 participants