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

Opacity property is not like the others #249

Open
migurski opened this issue Feb 6, 2013 · 8 comments
Open

Opacity property is not like the others #249

migurski opened this issue Feb 6, 2013 · 8 comments

Comments

@migurski
Copy link

migurski commented Feb 6, 2013

Opacity (and related comp-op) operates on a layer as a whole. However, it’s specified inline with the rest of the rules, which is confusing. This sort of thing shouldn’t work, or should perhaps throw a warning:

#foo[bar=1] { opacity: 0.2 }
#foo[bar=2] { opacity: 0.8 }

My experience has been that opacity gets set just once, for the whole layer, and I’m not sure how to predict which one would win in this case.

These properties should probably be specified on layers in the .mml file instead.

@springmeyer
Copy link

I agree that we should consider warning or throwing with "sorry opacity should only be set once per style" for the style you've provided because its not clear which wins.

But they are not exactly layer options, so I think they make more sense where they are, despite potential ambiguity, than in the mml. opacity ,comp-op, and image-filters are Style level properties, so in the case that multiple styles are used against a single layer (like when using ::attachments) it does make sense to allow opacity to be set twice like:

#countries {
  ::outline {
    line-color: #85c5d3;
    line-width: 2;
    line-join: round;
    opacity:.5;
  }
  polygon-fill: #fff;
  opacity:.1;
}

Would produce:

Screen Shot 2013-02-06 at 2 03 32 PM

@migurski
Copy link
Author

migurski commented Feb 6, 2013

Thanks, I’ll keep this in mind when I use opacity in the future.

@springmeyer
Copy link

@tmcw - do you think it is actionable to throw if one of these style level options is detected twice inside a single attachment?

@migurski
Copy link
Author

migurski commented Feb 6, 2013

(also thanks for the attachments pointer—I’ve always found that feature of Carto confusing so I don’t use it and don’t reach for it when I should)

@tmcw
Copy link
Contributor

tmcw commented Feb 6, 2013

@tmcw - do you think it is actionable to throw if one of these style level options is detected twice inside a single attachment?

Might be a little tricky, but seems possible.

@gravitystorm
Copy link
Contributor

I'd prefer it to not throw, and to let you set the option twice. The cascading rules of CSS should be clear, that defining the same value for the second time simply overrides it.

I think that keeping the cascade might be useful when making customisations to large third-party styles - perhaps you want to track an upstream style but override one or two settings in a separate mss file (making it easier to pull in updates). Throwing if opacity is defined twice would prohibit that approach, and it's not clear why we'd treat style-level rules differently from symbolizer-level rules in any case.

@migurski
Copy link
Author

I’d prefer it if style rules didn’t throw either, but we have here a style rule that’s not really a feature-based style rule at all. Does it belong in Carto in the first place?

@springmeyer
Copy link

logging for later reflection: as per my comment at g12n/cardboardMap@a7d37bc#commitcomment-3823331, it looks like when two image-filters are applied they are concatenated. the last should likely win, but needs more thought.

g12n added a commit to g12n/cardboardMap that referenced this issue Aug 9, 2013
As pointed out by Dane Springmeyer ( springmeyer ) blur couldnt be reset by zoom-level.

»image-filters actually operate at the style level (like the bar opacity parameter - see mapbox/carto#249) and are applied to the rendering canvas once all features are rendered. «

Signed-off-by: mge <[email protected]>
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

5 participants