Implement notification window and binding feedback on interaction by davidwuluetang · Pull Request #2893 · f3d-app/f3d

davidwuluetang · 2026-02-17T20:31:24Z

Describe your changes

Implement binding notification window to bottom left

Added APIs to libf3d.interactor
Modified interactor_impl::TriggerBinding()
Added RenderNotifications() on vtkF3DImguiActor

Issue ticket number and link if any

#2578

Checklist for finalizing the PR

I have performed a self-review of my code
I have added tests for new features and bugfixes
I have added documentation for new features
If it is a modifying the libf3d API, I have updated bindings
If it is a modifying the .github/workflows/versions.json, I have updated docker_timestamp

Continuous integration

Please write a comment to run CI, eg: \ci fast.
See here for more info.

Screenshot

github-actions · 2026-02-17T20:33:16Z

You are modifying libf3d public API! ⚠️Please update bindings accordingly⚠️!
You can find them in their respective directories: c, python, java, webassembly.

mwestphal

please share a screenshot of how it looks.

* Use `duration` for notification instead of `timeElapsed`. * Reorganized window layout and shrink font size to 80% of default.

mwestphal · 2026-03-22T12:27:31Z

Still working on this @davidwuluetang ?

davidwuluetang · 2026-03-23T15:32:32Z

For the feature itself, the work is basically done. For the event loop freeze and slow down problem, I tried some workaround. Which adding another observer to VTKInteractor watch for InteractionEvent, but this require a timer feeding real delta time to event loop (not longer use a fix delta time). So, Idk this is a good solution or not.

mwestphal · 2026-03-23T15:35:10Z

For the feature itself, the work is basically done. For the event loop freeze and slow down problem, I tried some workaround. Which adding another observer to VTKInteractor watch for InteractionEvent, but this require a timer feeding real delta time to event loop (not longer use a fix delta time). So, Idk this is a good solution or not.

Ok, let us know if you need help with that.

davidwuluetang · 2026-03-23T15:48:33Z

I just push all code, maybe you guys can try it out, it work but Idk how this solution can effect other things. If this is not a acceptable solution, you can unassign me from this issue, cus I really don't think I can come up a solution for this problem.

mwestphal · 2026-03-23T16:03:24Z

I just push all code, maybe you guys can try it out, it work but Idk how this solution can effect other things. If this is not a acceptable solution, you can unassign me from this issue, cus I really don't think I can come up a solution for this problem.

Got it, we will take a look.

Meakk · 2026-03-24T07:15:14Z

      interactor.addBinding({mod_t::CTRL, "Q"}, "exit", "Others", std::bind(docString, "Quit"));
      // clang-format on

+      interactor.initBindNotificationMap();


The API name is too implementation oriented. We should use Map because the user doesn't care and we may change impl details.
What about initNotificationSystem()? Or just initialize it automatically with the interactor if possible?

I don't have opinion with name change.

Or just initialize it automatically with the interactor if possible?

This function do more than init, it clear the map and reload the callback as well. So, if users make change on the binding naming or callback, they need to call this function after the change in order for the change reflect on binding notifications. That why I make it public.

Meakk · 2026-03-24T07:16:52Z

+   * Add or modify binding documentation callback to binding notification map using command string
+   * as key.
+   */
+  void setBindNotiCallback(std::string command, documentation_callback_t doc_callback);


implementation only functions starts with an upper case letter.

Meakk · 2026-03-24T07:31:32Z

+    if (elapsed > duration)
+    {
+      it = Notifications.erase(it);
+    }


Should we move the logic of notification cleanup to vtkF3DUIActor::SetTotalTime? That way you ensure that the map is not growing infinitely if imgui is not enabled.

Meakk · 2026-03-24T07:42:39Z

-    this->EventLoopObserverId =
+    this->EventLoopObserverId[0] =
      this->VTKInteractor->AddObserver(vtkCommand::TimerEvent, timerCallBack);
+    this->EventLoopObserverId[1] =


Is it needed? If the goal is to update the time when pressing a binding, we could just update the time in TriggerBinding function

This part is the workaround I mention for event loop freeze issue on Windows system when interacting with camera, not really relate to the binding notification feature itself. It freeze the animation and the UITotalTime update, make the notification stay on screen if you interacting the camera.

Meakk · 2026-03-24T07:43:00Z

        internals* that = static_cast<internals*>(clientData);
-        that->EventLoop(that->CallbackDeltaTime);
+        double now = vtkTimerLog::GetUniversalTime();
+        double dt = now - that->LastTime;


Can you use deltaTime instead of computing dt here?

It will work as well, but animation will look slightly faster when deltaTime is 1/30. I tested on my machine it stay constant on 36-37 loops per second when interacting.

davidwuluetang · 2026-03-24T17:03:53Z

So, you guys decided to take out the work from here?

Meakk · 2026-03-25T09:57:10Z

Yes, see #2970

Meakk · 2026-03-25T15:58:47Z

Closing this one in favor of #2970
I'll keep you as co-author on this one.
Please feel free to make comments on it if I missed something important.

initial implementation

8bd963e

mwestphal self-requested a review February 17, 2026 21:37

mwestphal requested changes Feb 18, 2026

View reviewed changes

davidwuluetang added 11 commits February 18, 2026 15:01

changed window position and made text same line

57fcc1f

reorganized some APIs and make them internal

57614b0

added binding keypress in notification

d16e0b5

WIP

7f1360c

* Use `duration` for notification instead of `timeElapsed`. * Reorganized window layout and shrink font size to 80% of default.

push for testing

a895f74

use event loop time for calculation

c4b5825

add options for visibility

ffbee72

add language bindings

7a2e9c5

add testing

afd3952

add test baselines

507c0d5

add baseline

d49b428

mwestphal linked an issue Mar 22, 2026 that may be closed by this pull request

binding feedback on interaction #2578

Closed

eventloop slow issue workaround

caa7df6

mwestphal requested a review from Meakk March 23, 2026 16:03

Meakk reviewed Mar 24, 2026

View reviewed changes

Meakk closed this Mar 25, 2026

Uh oh!

Conversation

davidwuluetang commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes

Issue ticket number and link if any

Checklist for finalizing the PR

Continuous integration

Screenshot

Uh oh!

github-actions Bot commented Feb 17, 2026

Uh oh!

mwestphal left a comment

Choose a reason for hiding this comment

Uh oh!

mwestphal commented Mar 22, 2026

Uh oh!

davidwuluetang commented Mar 23, 2026

Uh oh!

mwestphal commented Mar 23, 2026

Uh oh!

davidwuluetang commented Mar 23, 2026

Uh oh!

mwestphal commented Mar 23, 2026

Uh oh!

Meakk Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

davidwuluetang Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Meakk Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Meakk Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Meakk Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

davidwuluetang Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Meakk Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

davidwuluetang Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

davidwuluetang commented Mar 24, 2026

Uh oh!

Meakk commented Mar 25, 2026

Uh oh!

Meakk commented Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davidwuluetang commented Feb 17, 2026 •

edited

Loading

davidwuluetang Mar 24, 2026 •

edited

Loading