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.

Sign up for GitHub

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

Jump to bottom

Plot becomes jagged with min-max aggregation #4969

Open
emilk opened this issue Jan 30, 2024 · 1 comment
Open

Plot becomes jagged with min-max aggregation #4969

emilk opened this issue Jan 30, 2024 · 1 comment
Labels
😤 annoying Something in the UI / SDK is annoying to use egui Requires egui/eframe work 📈 plot Plots, charts, graphs, timeseries, …

Comments

@emilk
Copy link
Member

emilk commented Jan 30, 2024 •
edited
Loading

min-max aggregation causes a zig-zag line to be generated, causing jagged look of otherwise smooth lines:

Image

the solution is to instead output a thicker line, but that needs support in egui_plot.

We don't want to switch to another default aggregator though - min-max does great of showing the upper and lower bounds, which can be crucial.
@emilk emilk added 👀 needs triage This issue needs to be triaged by the Rerun team 😤 annoying Something in the UI / SDK is annoying to use egui Requires egui/eframe work and removed 👀 needs triage This issue needs to be triaged by the Rerun team labels Jan 30, 2024
@emilk emilk assigned emilk and unassigned emilk Jan 30, 2024
@Wumpf Wumpf mentioned this issue Jan 31, 2024
Merged
4 tasks
@emilk emilk self-assigned this Feb 1, 2024
emilk pushed a commit that referenced this issue Feb 1, 2024
@Wumpf
Clarify aggregation strings (#4987)
f321da9 
### What

* Fixes #4954
* Related to #4969

on hover plot:

![image](https://github.com/rerun-io/rerun/assets/1220815/f1583f64-42a6-4517-af4d-ad2fed1b2b06)

on hover settings:

![image](https://github.com/rerun-io/rerun/assets/1220815/0d5d2256-80a4-4a88-b370-5c5afa7a126c)


Also, made it default to `MinMaxAverage` since this looks better on all
my screens for smooth lines.


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4987/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4987/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4987/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4987)
- [Docs
preview](https://rerun.io/preview/141491880209d44706b04425ff8643f7e8dca9ef/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/141491880209d44706b04425ff8643f7e8dca9ef/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@emilk emilk mentioned this issue Feb 1, 2024
Merged
4 tasks
emilk added a commit that referenced this issue Feb 1, 2024
@emilk
Finer plot aggregation (#4998)
54d957d 
### What
* Workaround for #4969

We would previously aggregate the plots over a time-window equal in
width to a ui point.

The MinMax aggregator does a zig-zag between min and max, so this causes
a very jagged look, especially on high-dpi screens.

With this PR, we switch to using a physical pixels as the default
aggregation width, but for MinMax is is _half_ a pixel. This reduces the
jaggedness, but does not completely remove it. It comes at a cost though
of higher tessellation cost.

I have an idea for how we can throw rayon on the tessellation problem
though.

## Before
<img width="790" alt="Screenshot 2024-02-01 at 14 18 26"
src="https://github.com/rerun-io/rerun/assets/1148717/e12fd92c-819b-4ba6-9ce1-0b7e95a0e151">

## After
<img width="790" alt="Screenshot 2024-02-01 at 14 18 00"
src="https://github.com/rerun-io/rerun/assets/1148717/52c0f2e7-25d3-4f8b-b431-b4c91a8e67bf">


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using newly built examples:
[app.rerun.io](https://app.rerun.io/pr/4998/index.html)
* Using examples from latest `main` build:
[app.rerun.io](https://app.rerun.io/pr/4998/index.html?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[app.rerun.io](https://app.rerun.io/pr/4998/index.html?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4998)
- [Docs
preview](https://rerun.io/preview/17721aa538a03c890574d14795dce6502f8cc004/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/17721aa538a03c890574d14795dce6502f8cc004/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
@emilk emilk removed their assignment Feb 1, 2024
@emilk emilk mentioned this issue Apr 23, 2024
Closed
5 tasks
@emilk
Copy link
Member Author

emilk commented Apr 23, 2024 •
edited
Loading

We've had customers complain about the jagged lines, so perhaps the MinMax isn't a great default after all. It also seems pretty common to show a "simplified" (=potentially wrong) view of the data when you're zoomed out. Not that I think that's good, but maybe it's better than ugly looking plots.

EDIT: I've changed my mind again. Looking at how awful the plot aggregator in PostHog is I am now convinced that I rather have jagged, ugly plots, than misleading ones.

@emilk emilk added the 📈 plot Plots, charts, graphs, timeseries, … label Apr 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
😤 annoying Something in the UI / SDK is annoying to use egui Requires egui/eframe work 📈 plot Plots, charts, graphs, timeseries, …
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@emilk