-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Add functions for layout properties #956
Conversation
@@ -15,6 +16,10 @@ function createBucket(layer, buffers, collision, indices) { | |||
return; | |||
} | |||
|
|||
for (var k in layer.layout) { | |||
layer.layout[k] = new StyleDeclaration('layout', layer.type, k, layer.layout[k]).value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
layer.layout[k] = new StyleDeclaration('layout', layer.type, k, layer.layout[k]).calculate(z)
Pass the tile zoom as z
into createBucket
. Then you should be able to revert all the changes to the various individual bucket classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying this, but when it's calculated in create_bucket for some reason they don't get reevaluated at different zoom levels — i.e. with a style with
"text-rotate": {
"stops": [[14,0], [16, 190]]
}
if you load the page at zoom 18, the text is rotated, but if you load it at 14 it isn't — and never flips. Newly added labels are rotated at the z-level where they first come in. … 😕 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. When you load it at z14 and then zoom in, does it load z15+ tiles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see. Nope, worker_tile only ever knows about max z15 — so should I be threading the map's zoom through into the worker?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, right, z14 is the max for most VT sources. So the bug is that it appears rotated at z18. It's only ever loading z14 tiles, so only the first stop value should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, no, even when I change those z-levels to 10 and 13, they never recalculate on zoom using this method…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm figured this out
The big weird issue here is that functions only work up to z15 (in streets, for example) because of actual vector tile zoom levels. Is that just a caveat we put in the spec? |
@lbud yeah, I think it's all right for now. We can revisit this later. |
Yeah, if necessary we can recalculate z15, 16, etc geometries with z14 tile data, but let's wait to see if that's needed. |
503a76e
to
9391772
Compare
Closes #361.
text-offset
andicon-offset
) — right now they're hitting some bug in style_declaration.js