setzer22 / egui_node_graph

Build your node graph applications in Rust, using egui
MIT License
718 stars 134 forks source link

Update to egui 0.20 #79

Closed kkngsm closed 1 year ago

kkngsm commented 1 year ago
kkngsm commented 1 year ago

Now I am working on trunk support

kkngsm commented 1 year ago

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 commented 1 year ago

No worries :slightly_smiling_face:

kkngsm commented 1 year ago

Details

kkngsm commented 1 year ago

Support trunk this commit includes favicon file etc.

kkngsm commented 1 year ago

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

setzer22 commented 1 year ago

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. :smile: We just need to figure out a way to have both things working.

setzer22 commented 1 year ago

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. egui_node_graph_interaction_bug

kkngsm commented 1 year ago

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 commented 1 year ago

Hi,

Is there any update about that?

kkngsm commented 1 year ago

No, it has not been corrected since then.

setzer22 commented 1 year ago

@ferid-akifzade I'm sorry, I haven't had time to debug this either :sweat_smile:. 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 :smile: new_guee

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 commented 1 year ago

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 commented 1 year ago

@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 😄 new_guee new_guee

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 commented 1 year ago

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

kkngsm commented 1 year ago

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. https://github.com/setzer22/egui_node_graph/blob/89186a6888dd824312b6ea9143106163990df764/egui_node_graph/src/editor_ui.rs#L167 https://github.com/setzer22/egui_node_graph/blob/89186a6888dd824312b6ea9143106163990df764/egui_node_graph/src/editor_ui.rs#L794-L798

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. https://github.com/setzer22/egui_node_graph/blob/89186a6888dd824312b6ea9143106163990df764/egui_node_graph/src/editor_ui.rs#L521-L580

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

hakolao commented 1 year ago

Thinking https://github.com/emilk/egui/issues/2947 and https://github.com/emilk/egui/issues/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 commented 1 year ago

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 commented 1 year ago

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