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

Implement property functions for "line-dasharray" and "line-cap" #3045

Closed
lucaswoj opened this issue Aug 22, 2016 · 29 comments · Fixed by #10591
Closed

Implement property functions for "line-dasharray" and "line-cap" #3045

lucaswoj opened this issue Aug 22, 2016 · 29 comments · Fixed by #10591
Assignees

Comments

@lucaswoj
Copy link
Contributor

Implementing property functions for line-dasharray will require sharing information about LineAtlas with the worker, much like the *-pattern properties #2732

@lucaswoj lucaswoj changed the title Implement property functions for "line-dasharray" Implement property functions for "line-dasharray" and "line-width" Feb 15, 2017
@lucaswoj lucaswoj changed the title Implement property functions for "line-dasharray" and "line-width" Implement property functions for "line-dasharray" Feb 15, 2017
@lucaswoj lucaswoj changed the title Implement property functions for "line-dasharray" Implement property functions for "line-dasharray" and "line-width" Feb 15, 2017
@lucaswoj
Copy link
Contributor Author

This is currently blocked by implementation difficulties. line-dasharray is measured in units relative to line-width. We may need to change the units of line-dasharray to pixels #4158 before proceeding.

@lbud
Copy link
Contributor

lbud commented Jun 28, 2017

Re: line-dasharray: from #4434

High priority; high difficulty. Perhaps not feasible to support.

One thing we actually might consider, now that the line-width scaling happens in the shader and acts essentially like its own property function, is adding a new property like line-dasharray-units: ["width", "pixels"] (default: width). This would allow users using data-driven line-width to maintain full control over line-dasharray. I think this would be pretty easy to support after #4773 — floorwidth would just need to be 1 instead of the line-width values evaluated at floor(z).

This would enable something like
image
(where line-width is property-driven (5, 10, 15); line-dasharray is always [10px, 10px])

@mollymerp
Copy link
Contributor

mollymerp commented Jun 28, 2017

@lbud we talked about a variation of this (adding a new line-dasharray-pixels property), but I like your idea better.

@lbud
Copy link
Contributor

lbud commented Jun 29, 2017

(we could also use something like [relative, absolute], which is more recognizable/memorable but less explicit…)

@lbud lbud changed the title Implement property functions for "line-dasharray" and "line-width" Implement property functions for "line-dasharray" and "line-cap" Jul 24, 2017
@lbud
Copy link
Contributor

lbud commented Jul 24, 2017

I edited the title of this because line-width was implemented, and line-cap implementation is intertwined with line-dasharray.

@jfirebaugh
Copy link
Contributor

Is it possible to have one line atlas and line atlas texture for each tile, like we're planning to do for glyph and icon atlases?

@lbud
Copy link
Contributor

lbud commented Jul 26, 2017

@jfirebaugh probably. That might be helpful in the approach I'm working on for data-driven line-caps then (which is to create both round and non-round lines in the line atlas for each dash pattern and be able to pick the right one in the shader, which will increase the line atlas area used — this would limit us to 32 distinct line-dasharray patterns per line atlas) 🕵️‍♀️

@lbud
Copy link
Contributor

lbud commented Jul 27, 2017

(Although I think we should also be able to save space in the line atlas, if we need to, by utilizing r/g/b channels — right now it only utilizes the alpha channel.)

@lbud
Copy link
Contributor

lbud commented Aug 17, 2017

Style refactors should proceed for now without these properties — they are very complicated:

  • For line-dasharray, the LineAtlas is constructed dynamically: property values are resolved and then in draw_line, just before rendering, dash patterns are retrieved if available or added to the atlas. In order to enable property functions for dashed lines we'd need to enumerate all possible values, create the line atlas, pass atlas position data to the shader, and then query for those positions. This would be both difficult/zany and would make it very easy to overflow the line atlas. I don't think we should do this.
  • For line-cap it's slightly easier, only in that there are only two variants in the atlas based on line-cap values (round or not-round). So theoretically we could add round and not-round patterns for all lines as we add them to the line atlas and then use either the not-round position or round position in the shader. This would also cause us to use a lot more line atlas space (not-round lines are 1px thick, round lines are 15px). We could save space by using all channels instead of just the alpha channel, but I'm still not sure this is a good idea unless this property is really necessary.

@anandthakker
Copy link
Contributor

Yeah, this seems pretty intractable as things currently stand.

One possibility that @nickidlugash and I talked about was imposing a restriction on the type of style function that's allowed so that we can know all of its possible outputs ahead of time. E.g. (using the forthcoming expression syntax) "data-driven line-dasharray values may only use expressions of the form [match, [get, "myProperty"], "a", [literal, [0, 1]], "b", [literal, [3, 4]], ...]".

This would mitigate the zaniness and overflow issues, at the expense of weird restrictions we'd have to explain & implement 🤷‍♂️

@nickidlugash
Copy link

@lbud @jfirebaugh Any initial thoughts on this option? ^

@jfirebaugh
Copy link
Contributor

I think we should first investigate using per-tile line atlases. We had great results for per-tile glyph and sprite atlases, and doing the same for lines in theory allows us to make line-dasharrayand line-cap data-driven without restrictions.

@nickidlugash
Copy link

@jfirebaugh @lbud that sounds great 👍 Any idea when the team might have bandwidth to look into this? Right now I'm working on the assumption that we will not have DDS values for these properties in the near future (but if there is a possibility of getting these implemented soon, I'd be happy to do whatever I can on my end to make it happen).

@nickidlugash
Copy link

@jfirebaugh @lbud @anandthakker given that we do now have restrictions on use of expressions in text-font, would having the same restrictions be helpful for enabling data-driven line-dasharray and line-cap values? And/or does it still seem feasible to enable DDS by switching to a per-tile line atlas?

@lbud lbud removed their assignment Jan 25, 2018
@jplante
Copy link

jplante commented Feb 14, 2018

Just following up on @nickcordella last question to @jfirebaugh, @lbud, @anandthakker on December 11th.

@jfirebaugh
Copy link
Contributor

The answer is "we don't know, and haven't had time to investigate".

@jplante
Copy link

jplante commented Feb 14, 2018

Ok thanks.

This is a high priority item for our team

Thanks

@donnyv
Copy link

donnyv commented Sep 19, 2018

So is the "line-dasharray" data driven now? I'm using v0.48.0 and its telling me "property functions not supported" when using this.

'line-dasharray': { 
					"type": "identity", 
					"property": "line_dash_array"
				}

@mollymerp
Copy link
Contributor

no, it is not implemented yet and that is why this issue is still open. you can see full support details on the style-spec documentation https://www.mapbox.com/mapbox-gl-js/style-spec/#paint-line-line-dasharray

@mwamedacen
Copy link

Hi there, any update about the status of this feature request ? Thank you

@NameFILIP
Copy link

NameFILIP commented Sep 7, 2019

Is this feature on a short, mid or long term roadmap?

In my case, based on the feature properties I want to display some LineStrings as dashed lines, others as regular solid lines.

@asheemmamoowala
Copy link
Contributor

@NameFILIP this continue to be on our long term roadmap, but is not scheduled to be worked on anytime soon.

In my case, based on the feature properties I want to display some LineStrings as dashed lines, others as regular solid lines.

For now, the best way to do this would be to use filters to style the lines as part of different layers

@Mechazawa
Copy link

Mechazawa commented Sep 11, 2019

Is there any indication if this will be worked on this year? Data driven styling is a lot more manageable then creating a layer per line-dasharray value. Even just having support for ["get", "fooBar"] would greatly improve the usability.

@adrienj
Copy link

adrienj commented Jan 9, 2020

For the record, If anybody wants to implement something like @lbud showed using the current API, you could split your layers to match the line-width and the line-dasharray at "equal" size. In my use case I only care about line-widths of 1, 2 and 3px so it's relatively simple.

[
    {
        id: 'line-dashed-small',
        type: 'line',
        paint: {
            'line-width': ['get', 'width'],
            'line-dasharray': [4, 4]
        },
        filter: ['==', 'width', 1]
    },
    {
        id: 'line-dashed-large',
        type: 'line',
        paint: {
            'line-width': ['get', 'width'],
            'line-dasharray': [2, 2]
        },
        filter: ['==', 'width', 2]
    },
    {
        id: 'line-dashed-xlarge',
        type: 'line',
        paint: {
            'line-width': ['get', 'width'],
            'line-dasharray': [1, 1]
        },
        filter: ['>', 'width', 2]
    }
]

Result:
image

@zstadler
Copy link

@asheemmamoowala
I'm sorry to see that the "high priority" issue has been removed.
I'm rendering 6 different dash arrays based on the tracktype tag and it required duplicating the layer 6 times

image

@asheemmamoowala
Copy link
Contributor

@zstadler I don't think it is fair to communicate that an issue is high priority if it's been open for ~3 years. Removing the tag better reflects our current priorities.

image

@zstadler
Copy link

zstadler commented Jun 12, 2020

@asheemmamoowala
Given, as you wrote above, that the tag reflects the Mapbox priority, please consider my comment as a user request to implement this feature.
For your convenience, I've added an example of the impact the feature can have on users. IMHO, the fact that users have been waiting for the feature for ~3 years does not decrease its value.

@ClareTrainor
Copy link

Data-driven styling for line-dasharray would especially be helpful when there are a large number of layers that need to be duplicated. For example, there are often layers for each road class with traffic styling. Closed roads are often visualized with a solid black line or a dashed red line to differentiate them from the green, orange, red, and dark red congestion colors. On dark basemaps, a solid black line has little contrast with the background, making closed roads hard to see. A dashed line would be preferred and it would be a major benefit if all closed roads could be managed in the same layers as other traffic layers and styling.

Screen Shot 2021-01-31 at 6 56 19 PM

@mourner
Copy link
Member

mourner commented May 6, 2021

This has just landed and will be a part of the upcoming GL JS v2.3 release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.