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

Clean up widths of bridges and tunnels #441

Merged
merged 1 commit into from Apr 8, 2014

Conversation

matthijsmelissen
Copy link
Collaborator

There is currently not much logic behind the width of bridges and
tunnels. This commit gives bridges and tunnels consistent widths.

Changing the widths of regular roads, i.e. roads that are not in a
tunnel or on a bridge, is out of the scope of this commit, and will be
dealt with later. Highways other than motorway, trunk, primary,
secondary, tertiary, residential/unclassified and their links are also
out of scope.

The width of bridges and tunnels after this commit is either inherited
from, or defined in terms of the width of regular roads. This will make
the widths of tunnels and bridges easier to maintain.

The widths have been changed based on the following three principles.

  1. Make the casing of bridges just as wide as the casing of regular
    roads.

    Currently, the casing of bridges is a mess. Some bridges are
    narrower than regular roads, up to 1.8 pixel (trunk and primary
    roads on z15/z16). On the other hand, motorway links are 1.5 pixel
    wider on bridges on z12. There are also all kinds of cases in
    between.
    When the casing of bridges is narrower than the casing of regular
    roads, itwith too much of the background, especially when two roads
    run closely in parallel. Therefore, I have chosen to make the casing
    of bridges and regular roads equal.

  2. Make the diffence between bridgecasing and bridgefill, i.e. the black
    outline of bridges, per zoomlevel the same for all road types.

    Currently the black outline of bridges (casing minus fill) is
    thicker for some roads (z13: tertiary roads; z14: tertiary roads
    and tertiary link roads; z17: motorway roads, motorway links,
    residential/unclassified roads) compared to other roads on the same
    zomolevel. This is because they have a bit smaller bridgefill, and
    thus a larger black outline, compared to the other roads.
    The black outline (casing minus fill) should be a bit narrower than
    the regular outline, because the black stands out much more out
    than the regular casing which blends in with the fill of the road.
    As we keep bridges and regular roads the same width, consequently
    the fill on bridges needs to be a bit wider than the fill on
    regular roads.

  3. Keep the casing and fill of tunnels the same as the casing and fill
    of regular roads.

    Again there are currently large and inconsistent differences. Trunk
    and primary on z17 and larger are even 4 pixels narrower in tunnels.
    On the other hand, tertiary on z17 and larger are 0.5 pixel wider
    in tunnels. I see no reason to give tunnels a different width.
    Therefore, I have made tunnels equally wide as regular roads.

As the new definitions keep the casing constant, rather than the fill,
the most basic definition is now the width of the casing of roads,
instead the width of the fill. All other widths are now derived from
that.

This commit solves 3834 on trac and part of #265 on Github.

There is currently not much logic behind the width of bridges and
tunnels. This commit gives bridges and tunnels consistent widths.

Changing the widths of regular roads, i.e. roads that are not in a
tunnel or on a bridge, is out of the scope of this commit, and will be
dealt with later. Highways other than motorway, trunk, primary,
secondary, tertiary, residential/unclassified and their links are also
out of scope.

The width of bridges and tunnels after this commit is either inherited
from, or defined in terms of the width of regular roads. This will make
the widths of tunnels and bridges easier to maintain.

The widths have been changed based on the following three principles.

1. Make the casing of bridges just as wide as the casing of regular
   roads.
   > Currently, the casing of bridges is a mess. Some bridges are
     narrower than regular roads, up to 1.8 pixel (trunk and primary
     roads on z15/z16). On the other hand, motorway links are 1.5 pixel
     wider on bridges on z12. There are also all kinds of cases in
     between.
     When the casing of bridges is narrower than the casing of regular
     roads, itwith too much of the background, especially when two roads
     run closely in parallel. Therefore, I have chosen to make the casing
     of bridges and regular roads equal.

2. Make the diffence between bridgecasing and bridgefill, i.e. the black
   outline of bridges, per zoomlevel the same for all road types.
   > Currently the black outline of bridges (casing minus fill) is
     thicker for some roads (z13: tertiary roads; z14: tertiary roads
     and tertiary link roads; z17: motorway roads, motorway links,
     residential/unclassified roads) compared to other roads on the same
     zomolevel. This is because they have a bit smaller bridgefill, and
     thus a larger black outline, compared to the other roads.
     The black outline (casing minus fill) should be a bit narrower than
     the regular outline, because the black stands out much more out
     than the regular casing which blends in with the fill of the road.
     As we keep bridges and regular roads the same width, consequently
     the fill on bridges needs to be a bit wider than the fill on
     regular roads.

3. Keep the casing and fill of tunnels the same as the casing and fill
   of regular roads.
   > Again there are currently large and inconsistent differences. Trunk
     and primary on z17 and larger are even 4 pixels narrower in tunnels.
     On the other hand, tertiary on z17 and larger are 0.5 pixel wider
     in tunnels. I see no reason to give tunnels a different width.
     Therefore, I have made tunnels equally wide as regular roads.

As the new definitions keep the casing constant, rather than the fill,
the most basic definition is now the width of the casing of roads,
instead the width of the fill. All other widths are now derived from
that.

This commit solves 3834 on trac and part of gravitystorm#265 on Github.
@matthijsmelissen
Copy link
Collaborator Author

@Rovastar
Copy link
Contributor

Why is everything +2*casing-width- ?

Why not just double the casing width in the constants.

@matthijsmelissen
Copy link
Collaborator Author

Because a road has two casings (on both sides of the road).

The idea is that casing-width-XXX is now the actual width of the black line around the roads. That amount need to be subtracted twice from the casing to get the fill, as there is a black line on both sides of the fill.

The way it is specified now, it is easier for designers to work width: the pixel amount they enter as casing-width-XXX is the actual width they end up seeing. It might also be easier for comparing casing's with paths, for example. I agree that the old way was easier for the coders to work with, as it causes less repetition in the code.

If people prefer the old code-centered view over the designer-centered view, I don't mind changing it.

@gravitystorm
Copy link
Owner

I prefer @math1985 's approach where the casing width is the apparent width of the casing strokes

@Rovastar
Copy link
Contributor

OK fine it just wasn't clear and counter-intuitive from a coders point of view.

Could we add some comments in the code to that effect.

As a general point: As we are getting bigger and more complex code. I prefer to see comments in in the code explaining things a little. Otherwise it just looks like a homebrew project.

@matthijsmelissen
Copy link
Collaborator Author

@gravitystorm If there is any technical reason for not merging this pull request, could you let me know? If it's just a issue with lack of time, that's fine of course.

gravitystorm added a commit that referenced this pull request Apr 8, 2014
Clean up widths of bridges and tunnels
@gravitystorm gravitystorm merged commit 7f5235b into gravitystorm:master Apr 8, 2014
@gravitystorm
Copy link
Owner

I've merged this, since I know there's work pending to go on top. My only comments are:

  • there's a stray //etc that has crept in - that either needs more explaining or removed
  • I'm in two minds as to whether the width should be for the fill, or for the whole road. I mean, should width imply "casing = width + 2_casing" or "fill = width - 2_casing". I'm sure you've thought about it more than me though but at first glance I'd expect the widths to be the widths of the fill, and the whole road width to be "width + 2*casing-width". The commit introduced the reverse.
  • The turning circles are becoming a bit complicated! I think that's because of the above.

@matthijsmelissen
Copy link
Collaborator Author

Thanks for accepting the PR.

I'm in two minds as to whether the width should be for the fill, or for the whole road.

I commented on that in my PR:

As the new definitions keep the casing constant, rather than the fill, the most basic definition is now the width of the casing of roads, instead the width of the fill. All other widths are now derived from that.

In other words, bridges and tunnels have (now) the same casing as regular roads (because that way it looks best), but a different fill (because bridges have a smaller visible casing). Therefore, I thought specifying the casing would be cleaner. Otherwise, we would have to specify bridge-fill-width = fill-width + 2_casing-width - 2_bridge-casing-width, which is even uglier. I agree the turning circle definitions get rather complex, but I guess we can't avoid both problems at the same time.

@gravitystorm
Copy link
Owner

Or we could keep the fills a constant width, and if the casing is wider then the feature ends up wider rather than eating into the fill.

I think :-)

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