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

Render tourism outlines one zoom level sooner for small zoos and theme parks #3648

Closed
wants to merge 1 commit into from

Conversation

jeisenbe
Copy link
Collaborator

@jeisenbe jeisenbe commented Jan 18, 2019

Partially reverts #2835

Changes proposed in this pull request:

  • From z13 to z16, render tourism area outlines when way_pixels > 180 instead of the current limit of 750
  • Removed redundant code that rendered thinner outlines when way_pixels < 60; this has not been used since Hide tiny zoos and theme parks #2835

Explanation:

Prior to #2835, all zoos and theme parks were rendered from z10 if the way_pixels was > 20. This meant that very small features, for example 5 pixels tall and 5 pixels wide, would render. Those with less than way_pixels between 20 and 60 were rendered with a thin 1 pixel border, which was difficult to see and looked quite different than the wide, double border used for larger areas (as seen now).

#2835 improved this situation by only rendering tourism area outlines when the waypixels was > 750. This is 1/4 of 3000 way_pixels, the number used to show the name of the area, so the outline would show exactly 1 zoom level sooner than the name label. Very small areas were hidden.

However, this caused some mid-sized areas to disappear; for example a 30 by 20 pixel area is hidden, even though this is 3 times larger than an icon. This looks strange when there are two tourism areas adjacent, a situation sometimes found in areas with more than one theme park or zoo next to each other (as see in Singapore, below). If one of the areas is slightly smaller, it could disappear a zoom level sooner than it's neighbor, leaving a gap in the map.

I initially considered rendering all zoos and theme parks from >=z13, because that is where all landcover colors and patterns are now shown. However, I found that it was not helpful to show very small areas less than 100 waypixels. The limit in this PR is now set to 180, which will cause small zoos and theme parks to render approximately 1 zoom levels sooner than before. This solves the problems without losing most of the benefits of #2835

Test renderings:

Sentosa Island, Singapore

z15 Current
z15-sentosa-master

z15 After
z15-sentosa-after

z16 (Same)
z16-universal-studios-660033

Jurong Bird Park, Singapore

z13 Current
z13-jurong-bird-park-master
z13 After
z13-jurong-bird-after

z14 (unchanged)
z14-jurong-bird-same

z15 (unchanged)
z15-jurong-660033

Singapore Zoo and Night Safari
https://www.openstreetmap.org/#map=14/1.4043/103.7957

z13 Current
z13-singapore-zoos-master
z13 After
z13-signapore-zoos-after

z14 (Same)
z14-singapore-zoos-same

Bangor, Northern Ireland
https://www.openstreetmap.org/#map=15/54.6637905/-5.6726521

  • "Pickie Fun Park" is long and narrow, so it's rather challenging to render with an outline

z13 current
z13-bangor-master
z13 after
z13-bangor-after

z14 (same)
z14-bangor-master

z15 (same)
z15-pickie-fun-park-master

Damhead Miniature Railway
https://www.openstreetmap.org/#map=17/55.1115842/-6.5977664

  • There are 4 or 5 of these railway theme parks in rural Northern Ireland, all are similar to this one.

z15 (Same)
z15-damhead-master

z14 Current
z14-damhead-master
z14 After
z14-damhead-after

z13 (same before and after)
z13-damhead-railway

[Before #2835 this would have been the rendering at z13:]
z13 Damhead railway rendering with 20 way_pixels limit:
z13-damhead-way_pixels-20-limit

Previously tourism boundaries (zoos and theme parks) only rendered at z17 or if waypixels were >750 from z10 to z16
This commit changes the limit to 180 waypixes at z13 and above, while from z10 to z12 the way_pixels (area) limit remains
Usually this means that small tourism=zoo and tourism=theme_park areas will render one zoom level sooner.
@polarbearing
Copy link
Contributor

Test renderings seem plausible.

Could somebody educate me where the way_pixels values are generated?
I found them only being used in this repo, but not defined.
I found a formula in #1094 and #703, and I searched cartoCSS and mapnik docs without success.

@kocio-pl
Copy link
Collaborator

Maybe it's a database thing (Postgres/PostGIS)?

@polarbearing
Copy link
Contributor

The pixel_height appears to come from PostGIS, but so far I did find neither way_area nor way_pixels being calculated. I would assume @pnorman could know it?

@matthijsmelissen
Copy link
Collaborator

way_pixels comes from project.mml, for example way_area/NULLIF(!pixel_width!::real*!pixel_height!::real,0) AS way_pixels.

pixel_width and pixel_height in turn come from Mapnik:
https://github.com/mapnik/mapnik/wiki/PostGIS#other-tokens

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 18, 2019 via email

@matthijsmelissen
Copy link
Collaborator

So way_area is the actual area of the polygon in e.g. square meters, and
way_pixels is approximately the area of the polygon in pixels, correct?

Correct! Forgot to mention, but way_pixels is calculated by osm2pgsql.

@imagico
Copy link
Collaborator

imagico commented Jan 18, 2019

Note way_area is in Mercator square meters, not in actual square meters.

For making styling decisions regarding label size etc. normalization with !pixel_width!/!pixel_height! is not ideal because it is not invariant w.r.t. rendering resolution. See #2524. Normalization with !scale_denominator! would make more sense.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 19, 2019 via email

@matthijsmelissen
Copy link
Collaborator

Yes, would be great if that works.

@matthijsmelissen
Copy link
Collaborator

I think the new smaller outlines look messy and not so clear; personally I'd prefer the current rendering.

@matthijsmelissen
Copy link
Collaborator

PR looks technically fine.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 24, 2019 via email

@jeisenbe
Copy link
Collaborator Author

I've tested the way_pixels calculation change.

The scale_denominator = map_width_in_metres/ (map_width_in_pixels * 0.00028m)

Because 0.28mm is the standardized pixel size. So at z17 scale_denominator=4265.4591677, which means that one standard pixel is approximately the same as one square meter at this zoom level (near the equator). See: https://github.com/openstreetmap/mapnik-stylesheets/blob/master/zoom-to-scale.txt

So to convert way_area to pixels, divide by 0.28mm squared [=0.0000000784 meters] multiplied by the scale_denominator squared.
EG:

way_area/NULLIF(.0000000784*POW(!scale_denominator!, 2),0) AS way_pixels

I've pushed a commit to the border-labels branch, i.e. PR #3652 after testing.

@imagico
Copy link
Collaborator

imagico commented Jan 26, 2019

Thanks - to make the formula better understandable i would formulate it as

way_area/NULLIF(POW(!scale_denominator!*0.001*25.4/90, 2),0)

(that is millimeter-to-meter*millimeter-per-inch/assumed-ppi)

And i would not use it selectively for some features but roll it out universally in those cases where way_area is used in relation to the map display scale.

@jeisenbe
Copy link
Collaborator Author

jeisenbe commented Jan 26, 2019 via email

@jeisenbe
Copy link
Collaborator Author

I tested with way_area/NULLIF(POW(!scale_denominator!*0.001*25.4/90, 2),0) but found that this is not precisely the same:

Current way_pixels calculation (based on pixel_width and pixel_height):

  • Way_pixels = 478440. ...
    boondina-waypixels-current

With way_area/NULLIF(POW(!scale_denominator!*0.001*25.4/90, 2),0):

  • Way_pixels = 470938. ...
    boondina- pow scale_denominator 0 001 25 4 90 2 0

Compared to way_area/NULLIF(POW(!scale_denominator!*0.001*0.28*, 2),0) AS way_pixels

  • way_pixels = 478443. ...
    boondina- 0000000784 pow scale_denominator 2

In fact the assumed "ppi" for mapnik is based on a 0.28mm pixel, so it should be approximately 90.71 standardized "ppi" (pixels per inch):

way_area/NULLIF(POW(!scale_denominator!*0.001*25.4/90.7, 2),0) AS way_pixels:

  • Way_pixels = 478292. ...
    boondina- pow scale_denominator 0 001 25 4 90 7 2 0

I'm not convinced that way_area/NULLIF(POW(!scale_denominator!*0.001*25.4/90.71, 2),0) is more intuitive than way_area/NULLIF(POW(!scale_denominator!*0.001*0.28, 2),0) or way_area/NULLIF(POW(!scale_denominator!*0.00028, 2),0)

@imagico
Copy link
Collaborator

imagico commented Jan 27, 2019

I am sorry - my bad. I just assumed mapnik - like almost all other programs converting between pixel and real world units - would assume a certain integer ppi (historically often 72, recently more often 90 or 92).

@jeisenbe
Copy link
Collaborator Author

It looks like @matthijsmelissen is not in favor of the cartographic changes, and no one else has spoken out in support. I agree that the long and narrow areas do not look good with this change (though it works fine with shapes closer to square or circular) so I think this PR can be closed.

@jeisenbe jeisenbe closed this Jan 30, 2019
@jeisenbe jeisenbe deleted the tourism branch January 30, 2019 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants