mapbox-gl-js icon indicating copy to clipboard operation
mapbox-gl-js copied to clipboard

Make "devicePixelRatio" user-configurable

Open kristfal opened this issue 9 years ago • 14 comments

Hey,

Summary:

On (mobile) devices with very high ppi, render at a lower resolution than the device's native ppi and upscale it.

Long story:

Mapbox-gl-js fares very well on iOS devices performance wise (with #1606).

The story is a bit different on Android devices.

The most interesting findings we've had so far is that a Samsung Galaxy S3 (Released March 2013) is roughly 10% slower FPS wise than a Samsung Galaxy S6 (Released Q2 2015) under similar circumstances.

The GL draw calls take roughly equal time on the S3 and S6, despite a much faster CPU and GPU on the S6. The reason for the S6's lack of performance over the S3 seems to be the newer device's PPI. The S6 has a PPI of 577 vs 306 for the S3, and as a ref, 326 for iPhone 6.

This pattern repeats itself for almost all new Android devices, and the race is still going to 800 ppi and beyond. Android screen resolution is often 'unnecessary' high, which in turn leads to poor web-gl performance.

It is possible to downscale the rendering of mapbox-gl-js in html/css without touching the internal js code, but it also requires highjacking the touch / mouse handlers and scale down symbols and text in the style to make it work properly. We're following this approach, and do downscaling to an effective 326 ppi on any device with a higher native ppi. In the case of the S6, we're seeing an effective 3.5x increase in performance without any notices from users.

This workaround is probably outside the scope of your average mapbox-gl-js user. For gl-js not to be a performance hog on mobile (android) devices by default, device PPI should probably be handled.

kristfal avatar Jan 20 '16 11:01 kristfal

That's a valid feature request. Want to look into turning the suggestion into a pull request? I wonder how easy it is to make the ratio configurable as opposed to always on based on the screen.

mourner avatar Jan 20 '16 11:01 mourner

@mourner I was leaning towards 'max PPI / devicePixelRatio' option in the map config that would downscale only if the threshold was surpassed, but do think it would be better to add a 'target PPI / devicePixelRatio' option that would force a ratio regardless of native screen spec?

I haven't done any changes in the mapbox-gl code yet, just hacks in the app containing the map, but I'll look into it – probably starting this weekend.

kristfal avatar Jan 20 '16 12:01 kristfal

Hi,

we are using mapbox gl js for both android and iOS devices and we just want to add a huge +1 on this feature request. Sometimes the map is visibly struggling to keep up on our Androids and a 3.5x or even a 1.5-2x speed increase would transform the user experience into "why is this app so slow" into "this thing is great!".

@kristfal any pointers on what is needed to accomplish this? Thanks!

[UPDATE] Apparently it's enough to change both instances of "window.devicePixelRatio" to 1 (or perhaps 1.5? Do fractional values work?) and the map will automatically be shown in a lower resolution, without any artifacts (that I can see). Please correct me if I'm doing something wrong. The performance seems higher now, but unsure how much exactly. So far 1.5 seems to be the sweet spot between quality and performance.

SandroGrzicic avatar Jan 29 '16 10:01 SandroGrzicic

Is this solution one that's still being considered?

wprater avatar May 02 '17 00:05 wprater

@mourner @lucaswoj, why has this not been addressed after being open for 2 years? Are you still seeking an external contribution for this to get done?

psyrendust avatar Feb 28 '18 18:02 psyrendust

Our sincere apologies @psyrendust. To be honest this issue hasn't come up often enough to hit the top of our todo list. We are definitely open to an external contribution.

lucaswoj avatar Feb 28 '18 19:02 lucaswoj

@lucaswoj thanks for the quick reply. I'll see if I can get some time to do a pr for this. Off the top of your head, are there a lot of touch points in the code that this would affect?

psyrendust avatar Feb 28 '18 21:02 psyrendust

No problem @psyrendust. As best I can tell, the PR should be straightforward.

  • [ ] Add a devicePixelRatio setting to config.js which defaults to browser.devicePixelRatio
  • [ ] Add a devicePixelRatio getter and setter to index.js
  • [ ] Change all uses of browser.devicePixelRatio to config.devicePixelRatio
  • [ ] 🍰

lucaswoj avatar Feb 28 '18 21:02 lucaswoj

Copied from https://github.com/mapbox/mapbox-gl-js/pull/9014:

The problem with devicePixelRatio is that it's a global property that could also affect other maps on the same page. Instead of messing with devicePixelRatio, we could introduce a pixelRatio property on the map constructor that defaults to devicePixelRatio, but that also allows passing in other properties to render high resolution maps. We could also seed its initial value based on el.clientWidth / el.getBoundingClientRect().width.

kkaefer avatar Dec 04 '19 11:12 kkaefer

I'm handling custom devicePixelRatio like this:

Object.defineProperty(browser, 'devicePixelRatio', {
  get () {
    return window.devicePixelRatio * scale;
  }
});

Here's details: https://github.com/mapbox/mapbox-gl-js/pull/9014

And here's it in use: https://aviamaps.com/map (click print button to see map scaled)

pakastin avatar Feb 18 '20 07:02 pakastin

I’d like to take a stab at implementing the pixelRatio property on the map constructor, if you’d accept that as a PR.

We’re looking to get consistent pixel ratios when using toDataURL on the Mapbox canvas, regardless of the screen that the browser is running on.

fionawhim avatar May 13 '20 20:05 fionawhim

+1 for this. We are seeing major performance issues when running on a 27inch 5k screens - almost unusably slow.

danielwhatmuff avatar Aug 14 '20 12:08 danielwhatmuff

Any update?

leonardoviada avatar Feb 06 '23 16:02 leonardoviada

Any updates on this? That would be very nice feature!

beniaminrychter avatar Aug 06 '24 11:08 beniaminrychter

Also interested by that feature for a similar workflow, letting user screenshot the mapbox view via bubkoo/html-to-image.

Currently, the export is cropped when using a devicePixelRatio other than 100%, because eg:

  • map canvas width=1920px
  • vs map canvas cssStyleWidth=1280px
  • with a devicePixelRatio=1.5 (windows display scale to 150%)

See current implementation here https://github.com/mapbox/mapbox-gl-js/blob/fe307ba60527f20e4eca151a426bfc5895714c93/src/ui/map.ts#L4170-L4180

jo-chemla avatar Jul 21 '25 15:07 jo-chemla