Update to egui 0.20

[kkngsm]

• update to egui 0.20
• replace Stroke::none() with Stroke::None

[kkngsm]

Now I am working on trunk support

[setzer22]

Thanks! Looks like this was an easy upgrade :) EDIT: See below 😅

[setzer22]

Ooops, sorry about the hasty review. I have pulled the changes and it looks like the upgrade may take a bit more work than this. If you try running the examples and moving things around you'll see it's no longer working. You can't move nodes or alter their values.

This is probably related to this breaking change in 0.20:

⚠️ BREAKING: if you have overlapping interactive widgets, only the top widget (last added) will be interactive (emilk/egui#2244).

I suspect how this is an issue, since the way we were doing background click detection conflicts with this new way of handling events in egui. Shouldn't be a difficult fix but unfortunately I don't have a lot of time to debug this right now.

[kkngsm]

I'm sorry...
I mistakenly thought it was a bug caused by trunk and was in the process of trying to figure out a fix.

It was an overall bug.

[setzer22]

No worries 🙂

[kkngsm]

Details

• Fixed bug of interaction
  Changed node to be drawn by Area and Frame above it. This allows the interaction to work.

• Fixed node display
  Changed to draw from Frame information and removed body_bottom Shape.　

[kkngsm]

Support trunk
this commit includes favicon file etc.

[kkngsm]

Oh... sorry.
I can't frame out the node, so I need to fix it a bit more.

[setzer22]

Yeah, the problem with using Area to draw the nodes is that an area is made to always be inside the screen, but the node editor needs to allow panning nodes outside the view.

But it seems you're going in a good direction. At least with this current version #71 is no longer an issue. 😄 We just need to figure out a way to have both things working.

[setzer22]

I just realized there's also another problem with using an area: It would allow nodes to be drawn on top of any other area like Windows, but we typically don't want that. Egui doesn't expose a way to control the layer order of areas, and will "raise" whichever area was interacted to the top.

It wouldn't be so much the nodes being "inside" the node editor but floating "above" it like every other window. This might cause trouble in certain applications.

[kkngsm]

I agree. It seems that using Area was not a good idea.

I'll give it some more thought. And, I feel that the other Frame and Trunk commits are useful and independent of egui0.20 and would be better split into a separate PR.

[ferid-akifzade]

Hi,

Is there any update about that?

[kkngsm]

No, it has not been corrected since then.

[setzer22]

@ferid-akifzade I'm sorry, I haven't had time to debug this either 😅. I've been a bit bugged by several of egui's limitations, so I am currently working on a project that drops down to epaint and implements a different, slightly more retained-mode, UI system on top. Whether this is going to be a project worth releasing or not, it's hard to tell at this time, but I have some initial results. Nothing too fancy, but hey! At least zoom is working there without my previous ugly hacks 😄

I'm still interested in updating this library, but since I am not currently using egui 0.20, it's a bit hard to find time to commit to this fix. This sounded like it should've been an easy fix, but when I tried it I realized it's not as easy as I thought because of the way the new way input events are consumed in 0.20.

[ledyba]

FYI: egui v0.21 has been released: egui - Rust

I tried this PR with egui v 0.21, I can't compile....

error[E0053]: method `data_type_color` has an incompatible type for trait
  --> src\editor\synth\nodes.rs:56:62
   |
56 |   fn data_type_color(&self, _user_state: &mut GraphState) -> egui::Color32 {
   |                                                              ^^^^^^^^^^^^^
   |                                                              |
   |                                                              expected struct `ecolor::color32::Color32`, found struct `Color32`
   |                                                              help: change the output type to match the trait: `ecolor::color32::Color32`
   |
   = note: expected fn pointer `fn(&DataType, &mut GraphState) -> ecolor::color32::Color32`
              found fn pointer `fn(&DataType, &mut GraphState) -> Color32`

[ferid-akifzade]

@ferid-akifzade I'm sorry, I haven't had time to debug this either 😅. I've been a bit bugged by several of egui's limitations, so I am currently working on a project that drops down to epaint and implements a different, slightly more retained-mode, UI system on top. Whether this is going to be a project worth releasing or not, it's hard to tell at this time, but I have some initial results. Nothing too fancy, but hey! At least zoom is working there without my previous ugly hacks 😄

I'm still interested in updating this library, but since I am not currently using egui 0.20, it's a bit hard to find time to commit to this fix. This sounded like it should've been an easy fix, but when I tried it I realized it's not as easy as I thought because of the way the new way input events are consumed in 0.20.

It looks great! I hope it will go well.

FYI: egui v0.21 has been released: egui - Rust

I tried this PR with egui v 0.21, I can't compile....

error[E0053]: method `data_type_color` has an incompatible type for trait
  --> src\editor\synth\nodes.rs:56:62
   |
56 |   fn data_type_color(&self, _user_state: &mut GraphState) -> egui::Color32 {
   |                                                              ^^^^^^^^^^^^^
   |                                                              |
   |                                                              expected struct `ecolor::color32::Color32`, found struct `Color32`
   |                                                              help: change the output type to match the trait: `ecolor::color32::Color32`
   |
   = note: expected fn pointer `fn(&DataType, &mut GraphState) -> ecolor::color32::Color32`
              found fn pointer `fn(&DataType, &mut GraphState) -> Color32`

I fixed most of the errors and can compile and run the corrected version. But like egui 0.20 version interacting with nodes not working. I don't know why.

[kkngsm]

We already know that Area is inappropriate, so I removed the code using Area.

[kkngsm]

The current problem is that I need stopPropagation() in javascript, and I don't know how to do it!

Currently, responses are obtained in two places.

egui_node_graph/egui_node_graph/src/editor_ui.rs

Line 167 in 89186a6

let r = ui.allocate_rect(ui.min_rect(), Sense::click().union(Sense::drag()));

egui_node_graph/egui_node_graph/src/editor_ui.rs

Lines 794 to 798 in 89186a6

	let window_response = ui.interact(
	outer_rect,
	Id::new((self.node_id, "window")),
	Sense::click_and_drag(),
	);

However, with the update of egui's event system, this no longer works.

Perhaps window_response was a bad hack. For example, I expect that we can use the Response of the Frame used to draw the node in this PR.

egui_node_graph/egui_node_graph/src/editor_ui.rs

Lines 521 to 580 in 89186a6

	Frame::none().inner_margin(margin).show(ui, |ui| {
	ui.vertical(|ui| {
	ui.horizontal(|ui| {
	ui.add(Label::new(
	RichText::new(&self.graph[self.node_id].label)
	.text_style(TextStyle::Button)
	.color(text_color),
	));
	ui.add_space(8.0); // The size of the little cross icon
	});
	title_height = ui.min_size().y;
	ui.add_space(margin.top);
	// First pass: Draw the inner fields. Compute port heights
	let inputs = self.graph[self.node_id].inputs.clone();
	for (param_name, param_id) in inputs {
	if self.graph[param_id].shown_inline {
	let height_before = ui.min_rect().bottom();
	if self.graph.connection(param_id).is_some() {
	ui.label(param_name);
	} else {
	// NOTE: We want to pass the `user_data` to
	// `value_widget`, but we can't since that would require
	// borrowing the graph twice. Here, we make the
	// assumption that the value is cheaply replaced, and
	// use `std::mem::take` to temporarily replace it with a
	// dummy value. This requires `ValueType` to implement
	// Default, but results in a totally safe alternative.
	let mut value = std::mem::take(&mut self.graph[param_id].value);
	let node_responses = value.value_widget(
	&param_name,
	self.node_id,
	ui,
	user_state,
	&self.graph[self.node_id].user_data,
	);
	self.graph[param_id].value = value;
	responses.extend(node_responses.into_iter().map(NodeResponse::User));
	}
	let height_after = ui.min_rect().bottom();
	input_port_heights.push((height_before + height_after) / 2.0);
	}
	}
	let outputs = self.graph[self.node_id].outputs.clone();
	for (param_name, _param) in outputs {
	let height_before = ui.min_rect().bottom();
	ui.label(&param_name);
	let height_after = ui.min_rect().bottom();
	output_port_heights.push((height_before + height_after) / 2.0);
	}
	responses.extend(
	self.graph[self.node_id]
	.user_data
	.bottom_ui(ui, self.node_id, self.graph, user_state)
	.into_iter(),
	);
	})
	});

But I've tried several patterns and they haven't worked.

[hakolao]

Thinking emilk/egui#2947 and emilk/egui#2471 are probably related. Debugged some:

        println!(
            "{:?} {:?}",
            window_response.drag_delta(),
            window_response.clicked_by(PointerButton::Primary)
        );

These always return 0.0, 0.0 and false.

One could probably hack around it

[setzer22]

Thank you all for the hard work, and sorry it took me a while to get back to this PR :) It seems #85 supersedes this?

[kkngsm]

Sorry for not being involved in the discussion lately. I think it's so good！