Adding advertising=column icon #3136

james2432 · 2018-03-20T15:25:58Z

From https://www.openstreetmap.org/#map=19/45.51028/-73.61213 (node):
Z19:

Z18:

From https://www.openstreetmap.org/#map=19/48.53969/2.65847 (way):

kocio-pl · 2018-03-20T15:34:52Z

I think you should skip using issue number in the names of PRs and commits - it's unnecessary technical detail which adds visual clutter and can be easy checked inside the PR ticket.

james2432 · 2018-03-20T15:36:54Z

Ok future commits and PR I will only put inside the PR body.

polarbearing · 2018-03-20T22:53:37Z

I had forgotten that this was unfinished in January, thanks for progressing.
As there is some new code involved we probably need to see that rendered. The two links in the first post are map areas only, not objects?

james2432 · 2018-03-20T22:57:14Z

im working on the renderings now, didnt have access to the docker vm

kocio-pl · 2018-03-21T00:13:23Z

openstreetmap-carto.style

@@ -11,6 +11,7 @@ node,way   addr:housename      text  linear
 node,way   addr:housenumber    text  linear
 way        addr:interpolation  text  linear
 node,way   admin_level  text         linear
+node,way   advertising  text         polygon


I think this is not a good idea - it would mean database migration. We add keys from hstore like tags->'intermittent'.

kocio-pl · 2018-03-21T00:35:37Z

amenity-points.mss

@@ -969,6 +970,15 @@
      marker-file: url('symbols/shop/variety_store.svg');
    }
  }
+


Unwanted spaces.

kocio-pl · 2018-03-21T00:37:18Z

amenity-points.mss

@@ -969,6 +970,15 @@
      marker-file: url('symbols/shop/variety_store.svg');
    }
  }
+
+  [feature = 'advertising']{
+    [advertising = 'column'][zoom >= 19] {


Currently we use single features like [feature = 'advertising_column']. It is a text concatenation in project.mml.

james2432 · 2018-03-21T01:16:04Z

Fixed, tested, same result now; less the database migration requirement

kocio-pl · 2018-03-21T01:18:14Z

project.mml

@@ -1488,6 +1489,7 @@ Layer:
            OR tourism IN ('artwork', 'alpine_hut', 'camp_site', 'caravan_site', 'chalet', 'wilderness_hut', 'guest_house', 'apartment', 'hostel',
                           'hotel', 'motel', 'information', 'museum', 'viewpoint', 'picnic_site')
            OR amenity IS NOT NULL -- skip checking a huge list and use a null check
+            OR tags->'advertising' IS NOT NULL


When the list is relatively short (like here - just 1 element) we don't skip checking, see for example:

OR leisure IN ('water_park', 'playground', 'miniature_golf', 'golf_course', 'picnic_table', 'fitness_centre', 'fitness_station', 'firepit', 'sauna')

kocio-pl · 2018-03-21T01:18:49Z

project.mml

@@ -2031,6 +2036,7 @@ Layer:
              OR tourism IN ('artwork', 'alpine_hut', 'hotel', 'motel', 'hostel', 'chalet', 'wilderness_hut', 'guest_house', 'apartment', 'camp_site', 'caravan_site', 'theme_park',
                             'museum', 'viewpoint', 'attraction', 'zoo', 'information', 'picnic_site')
              OR amenity IS NOT NULL -- skip checking a huge list and use a null check
+              OR tags->'advertising' is NOT NULL


The same comment about skipping list.

kocio-pl · 2018-03-21T01:19:05Z

project.mml

@@ -2179,6 +2186,7 @@ Layer:
                  OR tourism IN ('artwork', 'alpine_hut', 'hotel', 'motel', 'hostel', 'chalet', 'wilderness_hut', 'guest_house', 'apartment', 'camp_site', 'caravan_site', 'theme_park',
                                 'museum', 'viewpoint', 'attraction', 'zoo', 'information', 'picnic_site')
                  OR amenity IS NOT NULL -- skip checking a huge list and use a null check
+                  OR tags->'advertising' IS NOT NULL


The same comment about skipping list.

james2432 · 2018-03-21T01:33:50Z

Added checking for short list in project.mml

kocio-pl · 2018-03-21T01:53:36Z

Commit name should now skip the support for advertising key.

james2432 · 2018-03-21T10:31:10Z

Renamed commit comment

kocio-pl · 2018-03-22T11:38:07Z

There's also some project.mml conflict to resolve now.

james2432 · 2018-03-22T13:57:18Z

Done but is probably going to break when amenity=bbq is merged

kocio-pl · 2018-03-22T14:46:06Z

Thanks! Sometimes it's better to step back for a while - now I think this icon design is the best of what we were trying.

BTW: it doesn't show up on ways without building=* tag (like here), so it's probably the same more general problem as with #3005 (comment).

polarbearing · 2018-03-23T09:29:33Z

The Golden Litfass Column Award goes to everybody :-)

Greetings from the Litfaßplatz in Berlin...

james2432 changed the title ~~[WIP] Issue #3002 Adding advertising column icon and support for advertising key~~ [WIP] Adding advertising column icon and support for advertising key Mar 20, 2018

james2432 force-pushed the avertising_column branch from 36ccbef to 95d8856 Compare March 20, 2018 23:41

james2432 changed the title ~~[WIP] Adding advertising column icon and support for advertising key~~ Adding advertising column icon and support for advertising key Mar 20, 2018

kocio-pl reviewed Mar 21, 2018

View reviewed changes

amenity-points.mss Outdated

@@ -969,6 +970,15 @@

marker-file: url('symbols/shop/variety_store.svg');

}

}

Copy link

Collaborator

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unwanted spaces.

kocio-pl reviewed Mar 21, 2018

View reviewed changes

james2432 force-pushed the avertising_column branch 2 times, most recently from 8f29c63 to 5d8af20 Compare March 21, 2018 01:15

kocio-pl reviewed Mar 21, 2018

View reviewed changes

james2432 force-pushed the avertising_column branch from 5d8af20 to b206228 Compare March 21, 2018 01:33

kocio-pl changed the title ~~Adding advertising column icon and support for advertising key~~ Adding advertising=column icon Mar 21, 2018

Adding advertising=column icon

ef6b92d

james2432 force-pushed the avertising_column branch from b206228 to ef6b92d Compare March 21, 2018 10:30

Merge branch 'master' into avertising_column

f08fd2c

Tomasz-W mentioned this pull request Mar 22, 2018

Code needed to render prepared icons [meta ticket] #3105

Closed

13 tasks

kocio-pl merged commit 35e96bc into gravitystorm:master Mar 22, 2018

james2432 deleted the avertising_column branch March 22, 2018 15:26

kocio-pl mentioned this pull request Oct 21, 2018

Render man_made=telescope #3389

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding advertising=column icon #3136

Adding advertising=column icon #3136

james2432 commented Mar 20, 2018 •

edited

Loading

kocio-pl commented Mar 20, 2018

james2432 commented Mar 20, 2018

polarbearing commented Mar 20, 2018

james2432 commented Mar 20, 2018

kocio-pl Mar 21, 2018

kocio-pl Mar 21, 2018

kocio-pl Mar 21, 2018

james2432 commented Mar 21, 2018

kocio-pl Mar 21, 2018

kocio-pl Mar 21, 2018

kocio-pl Mar 21, 2018

james2432 commented Mar 21, 2018

kocio-pl commented Mar 21, 2018

james2432 commented Mar 21, 2018

kocio-pl commented Mar 22, 2018

james2432 commented Mar 22, 2018 •

edited

Loading

kocio-pl commented Mar 22, 2018

polarbearing commented Mar 23, 2018

Adding advertising=column icon #3136

Adding advertising=column icon #3136

Conversation

james2432 commented Mar 20, 2018 • edited Loading

kocio-pl commented Mar 20, 2018

james2432 commented Mar 20, 2018

polarbearing commented Mar 20, 2018

james2432 commented Mar 20, 2018

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

james2432 commented Mar 21, 2018

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

kocio-pl Mar 21, 2018

Choose a reason for hiding this comment

james2432 commented Mar 21, 2018

kocio-pl commented Mar 21, 2018

james2432 commented Mar 21, 2018

kocio-pl commented Mar 22, 2018

james2432 commented Mar 22, 2018 • edited Loading

kocio-pl commented Mar 22, 2018

polarbearing commented Mar 23, 2018

james2432 commented Mar 20, 2018 •

edited

Loading

james2432 commented Mar 22, 2018 •

edited

Loading