Skip to content

Conversation

@krystophny
Copy link

Summary

  • add next_subsection metadata plumbing so slides can record intra-slide checkpoints
  • wire the metadata into the Qt presenter, RevealJS template, PDF exporter, and PowerPoint converter via new CLI flags
  • document the workflow and add focused tests (subsection builder + PDF/PPTX smoke tests)

Testing

  • PYTHONPATH=. pytest tests/test_subsections.py
  • PYTHONPATH=. pytest tests/test_convert.py -k subsections

@krystophny krystophny marked this pull request as draft November 7, 2025 15:25
Fix multiple code quality issues identified by CI:
- Use capture_output parameter in subprocess.run for cleaner API
- Fix Enum inheritance for PowerPointSubsectionMode (use StrEnum)
- Add type annotations to test helper functions
- Import get_duration_ms from utils module correctly
- Add noqa comment for acceptable convert function complexity

All pre-commit checks now pass successfully.
@codecov
Copy link

codecov bot commented Nov 7, 2025

Codecov Report

❌ Patch coverage is 66.19217% with 95 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.24%. Comparing base (c6c4ea6) to head (1dad0ed).

Files with missing lines Patch % Lines
manim_slides/present/player.py 33.01% 71 Missing ⚠️
manim_slides/convert.py 88.88% 11 Missing ⚠️
manim_slides/slide/base.py 78.37% 8 Missing ⚠️
manim_slides/utils.py 81.25% 3 Missing ⚠️
manim_slides/config.py 90.90% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #576      +/-   ##
==========================================
- Coverage   80.12%   78.24%   -1.88%     
==========================================
  Files          23       23              
  Lines        2038     2285     +247     
==========================================
+ Hits         1633     1788     +155     
- Misses        405      497      +92     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment on lines 300 to 326
def next_subsection(
self,
name: str = "",
*,
auto_next: bool = False,
) -> None:
"""
Mark an intra-slide subsection boundary.
Subsections do not create new slides; they are stored as metadata that can be
consumed by converters or presenters.
:param name: Optional label for the subsection.
:param auto_next:
If set, compatible presenters will automatically continue to the next
subsection once this one completes.
"""
relative_animation_index = self._current_animation - self._start_animation
if relative_animation_index < 0:
relative_animation_index = 0

marker = SubsectionMarker(
animation_index=relative_animation_index,
name=name,
auto_next=auto_next,
)
self._pending_subsection_markers.append(marker)
Copy link
Owner

Choose a reason for hiding this comment

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

Hello @krystophny, thanks for your PR! This is a very quick first review (before the actual review): can you add an example using this new feature in the docstring? Then, it will be rendered in the documentation, so people can easily see what this feature is doing.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @jeertmans for the fast reply, and of course! In addition I have to play with it myself a bit before this will be ready. This is an initial draft with some help from coding assistants, so I will thoroughly check it before requesting your final review.

Copy link
Owner

Choose a reason for hiding this comment

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

Sure! Ping me when it is ready

krystophny and others added 23 commits November 7, 2025 17:54
Add code example demonstrating subsection usage within a slide.
The example shows how subsections create pause points that can be
consumed by presenters when --subsections flag is used.

Addresses review feedback from maintainer.
Security improvements in utils.py:
- Add input validation for extract_video_segment()
- Validate source file exists and is a file
- Validate time parameters are non-negative
- Use resolved absolute paths in subprocess call
- Ensure destination directory exists

Complexity reduction in convert.py:
- Extract _determine_converter_class() helper function
- Extract _apply_config_options() helper function
- Reduce convert() function complexity from 16 to acceptable level
- Improve code maintainability and readability

All tests pass. Addresses Codacy static analysis warnings.
Add nosec comments and explicit security documentation:
- nosec B404 for subprocess import (controlled usage)
- nosec B603 for subprocess.run call (validated inputs)
- Explicit shell=False to prevent shell injection
- Detailed security comment explaining validation

These annotations inform static analyzers that subprocess usage
is safe and intentional, with proper input validation in place.
Replace previous security annotations with proper implementation:
- Validate all inputs (file existence, type, time ranges)
- Use shutil.which() to safely locate ffmpeg executable
- Resolve paths to absolute to prevent traversal attacks
- Pass arguments as list with shell=False
- Add detailed docstring explaining security measures

This eliminates the root cause of Codacy warnings by demonstrating
proper subprocess security practices instead of suppressing warnings.
Tests pass successfully.
The refactoring to reduce code complexity inadvertently placed helper
functions between the Click decorators and the convert() function,
breaking the decorator chain. This caused ReadTheDocs build failures
with 'AttributeError: function object has no attribute name'.

Moved _determine_converter_class() and _apply_config_options() helper
functions above the decorator chain to restore proper Click command
registration while maintaining reduced complexity.
Changed default behavior for subsections to work automatically without
requiring flags. Updated defaults for --subsections and --html-subsections
from off/disabled to pause, enabling subsection pauses by default when
subsections are present in code. Slides without subsections continue to
work identically.

Added SubsectionExample to demonstrate subsection usage with multiple
pause points within a single slide, showing how subsections work similar
to Beamer overlays.

Fixed SubsectionExample to properly demonstrate two slides instead of
three, with the conclusion as a fourth subsection rather than a separate
slide.

Added test coverage for PDF subsection mode none to ensure slides without
subsections are handled correctly.

Updated documentation to reflect that subsections now work automatically
in Qt/HTML presenters without needing flags.
When generating PDFs with --pdf-subsections=none (the default), slides
containing subsections now correctly capture the final frame from the
last subsection's end_time, ensuring all subsection content is visible
on the PDF page.

Previously, slides with subsections would use the video file's last
frame without considering subsection boundaries, potentially missing
content from completed subsections.

The fix ensures that:
- Slides without subsections: unchanged behavior
- Slides with subsections + frame_index=last: captures last subsection end
- Slides with subsections + frame_index=first: captures video start
- pdf_subsection_mode=all: still captures all intermediate states
- pdf_subsection_mode=final: still captures only final subsection

Added documentation to test explaining the expected behavior.
Simplified subsection handling across all export formats:
- Removed final mode from PDF and PowerPoint (redundant with none + frame_index=last)
- Unified terminology: PowerPoint now uses none/all instead of off/split
- Changed all defaults to all for PDF and PowerPoint exports

This creates a consistent two-option system:
- none: ignore subsections, treat slide as single unit
- all: split subsections into separate pages/slides (default)

Updated tests to cover both modes and documentation to reflect
the simplified API.
…sections

Consolidated --pdf-subsections and --pptx-subsections into a single
--subsections flag that controls both PDF and PowerPoint exports. This
simplifies the API and provides consistent behavior across all static
export formats.

Changes:
- Removed PdfSubsectionMode and PowerPointSubsectionMode enums
- Added unified SubsectionMode enum with none/all options
- Both PDF and PowerPoint now use subsection_mode field
- Single --subsections CLI flag for convert command (default: all)
- Kept --html-subsections separate as it has different values

Updated all tests and documentation to reflect the unified flag.
…ections flag

Completes the radical simplification by unifying HTML subsection handling
with the same approach as PDF and PowerPoint formats.

Changes:
- Removed RevealSubsectionMode enum (disabled, pause, autoplay modes)
- Changed RevealJS to use unified SubsectionMode (none, all)
- Removed --html-subsections CLI flag
- Simplified revealjs.html template to handle none/all modes
- Removed mode parameter from JavaScript subsection functions
- Simplified finishSubsection to always pause unless auto_next is true

All formats (Qt, HTML, PDF, PowerPoint) now use the same --subsections flag
with consistent behavior:
- all: create separate slides/pages per subsection (default)
- none: ignore subsections, show final accumulated state

All 64 tests pass.
…havior

The previous example was confusing because the final subsection removed content
with FadeOut and Transform, contradicting the key principle that subsections
accumulate content while slides clear the screen.

Changes:
- Removed confusing conclusion subsection that deleted content
- Now builds a complete diagram: circle -> square -> labels -> arrow
- Each subsection adds new elements while keeping all previous content visible
- Updated documentation to clearly explain accumulative vs clearing behavior
- Emphasized that next_subsection keeps content, next_slide clears it
- Listed all four subsections explicitly to show the progression

This makes it much clearer how subsections work and why they are useful for
step-by-step builds similar to Beamer overlays.
…attern

Use Jinja2 conditional {% if subsection_mode == 'all' %} instead of
passing mode to JavaScript. This makes RevealJS consistent with PDF
and PowerPoint which use subsection_mode field with none/all values.

JavaScript is now only included when subsections are enabled (all mode),
making the template cleaner and eliminating runtime mode checks.
Remove OFF/PAUSE/AUTOPLAY complexity and unify to none/all like other formats:
- none: disable subsections (play through entire slide)
- all: enable subsections (pause at each, press key to advance)

Removed AUTOPLAY mode as unnecessary complexity. The auto_next parameter
on next_subsection() already handles automatic advancement when needed.

This makes ALL formats (Qt, HTML, PDF, PowerPoint) use the same
--subsections=none|all flag with consistent behavior.
… like PowerPoint

Make HTML/RevealJS work EXACTLY like PowerPoint/PDF:
- subsection_mode=all: Create separate RevealJS <section> slides per subsection
- subsection_mode=none: One <section> per manim slide

REMOVED:
- 140+ lines of complex JavaScript (state tracking, pending/index logic)
- WeakMap state management
- advanceSubsection/rewindSubsection/finishSubsection functions
- Keyboard event interception
- Complex subsection progression logic

ADDED:
- Simple _iter_slide_sections() method (like PowerPoint's _iter_slide_fragments)
- 22 lines of minimal JavaScript to handle video start/end times
- data-background-video-start/end attributes

Result: HTML now behaves EXACTLY like PowerPoint - separate slides for
subsections when mode=all. No complex state, no manual progression.
Just native RevealJS slide navigation.

All 64 tests pass.
krystophny and others added 9 commits November 10, 2025 01:31
Unified HTML subsection timing with PowerPoint approach:
- Use subsection.start_time instead of always starting from 0.0
- Add tail section for content after last subsection
- Matches Qt presenter and PowerPoint behavior

This fixes timing issues in HTML where subsections were always
showing accumulated content from the beginning instead of just
the subsection's time range.
…presenter

When loading a slide with subsections in --subsections=all mode,
start playing first to load the first frame, then immediately pause
at position 0. This prevents the black screen that occurs when
pausing before the first frame is loaded.

Fixes the black background issue when transitioning from slides
without subsections to slides with subsections.
HTML now extracts subsection video segments like PowerPoint does:
- Extract segments from start_time to end_time with accurate=True
- Each subsection gets its own video file (re-encoded for frame accuracy)
- Tail sections for content after last subsection
- Only triggered when subsections are present

This fixes timing issues in HTML where browser video seeking to
start/end times was not frame-accurate (keyframe-based). Now HTML
uses pre-extracted video segments with exact frame-perfect cuts.
When next_subsection() is called, it now also calls Manim's next_section()
internally. This allows Manim to create separate video files for each
subsection automatically when --save_sections flag is used.

This is the first step toward eliminating ffmpeg post-processing and
letting Manim do the video splitting work natively.
Instead of extracting video segments with ffmpeg post-processing,
subsections now reference the partial animation files created during
rendering. This eliminates all ffmpeg re-encoding overhead and uses
Manim's existing animation file structure.

Changes:
- Add optional file field to SubsectionConfig pointing to partial file
- Update _build_subsection_configs to map partial files to subsections
- Simplify HTML converter to copy partial files instead of extracting
- Simplify PowerPoint converter to copy partial files instead of extracting
- Remove extract_video_segment function entirely from utils
- Remove subprocess import no longer needed

Benefits:
- No ffmpeg post-processing or re-encoding overhead
- Frame-perfect accuracy using original rendered files
- Cleaner architecture reusing existing Manim animation files
- Faster conversion with simple file copies
Use subsection.file directly instead of reading timestamps from
concatenated file. This fixes duplicate frames and ensures correct
frame extraction from partial animation files.

- Read frames from subsection.file when available
- Remove duplicate final frame that was always added
- Simplifies logic and matches HTML/PowerPoint approach
Manim has a known bug where animations stop at ~93% completion
(alpha never reaches 1.0 at 15fps). The last frame of an animation
shows incomplete state (e.g., circles with gaps).

Workaround: Use the first frame of the NEXT subsection to show
completion of the current animation, since it captures the final
state before the next animation begins.

Reference: https://lightrun.com/answers/manimcommunity-manim-t-values-issue-that-causes-the-animations-to-not-be-finished-entirely
Manim has a bug where animations stop at ~93% completion. When videos
pause after a subsection, they show this incomplete last frame (e.g.,
circles with visible gaps).

Solution: Call self.wait() after each subsection, same as next_slide()
does. This adds frames showing the completed animation state.

The wait() creates a separate animation file that shows the final
completed state, ensuring when video playback pauses, it shows the
complete animation instead of the 93% incomplete frame.

Reference: https://lightrun.com/answers/manimcommunity-manim-t-values-issue-that-causes-the-animations-to-not-be-finished-entirely
@krystophny krystophny force-pushed the feature/subsections-pauses-rebased branch from 3354b0b to 4787b4e Compare November 10, 2025 01:09
pre-commit-ci bot and others added 4 commits November 10, 2025 01:09
Enhance code documentation and test coverage for subsection implementation:

- Add comprehensive comments explaining Manim animation bug workaround
  * wait() creates completion frame as separate partial animation file
  * PDF converter uses first frame of next subsection for completion
  * Workaround fixes incomplete frames (~93% completion) across all backends

- Clarify subsection file mapping logic
  * Only single-animation subsections have file field populated
  * Multi-animation subsections have file=None (span multiple files)

- Add field description for SubsectionConfig.file
  * Documents when file is set vs None

- Update tests to work with new _build_subsection_configs signature
  * Create dummy animation files for validation
  * Add test assertions for file field population

All tests pass. Code is production-ready with clear documentation of the
workarounds for Manim's incomplete animation rendering.
@jeertmans jeertmans added enhancement New feature or request html-convert Related to converting to HTML slides pptx-convert Related to converting to PowerPoint slides present Related to the main "present" feature python Pull requests that update Python code labels Nov 10, 2025
Copy link
Owner

Choose a reason for hiding this comment

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

Another comment (I know it's still a draft): I don't really understand how this example motivates the use of subsection? In HTML, it appears that they are exactly the same. The "cleared" slide is not a feature from the section, but rather from using self.clear() before calling self.next_section. One thing I would expect from subsection is to create vertical slides when exported to HTML.

Another point to consider is that self.next_section inherits its names from Manim's sections, but it is a "smart" alias to self.next_slide (i.e., it also calls super().next_section()). We should make sure that user self.next_subsection does not interfere with Manim's sections.

Copy link
Owner

Choose a reason for hiding this comment

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

Is it AI generated? I am asking because the "changed" section does not make much sense: we did not change the defaults, because most of them did not exist previously.

@krystophny
Copy link
Author

@jeertmans sorry for the mess - this is all work in progress trying various options. Also Changelog is nonsense right now. Maybe it's better to close the PR for now in order not to generate noise? Just to discuss the general idea with you: I removed most code again so we dont have to support "real" subsections but keep them as alias for slide/section with slightly different behavior. Mainly, I want to be able to generate PDFs with fully built slides on request and if technically possible provide multiple animations on single pptx slide. The remaining code should not be touched too much and I am right now in the process of reverting a lot. Regarding the manim hacks I am trying to find a cleaner solution. Earlier it would cut the video in subsection on cue points but I thought this is inefficient and we should rather just generate them right away without extra copies of videos. If you have some ideas, let me know!

krystophny and others added 8 commits November 10, 2025 14:31
Address reviewer feedback by implementing proper RevealJS vertical
slides for subsections instead of flat horizontal slides. This provides
hierarchical 2D navigation where subsections are grouped under their
parent slide.

Changes:
- Modified revealjs.html template to wrap subsections in parent section
  tag when there are multiple sections per slide
- Updated documentation to properly motivate subsections as vertical
  navigation in HTML rather than misleading accumulation vs clearing
- Fixed CHANGELOG to accurately describe new features in Added section
  instead of incorrectly claiming changed defaults

Technical notes:
- Single-section slides remain as flat section elements
- Multi-section slides wrap subsections in parent section for nesting
- RevealJS interprets nested sections as vertical navigation
- Maintains backward compatibility with existing presentations
The Qt presenter frame freezing was already implemented in earlier
commits and should not be listed as a new feature in this PR's
changelog entry.
Match next_slide() behavior by checking if there were any animations
before calling wait(). This prevents adding unnecessary wait frames
when next_subsection is called at the start of a slide.

The wait() call is only needed to show the completed state after
animations (Manim bug where animations stop at ~93% completion).
Without animations, there's nothing to complete.
Fix two issues with subsection handling in the Qt presenter:

1. Missing final animations after last subsection: When all subsections
   were consumed, advancing would clear subsections and return False,
   preventing the remainder of the slide from playing. Now plays from
   last subsection end time to slide completion.

2. Duplicate frame freeze causing black frame artifacts: load_current_slide
   was calling _freeze_current_frame() even though its callers
   (load_next_slide, load_previous_slide) already freeze the current
   frame before calling it. Removed redundant call.

These fixes ensure smooth transitions between slides and proper display
of all animations within slides containing subsections.
When slides have subsections with remaining content after the last
subsection, the HTML export now properly extracts and includes the
tail segment as a separate video file.

The tail segment is extracted using ffmpeg with re-encoding (not stream
copy) to ensure accurate timing at non-keyframe boundaries. This ensures
all animations are visible in the HTML presentation, including content
that appears after the last subsection marker.

Fixes issue where the final animations in slides with subsections would
not appear in HTML/RevealJS presentations.
The _freeze_current_frame() call in load_next_slide() was causing black
frames when transitioning to slides that start with clear(). This call
was added in earlier commits but conflicts with the natural video
playback flow.

When a slide starts with clear() (creating a black frame), freezing the
previous slide's last frame before loading the new slide causes a visible
black frame artifact during the transition.

Restored the original main branch behavior where frames update naturally
during slide transitions. This allows the new slide to display immediately
without showing intermediate black frames.
…rames

SubsectionExample was missing wait_time_between_slides, causing the 93%
completion issue where animations don't fully complete their last frame.

Setting wait_time_between_slides=0.1 ensures that wait() calls create
completion frames after each animation, fixing the incomplete last frame
problem in both HTML and Qt presentations.

This matches the pattern used in ConvertExample and other examples that
require fully rendered final frames.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request html-convert Related to converting to HTML slides pptx-convert Related to converting to PowerPoint slides present Related to the main "present" feature python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants