support tiles with extent != 4096 #2010
Conversation
| // round here because mapbox-gl-native uses integers to represent | ||
| // points and we need to do the same to avoid renering differences. | ||
| Math.round(point.x *= scale); | ||
| Math.round(point.y *= scale); |
There was a problem hiding this comment.
That doesn't look right. Math.round is immutable, so point will remain unrounded. Should be:
point.x = Math.round(point.x * scale);
point.x = Math.round(point.y * scale)Also easier to read.
There was a problem hiding this comment.
yeah, definitely not right. Thanks for catching that, I'm sure it would have caused some headaches later on
|
Do you happen to know if this PR resolves mapbox/mapbox-gl-test-suite#75? |
| @@ -20,11 +21,10 @@ function Tile(coord, size, sourceMaxZoom) { | |||
| this.uses = 0; | |||
| this.tileSize = size; | |||
| this.sourceMaxZoom = sourceMaxZoom; | |||
| this.pixelRatio = EXTENT / size; | |||
There was a problem hiding this comment.
What's this used for? Is it expected that standard 4096 extent and 512 tile gives a 8.0 "pixel ratio"?
There was a problem hiding this comment.
It was used for part of the conversion from pixels to tile units. 37e540f cleans up how that happens and removes tile.pixelRatio.
|
@mourner good call. When I rendered mapbox streets with a pattern for every line and fill (patterns use pixelsToTileUnits more) the total time fir pixelsToTileUnits was 0.20%. It's hard to measure how long the shared inline version takes for comparison, but I think it's small enough that it's fine. |
@lucaswoj nope, it doesn't resolve it. The fill test is slightly closer (0.0002 instead of 0.0005) but the bug is still there. |
ansis
force-pushed
the
variable-extent
branch
from
37e540f to
2bd1840
Compare
|
does anyone want to review this more or is it good to merge? |
|
LGTM! |
ansis
force-pushed
the
variable-extent
branch
from
2bd1840 to
656a761
Compare
It converts all data to the maximum extent supported by our buffers and only uses that constant extent everywhere else.
adds tile.pixelsToTileUnits(pixelValue, zoom) and removes tile.pixelRatio
ansis
force-pushed
the
variable-extent
branch
from
656a761 to
282a12c
Compare
This supports layers with extent != 4096 by converting all data to the maximum extent (8192) supported by our buffers. After the conversion, only the constant maximum extent is used.
fixes #1093
If a layer has an extent > 8192 it will lose precision when it is converted down to 8192. It will render ok, but it will be less exact when it is very overscaled. In order to increase the maximum extent, we would need to:
extrudeorlinesofarvalues instead of the position. The reduce precision inextrudeorlinesofarcould cause issues.mapbox-gl-js/js/data/line_bucket.js
Lines 34 to 35 in 3280d6d
I'm not sure whether either of those changes are worthwhile right now. 8192 seems high enough
👀 @jfirebaugh @lucaswoj @mourner