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

Canvas Source Performance Increase #5448

Closed
wants to merge 4 commits into from
Closed

Canvas Source Performance Increase #5448

wants to merge 4 commits into from

Conversation

gpbmike
Copy link
Contributor

@gpbmike gpbmike commented Oct 11, 2017

A recent fix for GPU related bug introduced a large performance hit to animated canvas sources. The intermediary step of copying data to an ImageData object, particularly with large canvases (my use case is 3000x3000), caused an unacceptable frame drop for my users.

This PR removes the intermediate step. The result is a dramatic increase in performance, back to 60fps.

I have confirmed that the change works with previously affected broadwell line GPUs referenced here: #4262

contextType is no longer needed when creating a canvas source.

Tests do not need an update since there is not a good way to test rendering at the moment (see #5303).

I appreciate all the effort that was put into confirming and working around the GPU issue when it was first presented. It would seem as though the work around is not needed anymore. I don't know if that's the result of a change somewhere else in the mapbox codebase or something else entirely.

Happy to answer questions.

Launch Checklist

  • briefly describe the changes in this PR
  • document any changes to public APIs
  • manually test the debug page

fixes #5301

@lbud
Copy link
Contributor

lbud commented Oct 11, 2017

I'm mystified by what fixed this on affected GPUs, but not having easy access to one myself I'm going to have to leave that mystery for someone else. @gpbmike I'll take your word for it that those GPUs are fine now without the intermediary image buffer.

I checked out this branch and there are just a few minor tweaks to make to fully revert #5155. I hope you don't mind if I push to this branch myself in the interest of expediency so we can get this in the next release — we're just about to cut a new release, so you opened this in the nick of time :)

@lbud
Copy link
Contributor

lbud commented Oct 11, 2017

Moving to #5449.

@gpbmike
Copy link
Contributor Author

gpbmike commented Oct 11, 2017

Thanks @lbud 🙏

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

Successfully merging this pull request may close these issues.

WebGL CanvasSource performance drop in 0.40.0
2 participants