Suggest users open an issue on crash, and other fixes (#1993)

* Print link to github issues on crash

* Add `rerun --version` output to bug report issue template

* Hide `venv` folder from vscode

* Add instruction for how to do non-uniform plot zoom

* Warn about how the native_viewer feature takes a lot of compilation time

* fix typo

* Remove some unwraps

* Experiment with enabling the clippy::unwrap_used lint, but no

.github/ISSUE_TEMPLATE/bug_report.md

@@ -33,5 +33,8 @@ Steps to reproduce the behavior:
 **Desktop (please complete the following information):**
  - OS: <!-- e.g. macOS Monterey 12.6 -->
 
+**Rerun version**
+<!-- Paste the output of `rerun --version` here. -->
+
 **Additional context**
 <!-- Add any other context about the problem here. -->

.vscode/settings.json

@@ -8,10 +8,10 @@
     "files.insertFinalNewline": true,
     "files.trimTrailingWhitespace": true,
     "files.exclude": {
-        "env/**": true,
-        "target/**": true,
         "target_ra/**": true,
         "target_wasm/**": true,
+        "target/**": true,
+        "venv/**": true,
     },
     "files.autoGuessEncoding": true,
     "python.formatting.provider": "black",

Cranky.toml

@@ -138,4 +138,8 @@ allow = [
   "clippy::missing_errors_doc",
   "trivial_casts",
   "unused_qualifications",
+
+  # This would be nice to enable, but we have way too many unwraps in our arrow store 😭
+  # Enabling this lint in 2023-04-27 yielded 556 hits.
+  "clippy::unwrap_used",
 ]

clippy.toml

@@ -2,6 +2,7 @@
 
 msrv = "1.67"
 
+allow-unwrap-in-tests = true
 
 # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_macros
 disallowed-macros = [

crates/re_build_build_info/src/lib.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::unwrap_used)]
+
 //! This crate is to be used from `build.rs` build scripts.
 //!
 //! Use this crate together with the `re_build_info` crate.

crates/re_build_web_viewer/src/lib.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::unwrap_used)]
+
 //! Build the Rerun web-viewer .wasm and generate the .js bindings for it.
 
 use cargo_metadata::camino::Utf8PathBuf;

crates/re_renderer/build.rs

@@ -12,6 +12,8 @@
 
 // TODO(cmc): this should only run for release builds
 
+#![allow(clippy::unwrap_used)]
+
 use std::path::Path;
 
 use walkdir::{DirEntry, WalkDir};

crates/re_tensor_ops/tests/tensor_tests.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::unwrap_used)]
+
 use re_log_types::component_types::{
     Tensor, TensorCastError, TensorData, TensorDataMeaning, TensorDimension, TensorId,
 };

crates/re_ui/src/design_tokens.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::unwrap_used)] // fixed json file
+
 use egui::Color32;
 
 /// The look and feel of the UI.

crates/re_viewer/src/misc/transform_cache.rs

@@ -282,7 +282,8 @@ fn inverse_transform_at(
                     *encountered_pinhole = true;
 
                     // TODO(andreas): If we don't have a resolution we don't know the FOV ergo we don't know how to project. Unclear what to do.
-                    if let Some(resolution) = pinhole.resolution() {
+                    if let (Some(resolution), Some(fov_y)) = (pinhole.resolution(), pinhole.fov_y())
+                    {
                         let translation = pinhole.principal_point().extend(-100.0); // Large Y offset so this is in front of all 2d that came so far. TODO(andreas): Find better solution
                         Ok(Some(
                             glam::Mat4::from_scale_rotation_translation(
@@ -291,7 +292,7 @@ fn inverse_transform_at(
                                 glam::Quat::IDENTITY,
                                 translation,
                             ) * glam::Mat4::perspective_infinite_lh(
-                                pinhole.fov_y().unwrap(),
+                                fov_y,
                                 pinhole.aspect_ratio().unwrap_or(1.0),
                                 0.0,
                             ),

crates/re_viewer/src/ui/view_spatial/ui_3d.rs

@@ -327,7 +327,7 @@ pub fn view_3d(
 
         view_from_world: eye.world_from_view.inverse(),
         projection_from_view: Projection::Perspective {
-            vertical_fov: eye.fov_y.unwrap(),
+            vertical_fov: eye.fov_y.unwrap_or(Eye::DEFAULT_FOV_Y),
             near_plane_distance: eye.near(),
         },
 

crates/re_viewer/src/ui/view_time_series/scene.rs

@@ -75,6 +75,7 @@ impl SceneTimeSeries {
         self.load_scalars(ctx, query);
     }
 
+    #[inline(never)] // Better callstacks on crashes
     fn load_scalars(&mut self, ctx: &mut ViewerContext<'_>, query: &SceneQuery<'_>) {
         crate::profile_function!();
 
@@ -106,6 +107,8 @@ impl SceneTimeSeries {
             );
 
             for (time, ent_view) in ent_views {
+                let Some(time) = time else { continue; }; // scalars cannot be timeless
+
                 match ent_view.visit5(
                     |_instance,
                      scalar: component_types::Scalar,
@@ -121,7 +124,7 @@ impl SceneTimeSeries {
                         const DEFAULT_RADIUS: f32 = 0.75;
 
                         points.push(PlotPoint {
-                            time: time.unwrap().as_i64(), // scalars cannot be timeless
+                            time: time.as_i64(),
                             value: scalar.into(),
                             attrs: PlotPointAttrs {
                                 label,
@@ -148,10 +151,10 @@ impl SceneTimeSeries {
             // If all points within a line share the label (and it isn't `None`), then we use it
             // as the whole line label for the plot legend.
             // Otherwise, we just use the entity path as-is.
-            let same_label = |points: &[PlotPoint]| {
-                let label = points[0].attrs.label.as_ref();
-                (label.is_some() && points.iter().all(|p| p.attrs.label.as_ref() == label))
-                    .then(|| label.cloned().unwrap())
+            let same_label = |points: &[PlotPoint]| -> Option<String> {
+                let label = points[0].attrs.label.as_ref()?;
+                (points.iter().all(|p| p.attrs.label.as_ref() == Some(label)))
+                    .then(|| label.clone())
             };
             let line_label = same_label(&points).unwrap_or_else(|| entity_path.to_string());
 
@@ -163,6 +166,7 @@ impl SceneTimeSeries {
     // segments.
     // A line segment is a continuous run of points with identical attributes: each time
     // we notice a change in attributes, we need a new line segment.
+    #[inline(never)] // Better callstacks on crashes
     fn add_line_segments(&mut self, line_label: &str, points: Vec<PlotPoint>) {
         crate::profile_function!();
 

crates/re_viewer/src/ui/view_time_series/ui.rs

@@ -17,6 +17,8 @@ use super::SceneTimeSeries;
 pub(crate) const HELP_TEXT: &str = "Pan by dragging, or scroll (+ shift = horizontal).\n\
     Box zooming: Right click to zoom in and zoom out using a selection.\n\
     Zoom with ctrl / ⌘ + pointer wheel, or with pinch gesture.\n\
+    You can also zoom by dragging a rectangle with the right mouse button.\n\
+    \n\
     Reset view with double-click.\n\
     Right click to move the time cursor to the current position.";
 

crates/re_web_viewer_server/build.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::unwrap_used)]
+
 use std::collections::{HashMap, HashSet};
 
 use cargo_metadata::{CargoOpt, Metadata, MetadataCommand, Package, PackageId};

crates/rerun/Cargo.toml

@@ -40,6 +40,7 @@ global_session = ["re_sdk?/global_session"]
 image = ["re_log_types/image"]
 
 ## Support spawning a native viewer.
+## This adds a lot of extra dependencies, so only enable this feature if you need it!
 native_viewer = ["dep:re_viewer"]
 
 ## Support for running a HTTP server that listens to incoming log messages from a Rerun SDK.

crates/rerun/src/clap.rs

@@ -80,14 +80,12 @@ impl RerunArgs {
         run: impl FnOnce(Session) + Send + 'static,
     ) -> anyhow::Result<()> {
         // Ensure we have a running tokio runtime.
-        #[allow(unused_assignments)]
         let mut tokio_runtime = None;
         let tokio_runtime_handle = if let Ok(handle) = tokio::runtime::Handle::try_current() {
             handle
         } else {
-            tokio_runtime =
-                Some(tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"));
-            tokio_runtime.as_ref().unwrap().handle().clone()
+            let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime");
+            tokio_runtime.get_or_insert(rt).handle().clone()
         };
         let _tokio_runtime_guard = tokio_runtime_handle.enter();
 

crates/rerun/src/crash_handler.rs

@@ -59,7 +59,8 @@ fn install_panic_hook(_build_info: BuildInfo) {
 
         eprintln!(
             "\n\
-            Troubleshooting Rerun: https://www.rerun.io/docs/getting-started/troubleshooting"
+            Troubleshooting Rerun: https://www.rerun.io/docs/getting-started/troubleshooting\n\
+            Report bugs: https://github.com/rerun-io/rerun/issues"
         );
 
         #[cfg(feature = "analytics")]
@@ -135,6 +136,17 @@ fn install_signal_handler(build_info: BuildInfo) {
     }
 
     unsafe extern "C" fn signal_handler(signal_number: libc::c_int) {
+        fn print_problem_and_links(signal_name: &str) {
+            write_to_stderr("Rerun caught a signal: ");
+            write_to_stderr(signal_name);
+            write_to_stderr("\n");
+            write_to_stderr(
+                "Troubleshooting Rerun: https://www.rerun.io/docs/getting-started/troubleshooting\n",
+            );
+            write_to_stderr("Report bugs: https://github.com/rerun-io/rerun/issues\n");
+            write_to_stderr("\n");
+        }
+
         let signal_name = match signal_number {
             libc::SIGABRT => "SIGABRT",
             libc::SIGBUS => "SIGBUS",
@@ -150,15 +162,10 @@ fn install_signal_handler(build_info: BuildInfo) {
         // but writing to stderr is one of them.
         // So we first print out what happened to stderr so we're sure that gets out,
         // then we do the unsafe things, like logging the stack trace.
-        // We take care not to allocate any memory along the way.
-
-        write_to_stderr("\n\n");
-        write_to_stderr("Rerun caught a signal: ");
-        write_to_stderr(signal_name);
-        write_to_stderr("\n\n");
-        write_to_stderr(
-            "Troubleshooting Rerun: https://www.rerun.io/docs/getting-started/troubleshooting\n\n",
-        );
+        // We take care not to allocate any memory before we generate the call stack.
+
+        write_to_stderr("\n");
+        print_problem_and_links(signal_name);
 
         // Ok, we printed the most important things.
         // Let's do less important things that require memory allocations.
@@ -167,7 +174,13 @@ fn install_signal_handler(build_info: BuildInfo) {
 
         let callstack = callstack();
         write_to_stderr(&callstack);
+        write_to_stderr("\n");
 
+        // Let's print the important stuff _again_ so it is visible at the bottom of the users terminal:
+        write_to_stderr("\n");
+        print_problem_and_links(signal_name);
+
+        // Send analytics - this also sleeps a while to give the analytics time to send the event.
         #[cfg(feature = "analytics")]
         {
             let build_info = BUILD_INFO
@@ -176,15 +189,6 @@ fn install_signal_handler(build_info: BuildInfo) {
             send_signal_analytics(build_info, signal_name, callstack);
         }
 
-        // Let's print the important stuff _again_ so it is visible at the bottom of the users terminal:
-        write_to_stderr("\n");
-        write_to_stderr("Rerun caught a signal: ");
-        write_to_stderr(signal_name);
-        write_to_stderr("\n");
-        write_to_stderr(
-            "Troubleshooting Rerun: https://www.rerun.io/docs/getting-started/troubleshooting\n\n",
-        );
-
         // We are done!
         // Call the default signal handler (which usually terminates the app):
         // SAFETY: we're calling a signal handler