Move control over road rendering order to SQL #626
Conversation
|
On my system (with only Luxembourg loaded) I don't notice any effects on performance. However, perhaps it would be good if someone could test performance on systems with a larger database. |
Importing the planet now to do a test. |
|
If you post a comparison to your test server/machine I'll have have a look especially if you have the UK that I know where problem areas might occur |
In terms of rendering not a lot will change. Do you have a link to areas you're interested in? |
|
This also solves #611. |
|
@gravitystorm Apart from the merge conflict, do you agree with merging this? |
|
pinging myself on this |
|
I have rebased this PR. |
|
@pnorman Did you have time to look at this already? It changes quite a lot of code, so I would like to merge it as soon as possible. |
|
Normal (d40f6b9) rendering speed 8.075 MT/s, SQL re-ordering (db84b2b) rendering speed 8.086. This is statistically significant and a speed increase of 0.1%. This also holds true for with a colde cache, but that result might not be statistically significant. Database not clustered (order by way), no custom indexes. Looking at per zoom results, there's a minor slowdown on z18 which may be below statistical significance, and increasing speedups with decreasing zoom. None are very significant, with the overall being 0.1%, and I'm not positive all are statistically significant on their own. |
|
Assigned to @gravitystorm for review and merging if acceptable. |
|
@pnorman Thanks! |
|
@gravitystorm Could you prioritize the review of this PR? It's a rather large PR, so I'd rather not have to rebase this every week. |
| ::casing { | ||
| [zoom >= 12] { | ||
| [feature = 'highway_motorway_link'], |
There was a problem hiding this comment.
Has the "_link" not been removed from the feature value by the SQL query? Why does this (and further occurrences elsewhere in this file) still explicitly test for "highway_motorway_link"?
|
Thank you, it shouldn't. I will look at it. |
|
Fixed. Thanks again @woodpeck. |
Currently, rendering order of road rendering within one layer is handled by the z_order column, which comes from osm2pgsql. As such, we have little control over road rendering without reloading the database. This PR moves control over the rendering order to the SQL query. This adds complexity to the SQL queries, but increases customizability, and simplifies the roads.mms code. This solves the following issues: * gravitystorm#462 (Move rendering order road types from osm2pgsql to our SQL queries) * gravitystorm#163 (Railways are now drawn above roads) * gravitystorm#167 (Tramway layering issues) * gravitystorm#168 (Paths are now drawn below link roads) * Trac 2024 (Service roads are now rendered below link roads) * Trac 3649 (Service roads are now rendered below race tracks) * Pedestrian and living streets are now consistently ordered * Footways are now always displayed under service ways
This corrects a mismatch between the queries and the style sheet for the roads-low-zoom and roads-text-name layers. This should not have influence on performance.
|
And rebased. |
|
Converted trac references to Github references. |
|
Merged, great work. I'm disappointed that we need to involve this level of complexity in the SQL queries though - especially because it'll need to be used over and over in so many different projects. So when we're happy with the ordertable stuff and fix any bugs that are yet to be found, we should take it upstream to osm2pgsql somehow. |
|
Yes, agreed. And thanks for merging. |
Currently, rendering order of road rendering within one layer is handled
by the z_order column, which comes from osm2pgsql. As such, we have
little control over road rendering without reloading the database.
This PR moves control over the rendering order to the SQL query.
This adds complexity to the SQL queries, but increases customizability,
and simplifies the roads.mms code.
This solves the following issues: