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

Give oceans outline and simplify shapefiles on z0-7 #3065

Merged

Conversation

matthijsmelissen
Copy link
Collaborator

Before:
screen shot 2018-02-11 at 20 55 37

After:
screen shot 2018-02-11 at 21 00 37

Still needs testing on more realistic data.

@kocio-pl
Copy link
Collaborator

Looks nice for me.

@rrzefox
Copy link

rrzefox commented Feb 16, 2018

deployed in the usual location for global testing. Looks good IMHO in the low zoom levels. Looks a bit weird on Z7 where it overlaps with dashed borders, e.g. http://bl.ocks.org/matthijsmelissen/raw/af7a602c222dbf1ff1a2c0d84ed755b7/#7.00/37.519/16.197

@matthijsmelissen
Copy link
Collaborator Author

I reduced the simplification to prevent artefacts at the antemeridian (visible in the demo above).

Rendering demo:
screen shot 2018-03-05 at 00 03 09

As far as I'm concerned this is ready to be merged.

@matthijsmelissen
Copy link
Collaborator Author

@kocio-pl What do you think?

@kocio-pl
Copy link
Collaborator

kocio-pl commented Mar 7, 2018

I still like it 😄 however I'd like to know if it won't produce performance problems (see also #3103 (comment))? Do you have the idea how should we estimate it in general?

@matthijsmelissen
Copy link
Collaborator Author

I don't expect performance problems. The problems in #1938 where caused by the ST_Within check (we were checking for admin polygons located within big admin_level 2 polygons), which we don't do now - this is a totally different approach.

It seems to run without problems on @rrzefox's server.

Copy link
Collaborator

@pnorman pnorman left a comment

Choose a reason for hiding this comment

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

👍 based on previews, but I haven't run it myself.

@matthijsmelissen matthijsmelissen merged commit d8fc3c7 into gravitystorm:master Mar 9, 2018
@kocio-pl
Copy link
Collaborator

I think this is a small side effect of this change - when the river mouth is big you can see this outline on the water up to z7:

screenshot-2018-3-24 openstreetmap
screenshot-2018-3-24 openstreetmap 1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants