-
Notifications
You must be signed in to change notification settings - Fork 88
add downsample prop to VictoryZoomContainer #503
Conversation
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.
Approved pending extracting downsampling method
@@ -70,6 +78,10 @@ export const zoomContainerMixin = (base) => class VictoryZoomContainer extends b | |||
} | |||
}]; | |||
|
|||
constructor(props) { |
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.
Why was this added?
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 total mistake!
// core downsampling algo | ||
|
||
// important: assumes data is ordered by dimension | ||
// get the start and end of the data that is in the current visible domain |
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.
You can check on the sortKey
prop
…wnsampleZoomFriendly
b41a4ee
to
40b5e8a
Compare
Requires FormidableLabs/victory-core#276 now |
if (startIndex !== 0) { startIndex -= 1; } | ||
if (endIndex !== -1) { endIndex += 1; } | ||
|
||
const visableData = data.slice(startIndex, endIndex); |
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.
NIT: visableData
-> visibleData
downsample
prop onVictoryZoomContainer
.Downsamples according to the
dimension
prop (defaults to"x"
if dimension is not set).Assumes data is sorted by dimension. (which is the case with almost all large time-series data) Need to make sure we note this in the docs.
Performance gains are pretty great...
Before: 1,000 points render at 3.5 FPS.
After: 100,000 render at 8 FPS.