Skip to content

#6 live preview#24

Merged
w-m merged 23 commits intomainfrom
#6-live-preview
Jul 3, 2025
Merged

#6 live preview#24
w-m merged 23 commits intomainfrom
#6-live-preview

Conversation

@fbranschke
Copy link
Copy Markdown
Collaborator

implements a live preview for the conversion

fixes some mistakes with the caching for read_file and write_file. The bug did not appear in previous versions, because we never wrote the same files and never read changed files.

@fbranschke fbranschke requested review from Copilot and w-m June 4, 2025 17:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a live-preview feature to the conversion UI and fixes caching for file read/write transforms.

  • Adds a “Live preview” checkbox to the Convert tab and hooks it into the conversion flow.
  • Tracks a file’s last-modified time in from_json to invalidate stale cache.
  • Refactors the conversion logic in InteractiveConversionTool to queue and handle preview vs. final conversions.

Reviewed Changes

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

File Description
src/ffsplat/render/viewer.py Updated add_convert signature to include live preview and stored scene folder for removal.
src/ffsplat/models/operations.py Recorded last_modified timestamp for read_file transforms.
src/ffsplat/coding/scene_encoder.py Added explicit else branch for write_file transforms in _encode_fields.
src/ffsplat/cli/live.py Removed top‐level helpers, introduced threading, and revamped conversion flow and live‐preview callbacks.
Comments suppressed due to low confidence (1)

src/ffsplat/render/viewer.py:236

  • [nitpick] For UI consistency, consider capitalizing the label: use "Live preview" instead of "live preview".
self._live_preview_checkbox = self.server.gui.add_checkbox("live preview", False)

@w-m
Copy link
Copy Markdown
Owner

w-m commented Jun 5, 2025

I find it quite hard to reason about the conversion_wrapper code, with the many states there are, and the different if statement.

Could you try disentangling the UI code from the processing queue? Maybe we should have a proper processing queue, which holds the state (and can be queried about it to display UI progress), we can enqueue new coding operations, and we can be singled when some finish. Then inside the this processing queue it should become easier to reason about the different states it can be in.

Otherwise I fear this is going to break in subtle but annoying ways (now that there's threading, too).

@fbranschke
Copy link
Copy Markdown
Collaborator Author

You are right @w-m, I will take some time to restructure the live.py.

I could also move most of the changes made to the nerfview viewer to live.py. I think it makes sense to keep the changes made to this dependency as small as possible.

@fbranschke fbranschke marked this pull request as draft June 5, 2025 16:41
@fbranschke fbranschke marked this pull request as ready for review July 2, 2025 14:02
@w-m w-m merged commit ca9f5cc into main Jul 3, 2025
3 checks passed
@w-m
Copy link
Copy Markdown
Owner

w-m commented Jul 3, 2025

👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants