Implement data-driven styling for line-dasharray

[lucaswoj]

⚠️ Must be merged in tandem with https://github.com/maplibre/maplibre-style-spec/pull/1100

This PR resolves #1235 by making the line-dasharray property "data-driven", , meaning it can now reference feature property values in expressions.

Due to limits in the expression system, arrays must be specified in the style using the ["literal", [...]] syntax. Feature properties cannot directly contain array values.

Example:

"paint": {
  "line-dasharray": [
    "case",
    ["==", ["get", "road_type"], "trail"], ["literal", [1, 1]],
    ["==", ["get", "road_type"], "highway"], ["literal", [4, 2]],
    ["literal", [2, 2]]
  ]
}

Launch Checklist

Phase 1: Basic Functionality

• [x] Update line_sdf.vertex.glsl and line_sdf.fragment.glsl
• [x] Update line_program.ts
• [x] Update program_configuration.ts
  • [x] Register line-dasharray and its associated varyings under attributeNameExceptions
  • [x] Modify CrossFadedConstantBinder for line-dasharray support
  • [x] Modify ProgramConfiguration for line-dasharray support
• [x] Update style/properties.ts
  • Review CrossFadedDataDrivenProperty to ensure support for data-driven line-dasharray
• [x] Update draw_line.ts
  • [x] Add a call to programConfiguration.setConstantDasharray for line-dasharray
• [x] Briefly describe the changes in this PR.
• [x] Confirm your changes do not include backports from Mapbox projects (unless with compliant license) - if you are not sure about this, please ask!
• [x] Link to related issues.
• [x] ~Include before/after visuals or gifs if this PR includes visual changes.~
• [x] Write integration tests for all new functionality.
• [x] Figure out how to use draft style spec version in CI
• [ ] Get tests for constant line-dasharray passing again
• [ ] Double check understanding of how data is stored line line-atlas
• [ ] Trace codepath for line-cap property functions, determine how it should interact with line-dasharray
• [ ] Trace codepath for line-pattern property-functions, determine which parts of it can work for line-dash array
• [ ] Try to replace lineDasharrayAttributes with patternAttributes as support for the latter is better throughout the codebase
• [ ] Ensure we are preserving fromScale and toScale functionality https://github.com/maplibre/maplibre-gl-js/pull/5812/files#r2064445752
• [ ] Figure out where to add call to LineAtlas#getDash
• [ ] See https://github.com/maplibre/maplibre-style-spec/pull/1100 for additional Launch Checklist items.
• [ ] Add an entry to CHANGELOG.md under the ## main section.
• [ ] Write unit tests for all new functionality.

Phase 2: Performance Benchmarking / Tuning

• [ ] Post benchmark scores.
• [ ] Do manual testing of edge cases
  • adding a large number of line dasharrays
  • different line caps with data-driven styling

Phase 3: Merge

See https://github.com/maplibre/maplibre-gl-js/pull/5768 for similar workflow

• [ ] Merge https://github.com/maplibre/maplibre-style-spec/pull/1100
• [ ] Publish new version of maplibre-style-spec
• [ ] Remove postinstall script from package.json
• [ ] Update package.json to reference latest version of maplibre-style-spec
• [ ] Merge this PR

[1ec5]

Due to limits in the expression system, arrays must be specified in the style using the ["literal", [...]] syntax. Feature properties cannot directly contain array values.

This would be a good use case for being able to form arrays dynamically: maplibre/maplibre-style-spec#950.

[pnorman]

It's great to see work here and I'm looking forwards to when the feature is merged!

[lucaswoj]

Woohoo! All render tests are now passing. I think there are a few edge cases that are broken but untested. However this is a big step closer to marking this "ready for review."

[codecov[bot]]

Codecov Report

:x: Patch coverage is 64.56693% with 90 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 93.05%. Comparing base (db8e0de) to head (14e900a). :warning: Report is 3 commits behind head on main. :white_check_mark: All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
src/data/program_configuration.ts	54.28%	47 Missing and 1 partial :warning:
src/data/bucket/pattern_bucket_features.ts	13.04%	20 Missing :warning:
src/data/bucket/bucket_line_atlas.ts	59.25%	11 Missing :warning:
src/source/worker_tile.ts	69.23%	4 Missing :warning:
src/data/bucket/line_bucket.ts	57.14%	3 Missing :warning:
src/data/bucket/fill_bucket.ts	50.00%	2 Missing :warning:
src/data/bucket/fill_extrusion_bucket.ts	60.00%	2 Missing :warning:

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #5812       +/-   ##
===========================================
+ Coverage   69.68%   93.05%   +23.36%     
===========================================
  Files         289      291        +2     
  Lines       40179    40358      +179     
  Branches     1900     5449     +3549     
===========================================
+ Hits        28000    37556     +9556     
+ Misses      11432     2647     -8785     
+ Partials      747      155      -592     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pnorman]

Is there any way others can help with this right now?

[lucaswoj]

Thanks for the offer @pnorman! Right now I'm investigating one lingering bug, related to rendering data-driven dash array with round caps, and then I'll be campaigning for a PR review. I can't think of anything that others could help with right now.

[lucaswoj]

This PR is ready for review!

Please don't hesitate to reach out to me in the OSM Slack with any questions. I appreciate that this is a lot of code to review. 🙇🏻

[dschep]

Due to limits in the expression system, dasharrays must be specified in the style using the ["literal", [...]] syntax. Feature properties cannot directly contain array values.

Does this apply to all use of line-dasharray or only within data driven expressions? If it's the former, that makes this a breaking change right? And it would be breaking almost all existing MapLibre styles(at least all styles that use line-dasharray, which is a lot of them)

[lucaswoj]

Due to limits in the expression system, dasharrays must be specified in the style using the ["literal", [...]] syntax. Feature properties cannot directly contain array values.

Does this apply to all use of line-dasharray or only within data driven expressions? If it's the former, that makes this a breaking change right? And it would be breaking almost all existing MapLibre styles(at least all styles that use line-dasharray, which is a lot of them)

There's no breaking change here. The existing non-expression syntax like "line-dasharray": [1, 2] will still work. What I'm trying to say here is that there's no way to create an arbitrary array at runtime, like if you wanted to calculate the dash width / gap with from a feature property, "line-dasharray": ["array", ["get", "foo"], ["get", "bar"]]

[HarelM]

I've added some minor comments I believe. This looks good, especially the part where the tests cover all the newly added functionality. Thanks a lot for pushing this forward! Let me know what else you need from my side to push this through.

[lucaswoj]

Thanks for the review @HarelM. I've gone through the review and responded to each comment. In cases where I was able to make the requested change, I resolved the comment. This is ready for another round of review.

[HarelM]

Can you fix the lint warning please?

[lucaswoj]

I've done another round of code review changes. Again, I've left open any conversations that I wasn't able to obviously resolve.

[HarelM]

I've just released version 24.2.0 of the style-spec.

[socket-security[bot]]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​maplibre/​maplibre-gl-style-spec@​24.1.1 ⏵ 24.2.0	+1		+3	+5	+31

View full report

[lucaswoj]

I've done another round of revisions, including updating package.json to reference the newly released style spec version.

[birkskyum]

Is there a demo using this new feature anywhere?

[lucaswoj]

Is there a demo using this new feature anywhere?

I don't have anything up online. Sorry!

[HarelM]

I think the only thing left is the changelog. Can you please add to it and I'll merge this PR and release a new version.

[lucaswoj]

Done