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

Sets outline and opacity defaults for polygon fill #40

Closed
wants to merge 3 commits into from
Closed

Sets outline and opacity defaults for polygon fill #40

wants to merge 3 commits into from

Conversation

mholthausen
Copy link
Member

@mholthausen mholthausen commented May 12, 2021

This MR

  • uses mapfileStyle.width (if defined) as outlineWidth
  • sets default values for outlineOpacity and opacity
    • depending on mapfileStyle.color and mapfileStyle.outlineColor the corresponding opacity values (fillOpacity and outlineOpacity) are set to the defined value or a default value
  • provides a new test case for polygons defined with separate styles for fill and outline values

@KaiVolland @jansule please review

@mholthausen mholthausen marked this pull request as ready for review May 17, 2021 13:54
Copy link
Contributor

@KaiVolland KaiVolland left a comment

Choose a reason for hiding this comment

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

I think this will add unneeded information to the geostyler-stlye.
If you have a very simple geostyler-style symoblizer like this:

{
  "kind": "Fill",
  "color": "#0E1058"
}

… and you write it to mapfile and reread this mapfile the result will be:

{
  "kind": "Fill",
  "color": "#0E1058",
  "fillOpacity": 0, // Not sure if its 0 but this would be even wrong
  "outlineOpacity": 0 // Not sure if its 0 but this would be even wrong
}

@mholthausen
Copy link
Member Author

The result will be

{
  "kind": "Fill",
  "color": "#0E1058",
  "fillOpacity": 1, // beacuse color is defined
  "outlineOpacity": 0 // beacuse there is no outlineColor defined
}

Additionally the way (writeStyle) to generate a Mapfile from geostyler-style is not implemented yet.

@jansule
Copy link
Contributor

jansule commented May 18, 2021

I think this will add unneeded information to the geostyler-stlye.

I think our concept of file based similarity is a little outdated. It might be better to follow a visual-similarity-approach.

There is already an issue for this (geostyler/geostyler#1444) and this should also be discussed in the next monthly (geostyler/geostyler#1451).

IMHO, this should be non-blocking for now.

@mholthausen mholthausen closed this by deleting the head repository Oct 17, 2023
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