-
Notifications
You must be signed in to change notification settings - Fork 8
.net5.0, Removes the use of Newtonsoft.Json in favor of System.Text.J… #18
base: develop
Are you sure you want to change the base?
Conversation
…son, it also removes the use of Tuples for (X,Y(Z)) Structures and instead uses System.Numerics.Vector which cuts down on the amount of reference parameters (register allocation) at the call site as well as increases performance for mathematical operations associated.
This should be what you were looking for @xivk it also contains the work of @FObermaier |
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 think that Tile needs some work to use 3 backing vectors rather than transporting around 12 doubles, I think perhaps float should also be sufficient rather than double.
If that is okay then I would change TileMath to use MathF instead of Math as well as in a few other places (WebMercatorHandler)
See https://github.com/juliusfriedman/NetTopologySuite.IO.VectorTiles/tree/VectorizeTile for a branch which attempts this.
The only thing I noticed is that one of the unit tests failed with an off by 1 issue after changing to float.
The most immediate optimization I can think of in that branch would be to modify CalculateBounds
and the Tile
constructor to create the vectors only one time as right now they are created twice because I put Zoom in the wrong vector afg...
this.X = x; | ||
this.Y = y; | ||
this.Zoom = zoom; | ||
_id = Tile.CalculateTileId(this.Zoom = zoom, this.X = x, this.Y = y); |
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.
TODO: Should be a single vector backing these
var n = Math.PI - ((2.0 * Math.PI * this.Y) / Math.Pow(2.0, this.Zoom)); | ||
this.Left = (double) ((this.X / Math.Pow(2.0, this.Zoom) * 360.0) - 180.0); | ||
this.Top = (double) (180.0 / Math.PI * Math.Atan(Math.Sinh(n))); | ||
var zoomSquare = Math.Pow(2.0, this.Zoom); |
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.
TODO: Should be a single vector backing these
this.Y.GetHashCode() ^ | ||
this.Zoom.GetHashCode(); | ||
} | ||
public override int GetHashCode() => HashCode.Combine(X.GetHashCode(), Y.GetHashCode(), Zoom.GetHashCode()); |
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.
Would be the individual backing vector hashcodes.
.net5.0, Removes the use of Newtonsoft.Json in favor of System.Text.Json, it also removes the use of Tuples for (X,Y(Z)) Structures and instead uses System.Numerics.Vector which cuts down on the amount of reference parameters (register allocation) at the call site as well as increases performance for mathematical operations associated.