diff --git a/lib/screens/video_player/video_player_controls.dart b/lib/screens/video_player/video_player_controls.dart index ed8f1ef..592736a 100644 --- a/lib/screens/video_player/video_player_controls.dart +++ b/lib/screens/video_player/video_player_controls.dart @@ -112,6 +112,12 @@ class _DesktopControlsState extends ConsumerState { timer.reset(); } + // Height measurement logic remains here for architectural reasons: + // 1. The video controls widget owns and renders the bottom menu UI elements + // 2. Only this widget has direct access to the menu's RenderBox for accurate measurement + // 3. Subtitle widgets are separate components that shouldn't know about control UI structure + // 4. Different players (LibMPV, MDK) can receive the same measurement without duplicating logic + // 5. Clean separation: controls handle UI measurement, players handle subtitle positioning // Use PostFrameCallback to measure height after layout void _measureMenuHeight() { WidgetsBinding.instance.addPostFrameCallback((_) { diff --git a/lib/util/subtitle_position_calculator.dart b/lib/util/subtitle_position_calculator.dart index f6e054b..5cbecba 100644 --- a/lib/util/subtitle_position_calculator.dart +++ b/lib/util/subtitle_position_calculator.dart @@ -1,28 +1,14 @@ import 'dart:math' as math; import 'package:fladder/models/settings/subtitle_settings_model.dart'; -/// Utility class for calculating subtitle positioning based on menu overlay state -/// Provides utilities for calculating the optimal vertical position of subtitles -/// based on user settings and the visibility or size of the player menu overlay. class SubtitlePositionCalculator { - // Configuration constants - static const double _fallbackMenuHeightPercentage = 0.15; // 15% fallback + static const double _fallbackMenuHeightPercentage = 0.15; static const double _dynamicSubtitlePadding = - -0.03; // -3% padding when we have accurate menu height so the subtitles are closer to the menu + 0.00; // Currently unused (0%). Reserved for future implementation of a user-adjustable slider to control subtitle positioning + // relative to the player menu static const double _fallbackSubtitlePadding = 0.01; // 1% padding for conservative fallback positioning - static const double _maxSubtitleOffset = 0.85; // Max 85% up from bottom + static const double _maxSubtitleOffset = 0.85; - /// Calculate subtitle offset using actual menu height when available - /// - /// Returns the optimal subtitle offset (0.0 to 1.0) where: - /// - 0.0 = bottom of screen - /// - 1.0 = top of screen - /// - /// Parameters: - /// - [settings]: User's subtitle settings containing preferred vertical offset - /// - [showOverlay]: Whether the player menu overlay is currently visible - /// - [screenHeight]: Height of the screen in pixels - /// - [menuHeight]: Optional actual height of the menu in pixels static double calculateOffset({ required SubtitleSettingsModel settings, required bool showOverlay, @@ -37,29 +23,19 @@ class SubtitlePositionCalculator { double subtitlePadding; if (menuHeight != null && screenHeight > 0) { - // Convert menu height from pixels to screen percentage menuHeightPercentage = menuHeight / screenHeight; - // Use negative padding since we have accurate measurement - can position closer subtitlePadding = _dynamicSubtitlePadding; } else { - // Fallback to static percentage when measurement unavailable menuHeightPercentage = _fallbackMenuHeightPercentage; - // Use positive padding for safety since we're estimating subtitlePadding = _fallbackSubtitlePadding; } - // Calculate the minimum safe position (menu height + appropriate padding) final minSafeOffset = menuHeightPercentage + subtitlePadding; - // If subtitles are already positioned above the safe area, leave them alone - // but still apply maximum bounds checking if (settings.verticalOffset >= minSafeOffset) { return math.min(settings.verticalOffset, _maxSubtitleOffset); } - // Position subtitles just above the menu with bounds checking - // Defensive: math.max(0.0, ...) ensures the offset is never negative, - // which could happen if future changes allow negative menuHeight or padding. return math.max(0.0, math.min(minSafeOffset, _maxSubtitleOffset)); } } diff --git a/lib/wrappers/players/lib_mpv.dart b/lib/wrappers/players/lib_mpv.dart index 4ed69c9..4d17d8d 100644 --- a/lib/wrappers/players/lib_mpv.dart +++ b/lib/wrappers/players/lib_mpv.dart @@ -168,13 +168,13 @@ class LibMPV extends BasePlayer { @override Widget? subtitles( bool showOverlay, { - double? menuHeight, + double? menuHeight, // Passed from video_player_controls.dart which owns the menu UI }) => _controller != null ? _VideoSubtitles( controller: _controller!, showOverlay: showOverlay, - menuHeight: menuHeight, + menuHeight: menuHeight, // Forward the measured height for accurate positioning ) : null; @@ -198,12 +198,12 @@ class LibMPV extends BasePlayer { class _VideoSubtitles extends ConsumerStatefulWidget { final VideoController controller; final bool showOverlay; - final double? menuHeight; + final double? menuHeight; // Accurate measurement from controls, null triggers fallback positioning const _VideoSubtitles({ required this.controller, this.showOverlay = false, - this.menuHeight, + this.menuHeight, // Receives pre-measured height rather than measuring internally }); @override @@ -257,7 +257,7 @@ class _VideoSubtitlesState extends ConsumerState<_VideoSubtitles> { return const SizedBox.shrink(); } - // Use the utility for offset calculation + // Use the utility for offset calculation with passed menuHeight final offset = SubtitlePositionCalculator.calculateOffset( settings: settings, showOverlay: widget.showOverlay, diff --git a/test/util/subtitle_position_calculator_test.dart b/test/util/subtitle_position_calculator_test.dart deleted file mode 100644 index 8197d23..0000000 --- a/test/util/subtitle_position_calculator_test.dart +++ /dev/null @@ -1,104 +0,0 @@ -import 'package:flutter_test/flutter_test.dart'; -import 'package:fladder/models/settings/subtitle_settings_model.dart'; -import 'package:fladder/util/subtitle_position_calculator.dart'; - -void main() { - group('SubtitlePositionCalculator', () { - test('returns original offset when overlay is hidden', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.2); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: false, - screenHeight: 800, - menuHeight: 120, - ); - - expect(result, equals(0.2)); - }); - - test('uses dynamic menu height when available', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.1); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: true, - screenHeight: 800, - menuHeight: 120, // 120/800 = 0.15 (15%) - ); - - // Should position at menu height + dynamic padding = (120/800) + (-0.03) = 0.12 - expect(result, closeTo(0.12, 0.001)); - }); - - test('uses fallback when menu height unavailable', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.1); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: true, - screenHeight: 800, - menuHeight: null, - ); - - // Should use fallback: 0.15 + 0.01 = 0.16 - expect(result, equals(0.16)); - }); - - test('preserves user offset when already above menu', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.3); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: true, - screenHeight: 800, - menuHeight: 120, - ); - - // Should keep original 0.3 since it's above menu area (0.12) - expect(result, equals(0.3)); - }); - - test('clamps to maximum offset', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.95); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: true, - screenHeight: 800, - menuHeight: 600, // Large menu that would push subtitles too high - ); - - // Should clamp to max 0.85 - expect(result, equals(0.85)); - }); - - test('handles zero screen height gracefully', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.1); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: true, - screenHeight: 0, - menuHeight: 120, - ); - - // Should use fallback when screen height is invalid - expect(result, equals(0.16)); - }); - - test('clamps to minimum offset', () { - const settings = SubtitleSettingsModel(verticalOffset: 0.05); - - final result = SubtitlePositionCalculator.calculateOffset( - settings: settings, - showOverlay: true, - screenHeight: 800, - menuHeight: 120, - ); - - // Should not go below 0.0 - expect(result, greaterThanOrEqualTo(0.0)); - }); - }); -}