Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

Implement explicit instance ordering #235

Open
gravitystorm opened this issue Jan 6, 2013 · 12 comments
Open

Implement explicit instance ordering #235

gravitystorm opened this issue Jan 6, 2013 · 12 comments
Milestone

Comments

@gravitystorm
Copy link
Contributor

This is an isolation of the problem reported at gravitystorm/openstreetmap-carto#23

We want to define lines (instance 'a') and dashes (instance 'b'), and want the dashes to always be on top of the lines. To do this, we define 'a' first.

#roads[zoom >= 14] {
  a/line-width: 5;
  a/line-color: blue;
  b/line-width: 15;
  b/line-dasharray: 5,15;
  [zoom >= 15] {
    a/line-width: 8;
    b/line-width: 16;
  }
  [highway = 'primary'] {
    a/line-color: red;
  }
}

You get expected behaviour at zoom 14 (black dashes over all lines)

zoom14

and unexpectedly, at zoom 15 the red lines are over the top of the dashes.

zoom15

The order of the instances is based on the order that each instance is defined in the file, but it's hard to see why, for primary roads at z15, it thinks that b/ is declared higher up in the file than a/.

@springmeyer
Copy link

Subtle bug, thanks for the solid report. However you must mean 'instances' not attachments, right?

@gravitystorm
Copy link
Contributor Author

Gah! Yes, I meant instances :-) I've updated the report to save further confusion.

@tmcw
Copy link
Contributor

tmcw commented Jan 7, 2013

Seems like the order is correct here: https://gist.github.com/4477412 since the nested rule with [type='highway'] makes a the last instance to appear in the style.

@gravitystorm
Copy link
Contributor Author

@tmcw - sorry, it's not clear whether you're saying there's a workaround (by rearranging things), or that carto is behaving as expected. Do you agree there's a bug?

@tmcw
Copy link
Contributor

tmcw commented Jan 15, 2013

y, to be a little clearer about this potential workaround:

// zoom >= 15, and the last style is b
#roads[zoom >= 14] {
  a/line-width: 5;
  a/line-color: blue;
  b/line-width: 15;
  b/line-dasharray: 5,15;

  a/line-width: 8;
  b/line-width: 16;
}

// highway=primary, last style is a
#roads[zoom >= 14] {
  a/line-width: 5;
  a/line-color: blue;
  b/line-width: 15;
  b/line-dasharray: 5,15;

  a/line-color: red;
}

@springmeyer
Copy link

report of inverted styles also at http://support.mapbox.com/discussions/tilemill/6834-land-and-water-inverting-at-zoom-level-17, but not sure/doubtful it has to do with instances...

@nebulon42 nebulon42 added the bug label Mar 2, 2016
@nebulon42 nebulon42 modified the milestone: 0.17 Jan 20, 2017
@nebulon42 nebulon42 added won't fix and removed bug labels Jan 20, 2017
@nebulon42
Copy link
Collaborator

After analysis I'm not really sure this is a bug. If it is one it is not easily fixed. For ordering carto uses the index (basically the location where the rule is written). If you look at which rules make up the definition on zoom level 15 you find that b/line-dasharray: 5,15; is the earliest rule here as the rest is overwritten later. Thus, here b takes precedence over a.

Basically you have to be careful in which order you write the rules for instances although it may not be obvious at first sight why the behaviour is as it is. To match the "intention" of the writer of the style carto would have to inherit the index of the first occurrence of the instance with all sorts of other problems.

I'm closing this as won't fix.

@gravitystorm
Copy link
Contributor Author

I'm closing this as won't fix.

That's a shame.

although it may not be obvious at first sight

I think that's quite an understatement! The whole point of this ticket is that instance order is both unstable and mostly unpredictable. I don't think anyone would read the original example and find that the re-ordering is useful, and I think only a tiny fraction of CartoCSS users would even realise that there is a risk of re-ordering at all.

If this is really wontfix then I will suggest making a new feature request for stable and predictable ordering of instances, likely based on alphabetical ordering, for the next major release.

@nebulon42
Copy link
Collaborator

If this is really wontfix then I will suggest making a new feature request for stable and predictable ordering of instances, likely based on alphabetical ordering, for the next major release.

This might be a good idea. With that one knows what to expect.

But feel free to add other insights or get others to add theirs. I have pondered some time about that and could not come up with a satisfactory solution without changing the way how instances work. But my insight into the core processes of carto is still incomplete. Unfortunately, those people with the greatest insight are not available for this project anymore.

@gravitystorm
Copy link
Contributor Author

a satisfactory solution without changing the way how instances work

My suggestion would be to track the index of the instance rules, ignoring if the particular attribute declaration is overridden later. This would mean that in my example, a is always a lower index than b, even when both line-width and line-color are both overridden later on.

Of course, I still believe that a more explicit way to order the instances would be helpful, but that's a bigger project.

@nebulon42
Copy link
Collaborator

I suspect that instance ordering upon serialization would be easier to implement. I planned 1.0 anyway after 0.18, so this would not be too far away. Instead of opening a new ticket and losing the context let's transform this into a feature request.

@nebulon42 nebulon42 added this to the 1.0 milestone Jan 23, 2017
@nebulon42 nebulon42 changed the title Instances swap order with nested rules Implement explicit instance ordering Jan 23, 2017
@nebulon42 nebulon42 reopened this Jan 23, 2017
@nebulon42
Copy link
Collaborator

Related and to be taken into account: #247

@nebulon42 nebulon42 modified the milestones: 1.0, 1.1, 2.0 Dec 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants