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

Re-add the ability to get major edge lines. #26

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

kpreid
Copy link
Contributor

@kpreid kpreid commented Jul 29, 2024

  • Bump version from 14.0.0 (current release) to 14.1.0, because we're adding a method.
  • Added back get_major_edge_line_indices, which was removed by commit 9ff8564. However, it is (still) buggy (it misses the first and last index), so it is marked deprecated. This is just so that this can go into a release which is a minor rather than major version bump.
  • Added replacement get_major_edges_line_indices which returns the correct lines. However, it returns all relevant edges, rather than allowing selection of a single edge.
  • Removed unused use std::sync::Arc;.

* Bump version from 14.0.0 (current release) to 14.1.0, because
  we're adding a method.
* Added back `get_major_edge_line_indices`, which was removed by
  commit 9ff8564.
  However, it is (still) buggy (it misses the first and last index),
  so it is marked deprecated. This is just so that this can go into
  a release which is a minor rather than major version bump.
* Added replacement `get_major_edges_line_indices` which returns
  the correct lines. However, it returns *all* relevant edges, rather
  than allowing selection of a single edge.
* Removed unused `use std::sync::Arc;`.
@OptimisticPeach
Copy link
Owner

Howdy -- this looks good to me. I'll try to make sure to remove the deprecated function next time I'm pushing a major revision.

I'll get this merged in.

@OptimisticPeach OptimisticPeach merged commit 1155af3 into OptimisticPeach:master Jul 29, 2024
jleibs pushed a commit to rerun-io/rerun that referenced this pull request Jul 30, 2024
…#7014)

### What

Instead of an icosahedron, use an octahedron. The edges of the
octahedron become axis-aligned great circles around the subdivided mesh,
which makes more sense for ellipsoid visualization since the axes and
their perpendicular planes are significant.

![Screenshot 2024-07-29 at 15 13
53](https://github.com/user-attachments/assets/4c4d90f8-f590-4949-8e84-1102ab2c17e3)

This will make it easy to also choose to draw *only* the axis-aligned
great circles (once
OptimisticPeach/hexasphere#26 is available),
making this a step towards implementing #6962.

### 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)
* [ ] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7014?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7014?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
* [X] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [X] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7014)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.
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.

2 participants