Skip to content

Conversation

@bircni
Copy link
Contributor

@bircni bircni commented Feb 8, 2025

This pull request introduces a new "Userdata" demo panel to the plot demo, showcasing how custom per-point user data can be attached and accessed during label formatting. It also refactors the plot item geometry to include an optional Id, allowing label formatters to receive both the item identifier and point index for richer tooltip customization. Several snapshot images are updated to reflect these demo and API changes.

Userdata demo and API enhancements:

  • Added a new Userdata demo panel to PlotDemo, demonstrating how to associate custom information with each plotted point and display it in tooltips using the label formatter.
  • Refactored PlotGeometry::Points to carry an optional Id, enabling identification of the plot item during hover interactions.
  • Updated the label formatter API to receive an optional tuple of (Id, index), allowing tooltips to display custom per-point data.
  • Modified all plot item implementations (Line, Polygon, Points, Arrows) to provide their Id in the geometry for correct user data association.

These changes make it easier to build interactive plots with custom per-point data and tooltips, and provide a clear demo for users to follow.

Reopening emilk/egui#4719

@bircni
Copy link
Contributor Author

bircni commented May 4, 2025

@lucasmerlin @emilk could you have a look at it?
The issue has several upvotes

@bircni bircni force-pushed the plot-point-userdata branch 2 times, most recently from fffbf1d to 472cf91 Compare August 8, 2025 19:04
@bircni
Copy link
Contributor Author

bircni commented Aug 8, 2025

@emilk Could you have a look at it again - I think I fixed everything now :-)

@bircni bircni force-pushed the plot-point-userdata branch from 472cf91 to 7d168be Compare August 8, 2025 19:09
@bircni bircni requested a review from emilk August 10, 2025 14:40
@bircni bircni force-pushed the plot-point-userdata branch from 7d168be to dbb93ee Compare November 7, 2025 13:19
@bircni
Copy link
Contributor Author

bircni commented Nov 7, 2025

@emilk rebased onto main - ready for a re-review again :-)

@michalsustr michalsustr self-assigned this Nov 22, 2025
@michalsustr
Copy link
Collaborator

I like the idea of this PR but I am super confused. It seems the label formatter doesn't update the tooltip with this data?

{}\nPosition: ({:.3}, {:.3})\nLabel: {}\nImportance: {:.1}%

There is just label and (x,y) - no extra new stuff.
image

Copy link
Contributor

@kitizz kitizz left a comment

Choose a reason for hiding this comment

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

Heading in the right direction. It seems like a fairly lightweight addition to the core logic. But as it stands there's a few issues:

  • [Blocker] Easy-to-fix bug that causes item to always be None
  • [Strong recommend] Remove the Optional wrapper from (Id, usize)
  • [Strong recommend] Refactor the demo to avoid the need for Arc and Mutex and make it easier to understand for newcomers.

Assuming the optional is dropped, here's an alternative approach that doesn't need Arc and Mutex. And maybe even more importantly, I think it's easier for the reader to understand the flow of the logic on first parse:

// ...

        // Sine data.
        let sine_points = (0..=500) // etc...
        let sine_id = Id::new("sine_wave");
        let sine_line = Line::new(
            "sin(x)",
            sine_points.iter().map(|p| [p.x, p.y]).collect::<Vec<_>>(),
        )
        .id(sine_id)
        .color(Color32::from_rgb(200, 100, 100));

        // Cosine data.
        let cosine_points = (0..=500) // etc...
        let cosine_id = Id::new("cosine_wave");
        let cosine_line = Line::new(
            "cos(x)",
            cosine_points.iter().map(|p| [p.x, p.y]).collect::<Vec<_>>(),
        )
        .id(cosine_id)
        .color(Color32::from_rgb(100, 200, 100));

        // Damped sine data.
        let damped_points = (0..=500) // etc...
        let damped_id = Id::new("damped_wave");
        let dampled_line = Line::new(
            "e^(-0.5x) · sin(2x)",
            damped_points.iter().map(|p| [p.x, p.y]).collect::<Vec<_>>(),
        )
        .id(damped_id)
        .color(Color32::from_rgb(100, 100, 200));

        // Store custom data for easy retrieval. Performance can be improved by caching this in the `UserdataDemo` struct.
        let custom_data = HashMap::from([
            (sine_id, sine_points),
            (cosine_id, cosine_points),
            (damped_id, damped_points),
        ]);

        Plot::new("Userdata Plot Demo")
            .legend(Legend::default().position(Corner::LeftTop))
            .label_formatter(move |name, value, (id, index)| {
                if let Some(points) = custom_data.get(&id)
                    && let Some(point) = points.get(index)
                {
                    format!(
                        "{}\nPosition: ({:.3}, {:.3})\nLabel: {}\nImportance: {:.1}%",
                        name,
                        value.x,
                        value.y,
                        point.custom_label,
                        point.importance * 100.0
                    )
                } else {
                    format!("{}\n({:.3}, {:.3})", name, value.x, value.y)
                }
            })
            .show(ui, |plot_ui| {
                // Sine wave with custom data
                plot_ui.line(sine_line);

                // Cosine wave with custom data
                plot_ui.line(cosine_line);

                // Damped sine wave with custom data
                plot_ui.line(dampled_line);
                
                // ... the rest ...

You could even include the Line objects in the HashMap, and loop over them in .show() instead of plot_ui.line(...) separately. But doesn't really offer a lot of value in this demo.

let lock = custom_data_.lock();
if let Some(points) = lock.get(&id) {
if let Some(point) = points.get(index) {
return format!(
Copy link
Contributor

Choose a reason for hiding this comment

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

When I run this demo, this branch is never executed. Might need some debugging here.
Screenshot 2025-12-13 at 08 40 52

I haven't parsed it deeply yet, but I can see that the intention is that the .show() capture is called first, and populates custom_data. And then any hover event then calls the .label_formatter capture and queries custom_data_.
It makes me slightly nervous. Like, it makes sense that show() will be called before label_formatter, but that's not explicitly documented or guaranteed anywhere.

But I'm looking at this, and thinking about how I would debug it, and thinking it will be unpleasant. And that's usually not a good sign. I'm going to keep reading and wrap my head around the full (intended) flow here. And hopefully I can come up with a more constructive suggestion for an alternative design here.

Plot::new("Userdata Plot Demo")
.legend(Legend::default().position(Corner::LeftTop))
.label_formatter(move |name, value, item| {
if let Some((id, index)) = item {
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe item is always None here when I run the demo

@michalsustr
Copy link
Collaborator

@bircni please rebase.

Also I think the data doesn't have to be under an Arc & mutex lock. It is rendered in the same thread, just after its definition.

@kitizz I like your example a lot! Also, I am thinking if we even need the

.label_formatter(move |name, value, (id, index)| {

I think

.label_formatter(move |id, index| {

should be enough to retrieve the name, and x&y values?

@kitizz
Copy link
Contributor

kitizz commented Dec 13, 2025

True... would you accompany it by a helper function that can retrieve any metadata?

struct PointId { // or some other better name
    plot_id: Id,
    index: usize,
}

and then (I don't know exactly where it should live)

fn plot_point(point_id: PointId) -> PlotPoint;
fn plot_name(point_id: PointId) -> String;

Also, confirming that this kind of breaking change is acceptable to land?

@bircni
Copy link
Contributor Author

bircni commented Dec 14, 2025

Just started rebasing in #223
Feel free to help :-)

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.

custom data for hover label

4 participants