Line-dasharray Impl. #39
Conversation
|
Thanks for the fast review! I did just a draft yesterday night and wasn't fully finished, I'll look into it. :) |
lib/src/model/tile_model.dart
Outdated
| } | ||
|
|
||
| return _paths; | ||
| if (dashLengths.length >= 2) { | ||
| if (_dashedPaths.containsKey(dashLengths)) { |
There was a problem hiding this comment.
Based on our previous discussion, would it make sense to remove _dashedPaths? The lifecycle of a tile could be quite long, and unless I'm mistaken the key (a list of doubles) could vary a lot.
Alternatively, we could limit the size of _dashedPaths (e.g. to some arbitrary size, e.g. 20)
In that case, to avoid looking up the key in the map twice, consider something like this:
var path = _dashedPaths[dashLengths];
if (path == null) {
path = _paths?.map ...
_dashedPaths[dashLengths] = path;
}
return path;There was a problem hiding this comment.
Hey, yes i think it makes sense to delete this completely. I've removed it now.
lib/src/model/tile_model.dart
Outdated
| @@ -64,10 +65,16 @@ class TileFeature { | |||
| _paths = decodePolygons(geometry).toList(growable: false); | |||
| break; | |||
| } | |||
| _geometry = null; | |||
There was a problem hiding this comment.
Leaving geometry in-place comes with a significant memory penalty. Why do we need to do this?
There was a problem hiding this comment.
Hey, wups jep, overlooked it. It makes sense to delete the geometry after decoding.
|
Lukas, this is looking good, we should get it in. Could you rebase over |
outsource path-dashing into seperate file move property into PaintFactory move generation of dashed paths into TileFeatures and prevent recalculations
rename some params reformat
outsource ringnumberprovider remove some comments
Lukas-Heiligenbrunner
force-pushed
the
line-dasharray
branch
from
2c0fa6a
to
0453a1b
Compare
|
Should I squash the commits? |
|
Definitely squash to your hearts content. That’s what I would end up doing
anyways.
…On Thu, Jul 14, 2022 at 3:09 PM Lukas-Heiligenbrunner < ***@***.***> wrote:
Should I squash the commits?
I couldn't manage to merge every commit correctly, the merge conflicts
were a pain.
So the history isn't fully correct now...
—
Reply to this email directly, view it on GitHub
<#39 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEKDQ7U7PTHEHX5CQEBUY4LVUCFZZANCNFSM5Z4KGZFQ>
.
You are receiving this because you commented.Message ID:
***@***.***>
|
|
squashed, rebased and merged on #47 |
|
Thanks for your contribution Lukas! |
|
Thanks for your help and patience! :) |
Hey David,
I tried to implement a missing paint property: line-dasharray which is heavily used within country boarderlines. I'm still quite unfamiliar with your project structure but I tried my best to integrate this.
I've parsed the dasharray with
jsonLayer['paint']['line-dasharray']and didn't use your expressionparser system. Should I've used that?The flutter inbuilt painter doesn't support line-dashing so I split the paths into subpaths with correct sizing.
I didn't really know where to place this function in your proj. structure. Do you have any suggestions?
Here you can see a before and after comparison:
I am open to criticism and hope you can find some time to review my code.
Hopefully this PR kind of leads into the right direction and helps :)
Kind regards
Lukas