mirror of
https://github.com/gabehf/Fladder.git
synced 2026-03-14 17:55:58 -07:00
refactor: Improve subtitle positioning logic and remove outdated tests
This commit is contained in:
parent
2884a5fadc
commit
c446210e6a
4 changed files with 15 additions and 137 deletions
|
|
@ -112,6 +112,12 @@ class _DesktopControlsState extends ConsumerState<DesktopControls> {
|
||||||
timer.reset();
|
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
|
// Use PostFrameCallback to measure height after layout
|
||||||
void _measureMenuHeight() {
|
void _measureMenuHeight() {
|
||||||
WidgetsBinding.instance.addPostFrameCallback((_) {
|
WidgetsBinding.instance.addPostFrameCallback((_) {
|
||||||
|
|
|
||||||
|
|
@ -1,28 +1,14 @@
|
||||||
import 'dart:math' as math;
|
import 'dart:math' as math;
|
||||||
import 'package:fladder/models/settings/subtitle_settings_model.dart';
|
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 {
|
class SubtitlePositionCalculator {
|
||||||
// Configuration constants
|
static const double _fallbackMenuHeightPercentage = 0.15;
|
||||||
static const double _fallbackMenuHeightPercentage = 0.15; // 15% fallback
|
|
||||||
static const double _dynamicSubtitlePadding =
|
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 _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({
|
static double calculateOffset({
|
||||||
required SubtitleSettingsModel settings,
|
required SubtitleSettingsModel settings,
|
||||||
required bool showOverlay,
|
required bool showOverlay,
|
||||||
|
|
@ -37,29 +23,19 @@ class SubtitlePositionCalculator {
|
||||||
double subtitlePadding;
|
double subtitlePadding;
|
||||||
|
|
||||||
if (menuHeight != null && screenHeight > 0) {
|
if (menuHeight != null && screenHeight > 0) {
|
||||||
// Convert menu height from pixels to screen percentage
|
|
||||||
menuHeightPercentage = menuHeight / screenHeight;
|
menuHeightPercentage = menuHeight / screenHeight;
|
||||||
// Use negative padding since we have accurate measurement - can position closer
|
|
||||||
subtitlePadding = _dynamicSubtitlePadding;
|
subtitlePadding = _dynamicSubtitlePadding;
|
||||||
} else {
|
} else {
|
||||||
// Fallback to static percentage when measurement unavailable
|
|
||||||
menuHeightPercentage = _fallbackMenuHeightPercentage;
|
menuHeightPercentage = _fallbackMenuHeightPercentage;
|
||||||
// Use positive padding for safety since we're estimating
|
|
||||||
subtitlePadding = _fallbackSubtitlePadding;
|
subtitlePadding = _fallbackSubtitlePadding;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Calculate the minimum safe position (menu height + appropriate padding)
|
|
||||||
final minSafeOffset = menuHeightPercentage + subtitlePadding;
|
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) {
|
if (settings.verticalOffset >= minSafeOffset) {
|
||||||
return math.min(settings.verticalOffset, _maxSubtitleOffset);
|
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));
|
return math.max(0.0, math.min(minSafeOffset, _maxSubtitleOffset));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -168,13 +168,13 @@ class LibMPV extends BasePlayer {
|
||||||
@override
|
@override
|
||||||
Widget? subtitles(
|
Widget? subtitles(
|
||||||
bool showOverlay, {
|
bool showOverlay, {
|
||||||
double? menuHeight,
|
double? menuHeight, // Passed from video_player_controls.dart which owns the menu UI
|
||||||
}) =>
|
}) =>
|
||||||
_controller != null
|
_controller != null
|
||||||
? _VideoSubtitles(
|
? _VideoSubtitles(
|
||||||
controller: _controller!,
|
controller: _controller!,
|
||||||
showOverlay: showOverlay,
|
showOverlay: showOverlay,
|
||||||
menuHeight: menuHeight,
|
menuHeight: menuHeight, // Forward the measured height for accurate positioning
|
||||||
)
|
)
|
||||||
: null;
|
: null;
|
||||||
|
|
||||||
|
|
@ -198,12 +198,12 @@ class LibMPV extends BasePlayer {
|
||||||
class _VideoSubtitles extends ConsumerStatefulWidget {
|
class _VideoSubtitles extends ConsumerStatefulWidget {
|
||||||
final VideoController controller;
|
final VideoController controller;
|
||||||
final bool showOverlay;
|
final bool showOverlay;
|
||||||
final double? menuHeight;
|
final double? menuHeight; // Accurate measurement from controls, null triggers fallback positioning
|
||||||
|
|
||||||
const _VideoSubtitles({
|
const _VideoSubtitles({
|
||||||
required this.controller,
|
required this.controller,
|
||||||
this.showOverlay = false,
|
this.showOverlay = false,
|
||||||
this.menuHeight,
|
this.menuHeight, // Receives pre-measured height rather than measuring internally
|
||||||
});
|
});
|
||||||
|
|
||||||
@override
|
@override
|
||||||
|
|
@ -257,7 +257,7 @@ class _VideoSubtitlesState extends ConsumerState<_VideoSubtitles> {
|
||||||
return const SizedBox.shrink();
|
return const SizedBox.shrink();
|
||||||
}
|
}
|
||||||
|
|
||||||
// Use the utility for offset calculation
|
// Use the utility for offset calculation with passed menuHeight
|
||||||
final offset = SubtitlePositionCalculator.calculateOffset(
|
final offset = SubtitlePositionCalculator.calculateOffset(
|
||||||
settings: settings,
|
settings: settings,
|
||||||
showOverlay: widget.showOverlay,
|
showOverlay: widget.showOverlay,
|
||||||
|
|
|
||||||
|
|
@ -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));
|
|
||||||
});
|
|
||||||
});
|
|
||||||
}
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue