Skip to content
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

feature: Rework network y-axis, linear interpolation for off-screen data #437

Merged
merged 26 commits into from
Apr 4, 2021

Conversation

ClementTsang
Copy link
Owner

@ClementTsang ClementTsang commented Mar 14, 2021

Description

A description of the change and what it does. If relevant (such as any change that modifies the UI), please provide screenshots of the change:

Rewrite of the y-axis labeling and scaling for the network widget, along with more customization. This still has one step to be optimized (cache results so we don't have to recalculate the legend each time), but will be done in another PR for sake of this one being too large already.

Furthermore, this change adds linear interpolation at the 0 point in the case a data point shoots too far back - this seems to have lead to ugly gaps to the left of graphs in some cases, because the left hand limit was not big enough for the data point. We address this by grabbing values just outside the time range and linearly interpolating at the leftmost limit. This affects all graph widgets (CPU, mem, network).

This can be optimized, and will hopefully be prior to release in a separate change.

Current look:
image

To do:

  • Cache computation results if possible Skipped for MVP, I'll do this directly after along with some optimizations
  • Details of change
  • Documentation changes (TODO: remember to update gif later before release)
  • Testing
    • Log
    • Normal/linear
    • Bytes
    • Bits
    • Config file + command line
    • Decimal prefix
    • Binary prefix
    • Resize properly

Issue

If applicable, what issue does this address?

Closes: #96

Type of change

Remove the irrelevant ones:

  • New feature (non-breaking change which adds functionality)

Test methodology

If relevant, please state how this was tested:

Furthermore, please tick which platforms this change was tested on:

  • Windows
  • macOS
  • Linux

If relevant, all of these platforms should be tested.

Checklist

If relevant, ensure the following have been met:

  • Change has been tested to work, and does not cause new breakage unless intended
  • Code has been self-reviewed
  • Documentation has been added/updated if needed (README, help menu, etc.)
  • Passes CI pipeline (clippy check and cargo test check)
  • Areas your change affects have been linted using rustfmt (cargo fmt)
  • No merge conflicts arise from the change

Other information

Provide any other relevant information to this change:

@ClementTsang ClementTsang changed the title feature: Add network y-axis scaling & unit options feature: Dynamic network y-axis, add scaling & unit options Mar 14, 2021
@ClementTsang ClementTsang force-pushed the customizable_network_axis branch from 3f9fa3e to 2495ef0 Compare March 27, 2021 20:21
@ClementTsang ClementTsang marked this pull request as ready for review April 4, 2021 06:37
/// Gotta get partial ordering? No problem, here's something to deal with it~
///
/// Note that https://github.com/reem/rust-ordered-float exists, maybe move to it one day? IDK.
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned by the comment, we can probably replace this with OrderedFloat when we do a refactor in regards to the interpolation data structures.

DataUnit::Bit => (
current_data.network_harvest.rx,
current_data.network_harvest.tx,
current_data.network_harvest.total_rx / 8, // We always make this bytes...
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we switch to getting bytes from harvester, remember to update this too!

None // There is no point.
};

// TODO: Cache network results: Only update if:
Copy link
Owner Author

@ClementTsang ClementTsang Apr 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the future (i.e. after this PR) to optimize; currently we do this full legend calculation throughout the entire set of data points each redraw cycle PER widget, which sounds costly at full data capacity. Ideally we can re-use some max entry calculations too (or change how we do interpolation + data point storage to make this faster).

@ClementTsang ClementTsang changed the title feature: Dynamic network y-axis, add scaling & unit options feature: Dynamic network y-axis, add scaling & unit options, add linear interpolation to data points at 0 Apr 4, 2021
@ClementTsang ClementTsang changed the title feature: Dynamic network y-axis, add scaling & unit options, add linear interpolation to data points at 0 feature: Rework legend y-axis, add linear interpolation to data points off screen Apr 4, 2021
@ClementTsang ClementTsang changed the title feature: Rework legend y-axis, add linear interpolation to data points off screen feature: Rework network y-axis, add linear interpolation to data points off screen Apr 4, 2021
@ClementTsang ClementTsang changed the title feature: Rework network y-axis, add linear interpolation to data points off screen feature: Rework network y-axis, linear interpolation for off-screen data Apr 4, 2021
@ClementTsang ClementTsang merged commit eb6a737 into master Apr 4, 2021
@ClementTsang ClementTsang deleted the customizable_network_axis branch April 4, 2021 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add customization to network range in Y-axis
1 participant