Notification system

[setzer22]

Sometimes it is necessary to give a message to the user when something goes wrong (or succeeds). A good example is the "Export OBJ" node, which currently doesn't do any indication of either success or failure.

In most apps this is done by a notification system that reports messages in bubbles. The bubbles stay alive for a certain time and show the message. The user should be able to click and dismiss the bubble.

A good example of what is needed can be seen here:

This could also become a standalone helper crate for other egui apps

[inact1v1ty]

Hi @setzer22! I've been following this project for a while. In fact, shortly before this repository appeared, I myself was thinking of doing something like this (nodes + rust + integration into engines). 🙂 I would like to tackle this issue (always wanted to mess up with egui).

[CatCode79]

There are already a couple of crates that would do for us: https://crates.io/crates/egui-toast https://crates.io/crates/egui-notify I don't know them well though and I can't tell you the pros and cons.

[inact1v1ty]

Oh, that prooved to be mush easier than I thought, thanks @CatCode79 🙂

[setzer22]

Hi @inact1v1ty! :slightly_smiling_face: I'm happy to know people are following the project, it's always nice to see people show up!

Sorry it took me a while to reply. Currently busy with lots of stuff, and doing some very minimal blackjack work in the background.

Integrating egui-toast or egui-notify sounds like a great idea. Is this something you'd like to tackle? I could give you a few pointers:

• This is code that would need to live inside the blackjack_ui crate.
• In particular, I think the RootViewport struct defined in application.rs would be a good spot to store this.
• In its update method, there is a TODO (line 236) that should show a notification when there's an error. The current behavior where it panics is not nice. I want to avoid panics in blackjack as much as possible because they're very disruptive for the user (total crash, loose unsaved work).
• The AppRootAction enum could get an extra variant ShowNotification so that the app_context can also send notifications by pushing this action to the list.
• On application_context.rs, line ~70, you will see there's an eprintln call. This error should be shown in a notification toast, not just printed to stderr.

There are other things related to error reporting that can be made nicer. But this should be a good starting point :smile:

[inact1v1ty]

Hello again @setzer22! Sadly I was not able to start working on the issue before due to personal and world-country-situation reasons. Now I'm safe and sound and have time and energy, so I'm able to continue contributing to open source and your amazing project in particular! Thank you for your tips, I'll come back later with some code done 🙂

[setzer22]

Glad to hear everything's better now 🙂 Looking forward to your changes! Let me know if you find any issues.

[inact1v1ty]

Hi @setzer22! Both libraries are using egui 0.19, so I decided to try to bump the version in blackjack and contribute it as a pr also. I have bumped egui, glam, rend3, wgpu, winit, and egui_node_editor and etc and have successfully fixed almost all compilation errors. I have one question though, the not-forked version of egui-wgpu doesn't have zoom_level and resolution arguments in RenderPass::execute_with_renderpass method. Is there a way to work around that that you may now?

[inact1v1ty]

Okay, there are one more such thing in egui_winit 😞 The WindowOrSize type in take_egui_input. I think I need some help from you: should I try to work around that and how, or should I bump version in your fork and use it. Maybe if your changes fix some important things that can't be worked around without them I should pr these changes to upstream egui crates 🤔

For both your changes I can make code compile without them, but as I understand they were made to mitigate some erroneous runtime behaviour, and I don't know exactly what behaviour and how.

[inact1v1ty]

Just to clarify - in the PR with egui-0.19 I was able to work around the API changes: both RenderPass::execute_with_renderpass signature and lack of WindowOrSize. So, waiting for your review 🙂

[setzer22]

the not-forked version of egui-wgpu doesn't have zoom_level and resolution arguments in RenderPass::execute_with_renderpass method. Is there a way to work around that that you may now?

I introduced these changes back when this code lived in egui_wgpu_backend around egui 0.15, and at the time this was necessary to get the right zooming behavior. But everything seems to work without it now? So maybe there has been some change that fixed the original issue I wasn't aware of :thinking: I need to investigate a bit further but as long as it works, the less forks we need the better!

[inact1v1ty]

Well, got back to working on this and I have following thoughts:

• egui-notify has bad visual representation of multiline notifications and also doesn't support custom toasts - which means no buttons with actions or etc.
• egui-toast suits better, but requires manual anchor calculation and doesn't have animations, which is a big deal for me personally (I really into making UI feel juicy with animations and have even developed a Unity animation library).

Also, both these libraries are not so complicated. And one more thing - blackjack for a lot things spills out info every frame, so we have both to track already spawned toasts and the library tracks toasts internally.

So, I think that for a program targeting to become a day-to-day tool will benefit from custom notification solution with buttons for interaction, animations and etc. This is my plan for now - to implement custom notifications for blackjack and maybe upstream some features from it to those two libraries.

[setzer22]

And one more thing - blackjack for a lot things spills out info every frame

The plan is to not necessarily recompute everything every frame. Specifically, I don't think the node graph should run and the mesh data get recomputed every frame at 60 frames per second. This is just how things are right now because I haven't gotten around to implement caching or change detection.

This is my plan for now - to implement custom notifications for blackjack and maybe upstream some features from it to those two libraries.

Indeed, with big software like this sometimes it makes more sense to bring in dependencies into the codebase so we can have better control and provide a more custom user experience.

If integrating requires substantial changes, I think it's fine to just bring the code into blackjack_ui. Rust can be a bit limiting when interacting with external dependencies with everything being private by default and the orphan rules. Just remember to check the licenses and always credit the original authors if you take code from other crates :+1: