Add support for line property functions #3033

lucaswoj · 2016-08-18T22:44:13Z

This PR adds support for line property functions

fixes #2729

Checklist

Related PRs

wandergis · 2016-08-22T04:03:57Z

great job!!!

lucaswoj · 2016-08-22T22:14:50Z

I'm seeing a statistically significant frame-duration slowdown on this branch.

`line2` Benchmarks

buffer: 1,158 ms
fps: 59 fps
frame-duration: 9.5 ms, 11% > 16ms
query-point: 0.79 ms
query-box: 57.17 ms
geojson-setdata-small: 16 ms
geojson-setdata-large: 127 ms

buffer: 940 ms
fps: 60 fps
frame-duration: 9.4 ms, 10% > 16ms
query-point: 0.82 ms
query-box: 54.48 ms
geojson-setdata-small: 13 ms
geojson-setdata-large: 111 ms

`master` Benchmarks

buffer: 1,034 ms
fps: 60 fps
frame-duration: 8.2 ms, 4% > 16ms
query-point: 0.80 ms
query-box: 56.17 ms
geojson-setdata-small: 11 ms
geojson-setdata-large: 102 ms

stirringhalo · 2016-08-24T00:23:34Z

Fantastic!
Keen on getting this released so I can start using this!

mollymerp · 2016-10-06T21:03:43Z

Per slack chat I will take a stab at the gl-native PR in the next couple days

mollymerp · 2016-11-17T00:58:01Z

Omg can it be?? This is finally ready for review! cc @lucaswoj for 👀

benchmark	master `8c58bdb`	line2 `48e8931`
map-load	132 ms	101 ms
style-load	109 ms	119 ms
buffer	871 ms	844 ms
fps	60 fps	60 fps
frame-duration	4.1 ms, 0% > 16ms	4.4 ms, 0% > 16ms
query-point	0.62 ms	0.62 ms
query-box	64.13 ms	66.54 ms
geojson-setdata-small	5 ms	5 ms
geojson-setdata-large	208 ms	207 ms

mollymerp · 2016-11-17T01:01:17Z

Launch steps:

merge shaders PR Add support for line paint property functions mapbox-gl-shaders#24
merge test-suite PR Add support for line paint property functions mapbox-gl-test-suite#136
update SHAs in native + gl-js Add support for line property functions #3033
merge native + gl-js

jfirebaugh · 2016-11-17T22:54:07Z

js/data/bucket/line_bucket.js

@@ -45,7 +44,12 @@ const lineInterface = {
        {name: 'a_data', components: 4, type: 'Uint8'}
    ]),
    paintAttributes: [
-        {property: 'line-color', type: 'Uint8'}
+        {property: 'line-color', type: 'Uint8'},
+		{property: 'line-blur', multiplier:10, type: 'Uint8', },


Nits: Mixed spaces and tabs, and missing space after :.

jfirebaugh · 2016-11-17T22:57:58Z

js/data/bucket/line_bucket.js

+		{property: 'line-blur', multiplier:10, type: 'Uint8', },
+		{property: 'line-opacity', multiplier:10, type: 'Uint8', },
+		{property: 'line-width', multiplier:10, type: 'Uint8', },
+		{property: 'line-gap-width', multiplier:10, type: 'Uint8', name: 'a_gapwidth', },


Should be a_gap_width, and then an override isn't necessary.

That would require a change to the shaders and native though 💭

jfirebaugh · 2016-11-17T22:58:21Z

js/data/bucket/line_bucket.js

@@ -5,7 +5,6 @@ const createVertexArrayType = require('../vertex_array_type');
 const createElementArrayType = require('../element_array_type');
 const loadGeometry = require('../load_geometry');
 const EXTENT = require('../extent');
-


Keep the whitespace here.

mollymerp · 2016-11-18T18:55:39Z

@jfirebaugh made your requested changes -- I'm hesitant to rename a_gapwidth in this PR (and add additional PRs to native and shaders) but lmk if you want me to do it.

jfirebaugh

Nah, we can handle it at a later time.

lucaswoj · 2016-11-21T18:17:40Z

😂 😂 😂 😂 😂

lucaswoj added the do not merge label Aug 18, 2016

lucaswoj force-pushed the line2 branch from 00aa18b to a72714f Compare August 22, 2016 18:20

lucaswoj changed the title ~~Add support for line-blur property functions~~ Add support for line property functions Aug 22, 2016

lucaswoj assigned lucaswoj and mollymerp and unassigned lucaswoj Oct 12, 2016

mollymerp force-pushed the line2 branch 2 times, most recently from 1ca6410 to 06ba18d Compare October 25, 2016 01:53

mollymerp force-pushed the line2 branch from 06ba18d to 4ece0b2 Compare November 3, 2016 15:17

mollymerp force-pushed the line2 branch 2 times, most recently from 3a6c0cd to 5a1f67b Compare November 11, 2016 23:27

mollymerp mentioned this pull request Nov 12, 2016

[core] update shaders for line property functions mapbox/mapbox-gl-native#6658

Merged

mollymerp force-pushed the line2 branch 4 times, most recently from 94f8c17 to 48e8931 Compare November 17, 2016 00:43

mollymerp mentioned this pull request Nov 17, 2016

add new dds line properties mapbox/mapbox-gl-style-spec#574

Closed

jfirebaugh mentioned this pull request Nov 17, 2016

Data driven styling for line-width attribute #3646

Closed

mollymerp force-pushed the line2 branch from e9da907 to d493908 Compare November 17, 2016 18:40

jfirebaugh suggested changes Nov 17, 2016

View reviewed changes

mollymerp mentioned this pull request Nov 18, 2016

Revert "Add ignores for not-yet-merged GL JS PR" mapbox/mapbox-gl-test-suite#180

Merged

mollymerp force-pushed the line2 branch 3 times, most recently from 17dd304 to 8f8f6d8 Compare November 18, 2016 01:05

jfirebaugh approved these changes Nov 19, 2016

View reviewed changes

Molly Lloyd added 10 commits November 18, 2016 17:36

more rebasing, get tests to pass

2398ca2

fix rebase errors, update package.json

399b3e1

rebase + add device_pixel_ratio to painter

3ca0246

bump shaders

f364021

add line multipliers

261d249

update shader + test-suite shas

ad62968

fix line-offset type and multiplier, update test-suite sha

0656fd3

revert weird spacing

cdf01e7

update test-suite sha

b64c466

bump test-suite sha

2a1384d

mollymerp force-pushed the line2 branch from 8f8f6d8 to 2a1384d Compare November 19, 2016 01:50

mollymerp merged commit 91395c8 into master Nov 21, 2016

mollymerp deleted the line2 branch November 21, 2016 16:51

stirringhalo mentioned this pull request Nov 24, 2016

Transportation layer openmaptiles/openmaptiles#47

Closed

stevage mentioned this pull request Jan 2, 2017

Line-widths computed incorrectly with data properties #3877

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for line property functions #3033

Add support for line property functions #3033

lucaswoj commented Aug 18, 2016 •

edited by mollymerp

Loading

wandergis commented Aug 22, 2016

lucaswoj commented Aug 22, 2016 •

edited

Loading

stirringhalo commented Aug 24, 2016

mollymerp commented Oct 6, 2016

mollymerp commented Nov 17, 2016

mollymerp commented Nov 17, 2016 •

edited

Loading

jfirebaugh Nov 17, 2016

jfirebaugh Nov 17, 2016

mollymerp Nov 18, 2016

jfirebaugh Nov 17, 2016

mollymerp commented Nov 18, 2016

jfirebaugh left a comment

lucaswoj commented Nov 21, 2016

Add support for line property functions #3033

Add support for line property functions #3033

Conversation

lucaswoj commented Aug 18, 2016 • edited by mollymerp Loading

Checklist

Related PRs

wandergis commented Aug 22, 2016

lucaswoj commented Aug 22, 2016 • edited Loading

line2 Benchmarks

master Benchmarks

stirringhalo commented Aug 24, 2016

mollymerp commented Oct 6, 2016

mollymerp commented Nov 17, 2016

mollymerp commented Nov 17, 2016 • edited Loading

jfirebaugh Nov 17, 2016

Choose a reason for hiding this comment

jfirebaugh Nov 17, 2016

Choose a reason for hiding this comment

mollymerp Nov 18, 2016

Choose a reason for hiding this comment

jfirebaugh Nov 17, 2016

Choose a reason for hiding this comment

mollymerp commented Nov 18, 2016

jfirebaugh left a comment

Choose a reason for hiding this comment

lucaswoj commented Nov 21, 2016

lucaswoj commented Aug 18, 2016 •

edited by mollymerp

Loading

lucaswoj commented Aug 22, 2016 •

edited

Loading

`line2` Benchmarks

`master` Benchmarks

mollymerp commented Nov 17, 2016 •

edited

Loading