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

Use willReadFrequently optimization hint on 2D canvases. #1808

Merged
merged 1 commit into from
Nov 5, 2022

Conversation

kircher1
Copy link
Contributor

@kircher1 kircher1 commented Nov 3, 2022

When setting willReadFrequently to true, it is a hint to the browser that it can decelerate the canvas. This can be beneficial to perf if the canvas is only used for decoding and getting image data in raw pixels. If the canvas was accelerated (i.e. hosting data on the GPU) there could be a lot of latency to get the image data back to the main thread.

Perf-wise, I was able to isolate this function into a benchmark to show the improvement:

From chromium on a relatively fast machine (~8ms vs ~2ms):

image

In practice, I'm seeing this improvement is better for slower machines and less noticeable on faster machines.

On Firefox I saw no difference.

Using the hint also removes the source of a warning when using hillshade layer:

image

Launch Checklist

  • Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
  • Briefly describe the changes in this PR.
  • Link to related issues.
  • Post benchmark scores.
  • Manually test the debug page.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 3, 2022

Bundle size report:

Size Change: +36 B
Total Size Before: 206 kB
Total Size After: 206 kB

Output file Before After Change
maplibre-gl.js 196 kB 196 kB +36 B
maplibre-gl.css 9.09 kB 9.09 kB 0 B
ℹ️ View Details No major changes

@HarelM
Copy link
Member

HarelM commented Nov 4, 2022

Should this be configurable like preserveDrawingBuffer? I'm not sure I know what the impact of this change unfortunately...

@kircher1
Copy link
Contributor Author

kircher1 commented Nov 4, 2022

Based on A/B testing the change, enabling this willReadFrequently hint provides a slight reduction in startup time for our usage of MapLibre. It also reduces the potential for dropped frames: if getImageData() is called during map moving, the current, slower version could easily add a few millis of overhead which could be the difference between hitting a frame target or dropping a frame.

Local benchmarking has corroborated the speed improvement (eg previously attached image); it's either faster or neutral.

Plus, given that the existing usage pattern of the canvas in raster_dem_tile_worker_source is now triggering a warning in Chrome, getting rid of this warning spam is a benefit in and of itself.

Since it appears to be an improvement that should benefit everyone, I don't think a config option is necessary. If there were negative tradeoffs, then I agree with the config idea, but that hasn't been the case.

FYI, here's the original proposal for 'willReadFrequently' operation, with more explanation: https://github.com/fserb/canvas2D/blob/master/spec/will-read-frequently.md

@HarelM
Copy link
Member

HarelM commented Nov 4, 2022

What about the following: will it affect 3D in any way?

When the user sets willReadFrequently to true, the UA can optimize for read access, usually by not using the GPU for rendering.

@kircher1
Copy link
Contributor Author

kircher1 commented Nov 4, 2022

No, that would be very surprising, but I see your point given the ambiguity of that sentence :)

My understanding is that this hint only affects the 2D canvases. and when I apply it locally, I am still seeing GPU usage spike up in heavier MapLibre scenes, implying WebGL is still accelerated even if the 2D canvases are not.

Here's the documentation from Firefox, worded more clearly: https://developer.mozilla.org/en-US/docs/Web/API/HTMLCanvasElement/getContext

willReadFrequently
A boolean value that indicates whether or not a lot of read-back operations are planned. This will force the use of a software (instead of hardware accelerated) 2D canvas and can save memory when calling getImageData() frequently.

@kircher1
Copy link
Contributor Author

kircher1 commented Nov 4, 2022

In terms of exactly how the browsers implements this though, I can't speak with authority. My stance is based on what I've read and benchmarking/perf results.

@HarelM HarelM merged commit 6990586 into maplibre:main Nov 5, 2022
@HarelM
Copy link
Member

HarelM commented Nov 5, 2022

Sorry, forget to ask for a changelog entry, can you create a new PR to add that please?

kircher1 added a commit to kircher1/maplibre-gl-js that referenced this pull request Nov 5, 2022
HarelM pushed a commit that referenced this pull request Nov 5, 2022
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.

None yet

2 participants