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

Place road casings below all roads #43

Merged
merged 1 commit into from
Jun 23, 2023

Conversation

Edefritz
Copy link
Contributor

@Edefritz Edefritz commented Jun 12, 2023

Prevents "bridge" like rendering effect of regular intersections.

See #42

Now I'm not 100% sure this is the correct approach. Maybe there is something else I didn't consider so someone should definitely double check for other cases.
But simply moving the casings for all four road types above the first rendered road type seems to address the issue and produces the desired outcome.

OSM
Screenshot 2023-06-12 at 11 13 38
Before PR
Screenshot 2023-06-12 at 11 13 25
After PR
Screenshot 2023-06-12 at 11 12 46

@nvkelso
Copy link
Collaborator

nvkelso commented Jun 14, 2023

+1 to this general approach. I'll followup with a code review soon.

@Edefritz
Copy link
Contributor Author

Thanks! :) I just noticed the diffs might look a bit confusing. The only thing I did was moving the road casing layers up in the rendering order. Git makes it seems as if I changed individual properties of some existing layers.

@bdon bdon merged commit 3557121 into protomaps:main Jun 23, 2023
2 of 3 checks passed
@bdon
Copy link
Member

bdon commented Jun 23, 2023

Thanks! Looks like we need to do the same for tunnels and bridges (all casings for all classes instead before strokes)

@Edefritz
Copy link
Contributor Author

Thanks again for looking into this. I guess now that the changes were merged, it would also make sense to increment the version of the npm package to 1.3.1?
The last release of that package was 5 months ago.

@Edefritz Edefritz deleted the road_casings_order branch July 12, 2023 06:33
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.

None yet

3 participants