Fix inconsistent header and colors in frame extraction dialog by dylantirandaz · Pull Request #1089 · MrNeRF/LichtFeld-Studio

dylantirandaz · 2026-04-08T15:51:32Z

Summary

Replaced all hardcoded ImVec4 text colors in the video extractor dialog with theme palette references (palette.text_dim, palette.warning, palette.success, palette.error) so the dialog respects the active theme
Wrapped the dialog content in a proper ImGui::Begin/End window with modal styling and a title-bar close button, matching the presentation of other floating panels (Input Settings, Marketplace)
The dialog can now be closed via the standard window close button in addition to the existing Cancel button

Test plan

Open the Frame Extraction dialog from the menu
Verify it now has a proper window title bar with an × close button
Confirm the close button dismisses the dialog
Check that secondary text (file paths, resolution preview, estimated frames) uses the theme's dimmed text color
Verify warning/success/error messages use the correct theme colors
Switch themes and confirm the dialog colors update accordingly
Confirm the Cancel button still works as before

Copilot

Pull request overview

Updates the Video Extractor “Frame Extraction” UI to better match other floating/modal panels and to respect the active theme, addressing issue #1005 (inconsistent styling and inability to close).

Changes:

Replaced hardcoded ImVec4 colors in the extractor dialog with theme palette colors (dim/warning/success/error).
Wrapped the extractor widget in a proper ImGui::Begin/End floating window with modal styling and a standard title-bar close button.
Ensured the panel disables itself when closed via the window close button (or when the widget requests closure).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
`src/visualizer/gui/windows/video_extractor_dialog.cpp`	Swaps hardcoded text/status colors to theme palette references inside the extractor widget rendering.
`src/visualizer/gui/native_panels.cpp`	Hosts the extractor widget in a self-managed ImGui window using modal theme styling and an `open` close button.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T15:55:44Z

        const char* ext = format_selection_ == 0 ? ".png" : ".jpg";
        char preview[128];
        std::snprintf(preview, sizeof(preview), filename_pattern_.data(), 1);
-        ImGui::TextColored(ImVec4(0.6f, 0.6f, 0.6f, 1.0f), LOC(VideoExtractor::EXAMPLE), preview, ext);
+        ImGui::TextColored(t.palette.text_dim, LOC(VideoExtractor::EXAMPLE), preview, ext);


filename_pattern_ is user-editable and is being passed directly as the std::snprintf format string for the preview. If the user enters an invalid/hostile printf pattern (e.g. "%s", "%n", or mismatched specifiers), this is undefined behavior and can crash or worse. Consider generating the preview without treating user input as a format string (e.g. replace a known token like "%d"/"{frame}" manually), or validate/sanitize the pattern so only an allowed placeholder is accepted before calling snprintf.

Copilot · 2026-04-08T15:55:45Z

+        if (ImGui::Begin(LOC(lichtfeld::Strings::VideoExtractor::TITLE), &open)) {
+            if (!widget_->render())
+                open = false;
+        }
+        ImGui::End();
+        t.popModalStyle();


t.pushModalStyle()/t.popModalStyle() are not exception-safe. PanelRegistry::draw_panels() catches exceptions from panel->draw(), so if widget_->render() throws, the style stack will remain unbalanced and can corrupt styling for the rest of the frame. Please ensure popModalStyle() executes on all exit paths (e.g. wrap the Begin/render/End in a try/catch that pops then rethrows, or use a small scope-guard RAII helper).

Suggested change

if (ImGui::Begin(LOC(lichtfeld::Strings::VideoExtractor::TITLE), &open)) {

if (!widget_->render())

open = false;

}

ImGui::End();

t.popModalStyle();

bool began = false;

try {

const bool visible = ImGui::Begin(LOC(lichtfeld::Strings::VideoExtractor::TITLE), &open);

began = true;

if (visible) {

if (!widget_->render())

open = false;

}

ImGui::End();

began = false;

t.popModalStyle();

} catch (...) {

if (began)

ImGui::End();

t.popModalStyle();

throw;

}

shadygm · 2026-04-08T16:45:04Z

Similar to my previous comments.

I was not able to test it since the mesh-2-splat doesn't open, please make sure to sync with master and provide some screenshots of the before and after the UI to make it a bit easier to review :p Thanks!

shadygm

No UI change.

Please make sure that you test your PRs and ask questions for comprehension before setting them for review (:

dylantirandaz · 2026-04-10T03:32:29Z

Okay i figured out the issue for this, going to fix so the checks pass but attached is a screenshot to make sure im correctly understanding what you are looking for

MrNeRF · 2026-04-10T12:19:06Z

basically move away from imgui to rmlui. At the current state this implementation kills the entire preview Window. It should match the old version just in rmlui!

dylantirandaz · 2026-04-10T17:51:43Z

is this good?

MrNeRF · 2026-04-10T21:11:30Z

I think so. I can't test now. I will check it tomorrow.

dylantirandaz · 2026-04-10T23:11:03Z

Sounds good

MrNeRF · 2026-04-11T05:31:19Z

This is how it used to look like:

and this is the current state on this branch:

Currently, it doesn't do anything on loading a video. Of course, the functionality must be restored.

dylantirandaz · 2026-04-11T06:11:44Z

okay sounds good, im still working on it u can ignore the latest push i havent tested yet and its late here so ill finish tmrw

dylantirandaz · 2026-04-11T20:55:49Z

fixed, ran into some issues with rewind/fast forward but i fixed those too

MrNeRF · 2026-04-12T18:17:27Z

Closer but still looks very different. The playback is also 4x slower etc.

dylantirandaz · 2026-04-17T05:22:11Z

Okay fixed the playback, not sure if I am able to recreate the UI one of one though

shadygm · 2026-05-06T21:48:22Z

@dylantirandaz, apologies for my late reply on this PR. Could you please check the builds and resolve the conflicts. Thanks!

Replace all hardcoded ImGui text colors with theme palette references so the dialog respects the active theme (dark, light, gruvbox, etc.): - Secondary/info text now uses palette.text_dim instead of inline grays - Warning, success, and error messages use palette.warning/success/error Wrap the dialog content in a proper ImGui::Begin/End window with modal styling and a title-bar close button, matching how other floating panels (Input Settings, Marketplace) present themselves. Closes MrNeRF#1005

Replace the native ImGui video extractor panel with a Python-managed RmlUI panel. Add a custom VideoPreviewElement for GL texture rendering with aspect-ratio letterboxing via CallbackTextureSource. Wire video player and frame extraction through python_runtime callbacks so the Python module avoids linking lfs_video directly.

- Fix locale keys to use correct translation strings - Add separator line styling on section headers - Make transport controls always visible (not just when video open) - Use 28dp round buttons with triangle icons matching old ImGui - Set panel width to 750dp matching old ImGui window size - Set preview area to 300dp with dark background and border - Add test data files to .gitignore

Video player functions are registered on the lichtfeld.io submodule but the panel was calling them on the root lichtfeld module, causing AttributeError on video load. Use lf.io.* prefix for all video player, feed_preview, and video_extract calls.

VideoPlayer::update() expects wall clock time (seconds since reference point), not delta time. The old ImGui code passed ImGui::GetTime(). Fix the callback to use steady_clock relative to app start. Also fix preview updates after step/seek by adding a _needs_preview_update flag that triggers feed_preview_element even when the player isn't playing.

Drive playback from Python using seek-based frame advancing instead of VideoPlayer::update() which auto-pauses at EOF. Fix CallbackTextureSource recreation causing black frames. Fix step forward/backward using seek with 1/fps offset.

Drive playback from Python using stepForward with frame-rate accumulator instead of broken VideoPlayer::update(). Fix seek- based rewind with retry for keyframe snapping. Fix EOF auto- pause by seeking to current position before play. Fix texture source recreation causing black frames during playback.

MrNeRF · 2026-05-10T07:58:41Z

Hey @dylantirandaz what's the state here given your last commits?

Replace direct OpenGL texture management (glad/glGenTextures/ glTexImage2D) with Rml::CallbackTextureInterface::GenerateTexture so the element works with the project's Vulkan rmlui backend. Frame data is converted from RGB to RGBA in CPU and uploaded through a freshly-constructed CallbackTextureSource whenever new pixels arrive. The previous source is released by move-assign, freeing the cached backend texture.

dylantirandaz · 2026-05-11T02:31:49Z

Im working on fixing some stuff but Im swamped with final exams so not sure on the timeline, just pushing code when I have free time

shadygm · 2026-06-14T10:59:39Z

hey @dylantirandaz, I was wondering if you have any updates regarding this PR (:

…ion-header

dylantirandaz · 2026-06-18T14:11:44Z

Migrated the video extractor from the old native ImGui panel to a Python-managed RmlUI panel: LichtFeld-Studio/src/python/lfs_plugins/
video_extractor_panel.py:74
Added the RmlUI layout and styling for the panel: LichtFeld-Studio/src/visualizer/gui/rmlui/resources/video_extractor_panel.rml:1, LichtFeld-Studio/src/
visualizer/gui/rmlui/resources/video_extractor_panel.rcss:1
Added a custom video-preview RmlUI element so the panel can display video frames inside RmlUI: LichtFeld-Studio/src/visualizer/gui/rmlui/elements/
video_preview_element.hpp:19
Registered that custom RmlUI element with the UI system: LichtFeld-Studio/src/visualizer/gui/rmlui/rmlui_manager.cpp:150
Wired the File menu to open lfs.video_extractor instead of the native ImGui panel: LichtFeld-Studio/src/python/lfs_plugins/file_menu.py:119
Exposed video player and extraction functions to Python/RmlUI through lichtfeld.io: LichtFeld-Studio/src/python/lfs/py_io.cpp:601
Hooked the Python callbacks back into the C++ video player/extractor implementation: LichtFeld-Studio/src/visualizer/visualizer_impl.cpp:321
Fixed unsafe filename pattern handling so user input is no longer passed directly to snprintf: LichtFeld-Studio/src/io/video_frame_extractor.cpp:90
Kept the old ImGui fallback safer by using the same filename helper and exception-safe modal style cleanup.
Added regression tests for the filename pattern behavior and RmlUI preview example: LichtFeld-Studio/tests/python/test_video_extractor_panel.py:1

Copilot AI review requested due to automatic review settings April 8, 2026 15:51

Copilot started reviewing on behalf of dylantirandaz April 8, 2026 15:52 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

dylantirandaz force-pushed the fix/frame-extraction-header branch from f30ab46 to dca534d Compare April 8, 2026 19:32

shadygm self-requested a review April 9, 2026 08:48

shadygm assigned dylantirandaz Apr 9, 2026

shadygm requested changes Apr 9, 2026

View reviewed changes

dylantirandaz force-pushed the fix/frame-extraction-header branch from dca534d to 6284324 Compare April 9, 2026 20:12

MrNeRF self-requested a review April 10, 2026 12:19

dylantirandaz force-pushed the fix/frame-extraction-header branch from 23cee8e to 6284324 Compare April 10, 2026 16:16

dylantirandaz force-pushed the fix/frame-extraction-header branch from 3fdd397 to 7e11da0 Compare April 10, 2026 17:59

dylantirandaz added 7 commits May 9, 2026 16:55

Polish retained video extractor playback and UI

078fa3f

dylantirandaz force-pushed the fix/frame-extraction-header branch from 6ec922d to 078fa3f Compare May 9, 2026 21:59

MrNeRF mentioned this pull request Jun 8, 2026

No reacton from the Extract Frames from Video buttom. #1279

Open

dylantirandaz added 3 commits June 18, 2026 08:45

Merge remote-tracking branch 'upstream/master' into fix/frame-extract…

8126697

…ion-header

Address video extractor review issues

5648f8c

Add video extractor panel regression tests

1bf0c21

Conversation

dylantirandaz commented Apr 8, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

shadygm commented Apr 8, 2026

Uh oh!

shadygm left a comment

Choose a reason for hiding this comment

Uh oh!

dylantirandaz commented Apr 10, 2026

Uh oh!

MrNeRF commented Apr 10, 2026

Uh oh!

dylantirandaz commented Apr 10, 2026

Uh oh!

MrNeRF commented Apr 10, 2026

Uh oh!

dylantirandaz commented Apr 10, 2026

Uh oh!

MrNeRF commented Apr 11, 2026

Uh oh!

dylantirandaz commented Apr 11, 2026

Uh oh!

dylantirandaz commented Apr 11, 2026

Uh oh!

MrNeRF commented Apr 12, 2026

Uh oh!

dylantirandaz commented Apr 17, 2026

Uh oh!

shadygm commented May 6, 2026

Uh oh!

MrNeRF commented May 10, 2026

Uh oh!

dylantirandaz commented May 11, 2026

Uh oh!

shadygm commented Jun 14, 2026

Uh oh!

dylantirandaz commented Jun 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants