Page MenuHomePhabricator

Kartotherian snapshots always return low DPI images
Closed, ResolvedPublic

Description

Self-explanatory. The image URLs appear to allow different DPI ratios, but this doesn't actually cause any display changes (default, with 2x parameter).

Event Timeline

Jc86035 created this task.Dec 2 2016, 11:30 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 2 2016, 11:30 AM
Yurik renamed this task from Kartographer's <mapframe> thumnails are not pixel-doubled to Kartotherian snapshots always return low DPI images.Dec 8 2016, 4:19 AM
Yurik moved this task from All map-related tasks to Kartotherian on the Maps board.
Yurik edited projects, added Maps (Kartotherian); removed Maps.
Jhernandez removed Yurik as the assignee of this task.Jul 10 2018, 4:48 PM
Jhernandez triaged this task as High priority.
Jhernandez added a project: Maps-Sprint.
Jhernandez added a subscriber: Jhernandez.

Reflecting reality

@MSantos This might be a good next maps task for when you're done with the one you're working on now.

MSantos claimed this task.Jul 17 2018, 5:17 PM
MSantos moved this task from Backlog to To-do on the Maps-Sprint board.

I found the problem, still looking for the solution. The problem is that the scale is overridden with the value 1 everytime with no conditional, see the following code:

// Overlays only support 2x scaling, so if scale is less than <1.5x, drop to 1x, otherwise - 2x
params.scale = (!params.scale || params.scale < 1.5) ? 1 : 2;

// Abaculus(?) doesn't position images with scale != 1
params.scale = 1;

Debugging Abaculus I could see that there are some calculations not working properly in the function abaculus.tileList when trying to find the tiles to stitch, it returns an empty array for the tiles to be rendered. Fun math is coming.

This task needed some fixes on abaculus and snapshot libraries, the PR's are listed below:

Mholloway moved this task from To-do to Needs review on the Maps-Sprint board.Jul 20 2018, 8:42 PM

After some review from @Mholloway was identified that we are stuck on the older version of abaculus. And updating the upstream code would force us to go ahead with T188674: Upgrade node-mapnik to 3.7.x.

I am considering this task stalled so we can have a discussion about what Michael pointed out in the PR topic:

So, unless we're ready to upgrade mapnik soon, we might have to publish this fork and begin using it for the scaling fix.

Update:
Waiting for upstream review to fix abaculus tile size calculations.

MSantos added a comment.EditedAug 9 2018, 2:27 PM

Update
Forked from upstream and now it will be published under kartotherian's organization.

What's next?

  • Publish to npm
  • Update Kartotherian/Snapshot

I visited the two links in the description and I'm still seeing the images with the same size on my low dpi monitor. Is that how we can check if it is fixed?

Actually, this task should be moved to To Deploy column, I found yesterday I haven't published @kartotherian/snapshot and now it will need another deploy to have this change on the beta cluster.

But, this fix won't show up on the links of the description until we migrate to Stretch and I am not sure if the beta cluster has an exposed HTTP port that you can see from the browser. For this fix, we needed to upgrade node-mapnik with the latest version of abaculus and because of that, we are unable to deploy it on Jessie machines.

After I deploy it to beta cluster you might need to do the following:

  • Connect to deployments-maps04 and tunnel port 6533

ssh -L 6533:localhost:6533 deployment-maps04.deployment-prep.eqiad.wmflabs

  • Access the links below for testing the existing data on the machine (Palestine):

http://localhost:6533/#8/31.494/35.261
http://localhost:6533/img/osm-intl,8,31.494,35.261,250x250.png
http://localhost:6533/img/osm-intl,8,31.494,35.261,250x250@2x.png

Mholloway added a comment.EditedAug 17 2018, 8:24 PM

Maybe now would be a good time to switch maps-beta.wmflabs.org from deployment-maps03 to deployment-maps04. Then we could verify further updates easily over HTTP. I'll do that later today.

@Jhernandez this ticket is now ready to sign-off

Looks like this is working on beta. Note that we won't be able to deploy to production until the production upgrade to Stretch (T198622) is complete. I'd recommend keeping the ticket open until then.

Nice, added that task as a subtasks (blocks this one). When it is done, we can resolve this one. Moving to the blocked column.

MSantos closed this task as Resolved.Apr 12 2019, 4:29 PM