Cartodb: Implement high resolution basemaps in Builder

[makella]

Hello!

I am wondering if there are any plans to support, in Builder, detecting viewer resolution to serve retina basemap tiles?

There is a big difference in the quality of the map...and with vector rendering, having a crisp looking overlay and a fuzzy basemap, the difference between the two is even more apparent and in some cases the maps look like they don't belong together -- see this example.

Even without overlay, the differences are pretty big:

cc @jaakla @noguerol

[jaakla]

Some tech details:

• all basemap styles support "@2x.png" for retina
• in the preview page it is set with minimal code as following:

      layer = L.tileLayer('/'+style+'/{z}/{x}/{y}' + (L.Browser.retina ? '@2x.png' : '.png'), {
     ...

[noguerol]

Prioritizing it.

[makella]

thank you!

[elenatorro]

Hi :) I opened a PR with a fix for this issue. However, @matallo and I have been talking about this solution and there are several things to take it into account:

1. Now, retina basemaps and labels are only active for retina viewports. It could be that a user wants to force retina to any viewport, so maybe we've to consider to add this option as a feature (i.e: activate retina checkbox) This is also discused here https://github.com/CartoDB/cartodb.js/pull/364

2. This fix only affects basemaps and labels, not the layers in between.

I've added the do_not_merge_yet_ label to the PR so we can discuss first if this is rather a good solution or not and think about the points above.

Thanks!! :)

cc @noguerol @makella

[jaakla]

I think that both proposed points are valid and should be done also, but I would not hold having quick win and big improvement of retina basemaps.

Looking to PR it is not clear to me how retina/noretina is selected, I do not know what is app_config.yml - is it in general config or set somehow in browser, in client side? I expect it to be set in browser, as in the Leaflet code I use as I gave above:

layer = L.tileLayer('//cartodb-basemaps-{s}.global.ssl.fastly.net/rastertiles/voyager_labels_under/{z}/{x}/{y}' + (L.Browser.retina ? '@2x.png' : '.png'), {
...

Here L.Browser.retina is dynamically set property in Leaflet.js. There other ways to do same, but in any case basically browser should know both retina and no-retina URLs and uses suitable based on browser window state.

For example I usually have normal, no-retina screen connected to my macbook retina screen. With the client-side approach I can open browser window in external screen, then it is detected as no-retina, and tiles will be without @2x, now when I move same window to internal retina screen, and do page reload, then it is reconfigured in client side to retina, and otherwise blurry basemap becomes sharp.

[makella]

@elenatorro I'd agree with @jaakla that it would be a big win for us to have crisper basemaps on retina devices... especially with vector rendering. in the case where a user is able to have their data layer render vector (if it passes all the checks) it is looking really bad (on retina) with the vector layer and super rasterized basemap...

if the data layer renders vector and the basemap detects retina, at a minimum, we will provide good experiences for that combination of map.

i don't understand enough about why we can't have the data layer detect retina, but do you think graphically, it will look really bad for layers that aren't rendering vector?

[matallo]

there are several situations here:

• indeed there's a L.Browser.retina method to append the retina suffix to the baselayer url template that in any case should have to be done at cartodb.js level, the thing is: at the moment of making that check cartodb.js knows nothing about the url template, in your example you know because you hardcoded it, but for any other map setup with cartodb.js the beasemap could or could NOT have the corresponding retina versions, thus returning a 404 image if the "@2x" doesn't exist
• what we can do, however, is at builder time, setup that urlTemplate with the "@2x" suffix. The PRs is not ideal though, just to spark the discussion, as it would do so in the case the screen you are seeing that UI is retina, you wouldn't be able to do so with a normal screen

that regarding the basemaps (and labels layers) ^^^

• another story is the data layers, which have their own idiosyncrasy

this is an old friend (3 years old, actually), there has already been some attempt for this, but I don't see a straight forward way to do so but forcing the retina version with a setting at builder time

https://github.com/CartoDB/cartodb.js/pull/364 (closed PR)
https://github.com/CartoDB/Windshaft/issues/243 (open PR, not merged)

hope this helps clarify 🙏

[jaakla]

Yep, I imagine it is more complex than with simple manual JS. Still with the server-level configuration I cannot imagine case that there are some cartodb servers only with retina tiles, and others with normal ones. Also I don't see cases where end-user (if so, then which one - builder or consumer of map) wants to manually set it, it is even hard to know without knowing your device hardware specifics which map is good. I would put it completely automatic, at least for all the built-in basemaps. It should work in end-user browser window level, so it cannot be done in server side only.

What about this approach: basemap URLs have additional dynamic parameter, e.g. {r} (AFAIK there is no common standard here), which can get empty or "@2x" value in client side. Server configures it as normal value (if specific basemap supports it), and carto.js understands and replaces it based on L.Browser.retina. It probably does not require any server-side changes (unless there is some validation check), and it automatically also enables retina for customer-defined custom basemap URLs. So server config value would be //cartodb-basemaps-{s}.global.ssl.fastly.net/rastertiles/voyager_labels_under/{z}/{x}/{y}{r}.png' .

Limitation is that the @2x is not really official standard and some basemaps, e.g. HERE satellite has different value, they use /512/ as tile size URL parameter value for higher resolution tiles. For this case two URL config approach would be needed - client (JS side) will know both and dynamically choose suitable - would be more universal. But this requires server-side config changes and UI changes for custom basemaps (if we want to support it there too) - 2 URL fields instead of one. But if we ignore HERE map specifics now, then {r} looks optimal solution for me

[matallo]

I cannot imagine case that there are some cartodb servers only with retina tiles, and others with normal ones.

I must have explained myself wrong, it is at cartodb.js point where L.Browser.retina is used and retina suffix appended (or not). The thing is cartodb.js knows nothing about the origin of the url template, and this will work for our basemaps, and for the basemaps with the @2x version, but not for other basemap

take this as an example: you set https://stamen-tiles-{S}.a.ssl.fastly.net/watercolor/{z}/{x}/{y}.jpg urltemplate in the example html (or via custom basemap in Builder!) and cartodb.js will try L.Browser.retina and append the @2x suffix if there is a retina screen, but as the retina version of the tile won't exist no tile will load then

I like the approach you describe next, and we already foresaw the issue with HERE basemaps, thus the retina url in the config. I think this is the way to go. Moving towards this direction besides adding the new retina urltemplate to the params passed to cartodb.js we need to support detection of this new param at cartodb.js so when it exists (and the viewport is retina) it makes the replacement.

@elenatorro create (or reopen) the corresponding CartoDB/cartodb.js issue, and leave this for Builder tracking.

ps: this regarding the basemaps and labels layers, data layers are another story that should be discussed separately (probably the former linked issue is a better place)

[jaakla]

Yep, we cannot just add automatically @2x everywhere, this has to be defined explicitly, via URL pattern (which would cool as it enables custom urls and even data overlays quite easily, but has also limits) or double URL definition (which has own limits).

[elenatorro]

In order to explain a little bit better what the PRs is doing and how we detect if the device supports retina or not:

app_config.yml

| basemap_example |
| - url           | => we have this info
| - retina_url    |    in the client side
+-----------------+

• If the basemap supports retina, then it has a retina_url, that can follow any format:

http://basemap_example/{z}/{x}/{y}@2x.png

or

http://basemap_example/{z}/{x}/{y}/512/png8... 

As @jaakla mentioned, @2x is not a standard, and by doing this we are able to set the retina url for every basemap, depending on its configuration.

Our current solution works as follows:

1. We have the information of the basemap.
2. We detect the device supports retina.
3. If it does, then we check if the basemap has a retina url.
4. If the basemap has a retina_url, then we use this url to paint it. Otherwise, it makes use of url.

In this solution, we cover the case when you load the map in a retina screen. However, as @jaakla mentioned above, when a user is using two screens, one of them no-retina, if the user moves from the no-retina screen to the retina screen, we should detect that the viewport resolution has changed and then reload the map. Am I right?

I've been talking with cartodb.js team, and they told me that we support high resolution using the @2x notation. It means that we've to tell cartodb.js that the client supports retina, so they can add @2x to the tiles url. Just opened an issue about this https://github.com/CartoDB/cartodb.js/issues/1831

The next steps will be able to paint the tiles between the basemap and the labels also in high resolution if the viewport supports it.

[jaakla]

when a user is using two screens, one of them no-retina, if the user moves from the no-retina screen to the retina screen, we should detect that the viewport resolution has changed and then reload the map. Am I right?

This would be nice, but probably it is quite marginal case, only happens if user moves window from one screen to another; so I would not spend time for this detection; unless there is already some readymade mechanism for this, which does this already.

[alrocar]

Hey mates, let's move this on :)

Going back to the original issue, the point was having our basemaps looking nice on retina screens.

So let's tackle this with the PR sent by @elenatorro, worse than having a perfect solution for retina screens, but much better than have nothing.

[matallo]

agree! who is the owner of this (which team will it be assigned)?

[alrocar]

It's on Charlie's kanban.

@elenatorro are you available for taking a look? otherwise I can take it this week

Thanks! :)

[elenatorro]

I can take a look to this again this afternoon, but I would like to discuss it first with someone to see if there is something else we can change / add to the current PR.

[javitonino]

Some question/comments I would like to see answered before going forward with this:

• Are we also considering changing the tiler to support retina tiles in static images? Or are we doing only the cartodb.js part?
• Regarding backend: we have to do a rake to update all existing layers (and mapcaps). Something similar to update_nokia_layers in layers.rake. It will take a long while to run.
• We should review the way we write the config in order to be extensible (and avoid trademarked words as "retina")

[jaakla]

Are we also considering changing the tiler to support retina tiles in static images? Or are we doing only the cartodb.js part?

Where are static images used by us? In maps listing in Builder? Or you mean static maps API? In any case I would keep this out of scope from here, it would be nice but it looks small win and a lot of extra effort. ps. it reminds me another feature idea #11943.

Regarding backend: we have to do a rake to update all existing layers (and mapcaps). Something similar to update_nokia_layers in layers.rake. It will take a long while to run.

You can update URLs of existing maps? I would see it to be very useful for the url base fix, even more than for retina. There is enough time for that, not that many users would see the difference really.

We should review the way we write the config in order to be extensible (and avoid trademarked words as "retina")

Right, I think "dpi" or "resolution multiplier" could be correct generic tech terms for this. There are screens where @3x and @4x also make sense nowadays, but lets not go into that.

[javitonino]

You can update URLs of existing maps?

Yes, somewhat painful but we have done it before (HERE changed their URL format).

[matallo]

Are we also considering changing the tiler to support retina tiles in static images? Or are we doing only the cartodb.js part?

Just re-read the whole thread, the quick win is only support retina basemaps. Retina data layers is another big issue (that probably would better be addressed via vector maps)

Regarding backend: we have to do a rake to update all existing layers (and mapcaps). Something similar to update_nokia_layers in layers.rake. It will take a long while to run.

As I envision it, this would need to be done in three different phases:

1. support for retina basemaps in cartodb.js which when not existing, i.e. current and new maps case, it just defaults to the current situation
2. add config to Builder with new retina basemaps taking into account your point "We should review the way we write the config in order to be extensible"
3. run rake to existing maps when there is a basemap with the corresponding retina version just add the new field that will be supported by cartodb.js

about the "retina" nomenclature, I'm afraid it is quite an standard, I wouldn't focus much on this topic http://leafletjs.com/reference-1.3.0.html#tilelayer-detectretina

how does that sound? we can tackle the cartodb.js part at frontend level, and have at least a POC

[javitonino]

Sounds like a plan. We can take the opportunity with 2 and 3 to solve #13458 as well, to avoid having to do multiple passes updating all layers.

[jaakla]

How it would work in Builder UI level? For built-in basemaps there is no UI - just internally instead of 1 URL there is definition of 2 patterns. But for custom basemaps I see 3 options:
a) ignore it, just do not support automatic DPI. User can hardcode @2x to the URL if needed, then the map is "always retina"
b) Support additional placeholder, e.g. {r} which will be replaced by '' or '@2x' depending on DPI in browser JS level (carto.js?). Works for 90% of cases (e.g. our and mapbox maps). In fact Leaflet 1.0 already supports this placeholder: https://github.com/Leaflet/Leaflet/issues/4103 , maybe just Builder UI blocks this?
c) Allow to define two separate URLs. This gives more flexibility, can support both size parameter (some URLs have 256 and 512) and apple-style retina code '@2x' etc. But from UI there should be 2 custom URL fields then, even if second one is hidden under 'advanced' button) then it is significant change there.

[elenatorro]

Yesterday, @matallo and I worked on the solution you describe in option c).

Allow to define two separate URLs.

We did a PoC to test how it would work with custom basemaps and it works great. We will open a PR so we can discuss the soltuion, but this is basically what we've added:

cartodb backend

• add retina url and retina label url to app_config.yml (this was already changed )

labels:
  url: 'http://{s}.basemaps.cartocdn.com/dark_only_labels/{z}/{x}/{y}.png'
retina:
  url: 'http://{s}.basemaps.cartocdn.com/dark_nolabels/{z}/{x}/{y}@2x.png'
retina_labels:
  url: 'http://{s}.basemaps.cartocdn.com/dark_only_labels/{z}/{x}/{y}@2x.png'

• add retina url and retina label url in LayerFactory classes (app/model_factories/carto/layer_factory.rb and app/model_factories/layer_factory.rb) in order to return a urlTemplateRetina field along with urlTemplate.

cartodb.js

• check the screen resolution with window.devicePixelRatio and use urlTemplateRetina instead of urlTemplate if the resolution is > 1. (src/vis/layers-factory.js)

[matallo]

support for retina basemaps in cartodb.js

NEXT!

[javitonino]

I've been working on this and https://github.com/CartoDB/cartodb/issues/13458 as a unit.

Overview

• cartodb.js like in the PoC. I just renamed the attributes.
• I changed the config structure. See it https://github.com/CartoDB/cartodb/blob/e6103578f355fd29d78d9890bda930039a8a07a5/config/app_config.yml.sample#L390-L402. The idea is that the keys under labels are merged to the main hash on labels layers, so you can overwrite what is needed (usually just the urls, but maybe some day the min/max zoom for example)
• I deprecated url in basemap config. urlTemplate is used almost everywhere, and I changed the old code that used url (very few instances)

Acceptance note
Test editor and builder, with maps created before and after this patch.

Deployment plan

1. Merge https://github.com/CartoDB/cartodb.js/pull/2065 (this renames retina to 2x)
2. Merge https://github.com/CartoDB/cartodb-platform/pull/4094 (configuration changes)
3. Merge https://github.com/CartoDB/cartodb/pull/13644 (builder changes to use the new configuration)
4. Change the configuration again to remove deprecated url parameter
5. Write and run rake to fix existing layers

Please, @elenatorro can you review the general idea as well as the JS bits (PRs 1 and 3)?

[elenatorro]

I have seen several issues that we need to solve:

1. When you create a new map, it shows voyager with retina but without labels.
2. When you go to basemap options and change to a different basemap and then select voyager again, labels appear but neither basemap label nor labels layer have retina enabled.
3. Then, if you refresh the page, voyager basemap layer has retina enabled, but labels do not. (I think this is the expected behaviour since there is no "label" field for voyager basemap in app_config.yml, but I do not really know if there should be).
4. Then, if you select voyager labels-below basemap, again, it seems retina is not applied.
5. However, if you refresh the page, then both basemap and labels layers have retina enabled.

In conclusion, I have some doubts:

1. We are not applying retina when selecting a different basemap, and I guess this is something we have to check in Builder.
2. Do we need to add labels in app_config for voyager basemap?

I have to go deeper to understand why this could be happening, but I wanted to discuss it here because maybe I missed something or someone has a better idea about these issues :)

And also, just to remember point 3 from this comment: we will have to enable retina in existing maps.

[javitonino]

1. Did you use the correct cartodb.js branch? I did not bother changing it in package.json (lazy, I know)
2. Did you copy app_config.yml.sample basemap section into your app_config.yml?
3. From what you describe, it's likely that we are missing something in frontend side (backend only controls the initial map creation, the rest is all frontend). I tested and it worked (at least, the saved model had the required attributes), but I have not done complete acceptance (no retina screen here :P, I'd have to trick cartodb.js).
4. Yes, for existing basemaps we need to run a rake task (step 5).

[elenatorro]

Yes, I did 1 & 2 :) About 3, yes, we are missing something but as I said, I am almost sure it is related with Builder (frontend), cartodb.js is working as expected.

[javitonino]

My fault, I forgot to test some commits. We tested it after that and it works fine, except for one little issue. When changing the basemap, builder shows the low-res map until you F5. It seems like we only have the hdpi code for the layers factory in cartodb.js, so it only applies to newly created layers, but not to updated layers.

[javitonino]

This is done for new maps. I've created https://github.com/CartoDB/cartodb-management/issues/5144 for existing maps.

[alrocar]

Awesome team work!! 🎉 ❤️

[elenatorro]

🚀 VAMOS 🚀

[makella]

amazing!!