Repository navigation
Tile columns cannot be zero when rounding - #2031
Conversation
lossyrob
left a comment
lossyrob
left a comment
There was a problem hiding this comment.
This is a good fix; just looking for a small change.
Have you signed the eclipse CLA with your github email?
| val targetCols = math.round(extent.width / cellwidth).toLong | ||
| val targetRows = math.round(extent.height / cellheight).toLong | ||
| val targetCols = 1L.max(math.round(extent.width / cellwidth).toLong) | ||
| val targetRows = 1L.max(math.round(extent.height / cellheight).toLong) |
There was a problem hiding this comment.
Thanks for the contribution!
This is a nitpick, but I think using math.max directly would be more optimal.
The 1L.max is a method implicitly added via RichLong (implicit here), which extends ScalaNumberProxy via IntegralProxy[Long], which is an unspecialized class. So, I would assume this would box. However, looking at the byte code, I don't see that it does:
Scala:
def withImplicit: Long =
1L.max(2L)
def withoutImplicit: Long =
math.max(1L, 2L)bytecode:
public long withImplicit();
Code:
0: getstatic #66 // Field scala/runtime/RichLong$.MODULE$:Lscala/runtime/RichLong$;
3: getstatic #71 // Field scala/Predef$.MODULE$:Lscala/Predef$;
6: lconst_1
7: invokevirtual #75 // Method scala/Predef$.longWrapper:(J)J
10: ldc2_w #76 // long 2l
13: invokevirtual #81 // Method scala/runtime/RichLong$.max$extension:(JJ)J
16: lreturn
public long withoutImplicit();
Code:
0: getstatic #87 // Field scala/math/package$.MODULE$:Lscala/math/package$;
3: lconst_1
4: ldc2_w #76 // long 2l
7: invokevirtual #90 // Method scala/math/package$.max:(JJ)J
10: lreturn
Not sure how this actually doesn't box...(sorry 'thinking out loud' here)
However, we do pay the price of the longWrapper call, which instantiates a class. So, the math.max direct call would be faster than the 1l.max call. By how much? Surely not a lot. But given one of two options, I'll always choose the faster one (and the one that produces less byte code) :)
Can you switch to that method?
There was a problem hiding this comment.
The Eclipse CLA is signed as [email protected]
snoe925
left a comment
snoe925
left a comment
There was a problem hiding this comment.
Changed to math.max
|
👍 |
2 participants
These round to zero values cause zero column tiles in a LatLng to WebMercator reproject. I could not figure out the root cause. It looked to me like reasonable values were used, and that rounding was the issue. An alternative fix might be to clip the data to the maximum extent of WebMercator in the projection.