From b4e4043cf0947dc65d731f1b1b69bab2fa21c32a Mon Sep 17 00:00:00 2001 From: "Kenzie (Schmoll) Davisson" <43759233+kenzieschmoll@users.noreply.github.com> Date: Thu, 9 Dec 2021 08:26:41 -0800 Subject: [PATCH] Add frame numbers to the flutter frames chart in the performance page (#3526) --- .../devtools_app/lib/src/common_widgets.dart | 34 ++++++ .../src/performance/flutter_frames_chart.dart | 109 ++++++++++++------ .../performance/performance_controller.dart | 7 +- packages/devtools_app/lib/src/theme.dart | 5 + 4 files changed, 119 insertions(+), 36 deletions(-) diff --git a/packages/devtools_app/lib/src/common_widgets.dart b/packages/devtools_app/lib/src/common_widgets.dart index 60764d0ae61..83e8e715542 100644 --- a/packages/devtools_app/lib/src/common_widgets.dart +++ b/packages/devtools_app/lib/src/common_widgets.dart @@ -12,6 +12,7 @@ import 'package:flutter/material.dart'; import 'analytics/analytics.dart' as ga; import 'config_specific/launch_url/launch_url.dart'; +import 'flutter_widgets/linked_scroll_controller.dart'; import 'globals.dart'; import 'scaffold.dart'; import 'theme.dart'; @@ -1196,6 +1197,39 @@ extension ScrollControllerAutoScroll on ScrollController { } } +/// An extension on [LinkedScrollControllerGroup] to facilitate having the +/// scrolling widgets auto scroll to the bottom on new content. +/// +/// This extension serves the same function as the [ScrollControllerAutoScroll] +/// extension above, but we need to implement these methods again as an +/// extension on [LinkedScrollControllerGroup] because individual +/// [ScrollController]s are intentionally inaccessible from +/// [LinkedScrollControllerGroup]. +extension LinkedScrollControllerGroupExtension on LinkedScrollControllerGroup { + bool get atScrollBottom { + final pos = position; + return pos.pixels == pos.maxScrollExtent; + } + + /// Scroll the content to the bottom using the app's default animation + /// duration and curve.. + void autoScrollToBottom() async { + await animateTo( + position.maxScrollExtent, + duration: rapidDuration, + curve: defaultCurve, + ); + + // Scroll again if we've received new content in the interim. + if (hasAttachedControllers) { + final pos = position; + if (pos.pixels != pos.maxScrollExtent) { + jumpTo(pos.maxScrollExtent); + } + } + } +} + /// Utility extension methods to the [Color] class. extension ColorExtension on Color { /// Return a slightly darker color than the current color. diff --git a/packages/devtools_app/lib/src/performance/flutter_frames_chart.dart b/packages/devtools_app/lib/src/performance/flutter_frames_chart.dart index 47982b9cb0a..7d138ba5e2d 100644 --- a/packages/devtools_app/lib/src/performance/flutter_frames_chart.dart +++ b/packages/devtools_app/lib/src/performance/flutter_frames_chart.dart @@ -12,6 +12,7 @@ import '../analytics/constants.dart' as analytics_constants; import '../auto_dispose_mixin.dart'; import '../banner_messages.dart'; import '../common_widgets.dart'; +import '../flutter_widgets/linked_scroll_controller.dart'; import '../globals.dart'; import '../scaffold.dart'; import '../theme.dart'; @@ -51,15 +52,20 @@ class _FlutterFramesChartState extends State static const outlineBorderWidth = 1.0; + double get frameNumberSectionHeight => scaleByFontFactor(20.0); + + double get frameChartScrollbarOffset => + defaultScrollBarOffset + frameNumberSectionHeight; + PerformanceController _controller; - ScrollController scrollController; + LinkedScrollControllerGroup linkedScrollControllerGroup; - FlutterFrame _selectedFrame; + ScrollController framesScrollController; - double horizontalScrollOffset = 0.0; + ScrollController frameNumbersScrollController; - double get availableChartHeight => defaultChartHeight - defaultSpacing; + FlutterFrame _selectedFrame; /// Milliseconds per pixel value for the y-axis. /// @@ -67,15 +73,14 @@ class _FlutterFramesChartState extends State /// target frame time for a single frame (e.g. 16.6 * 2 for a 60 FPS device). double get msPerPx => // Multiply by two to reach two times the target frame time. - 1 / widget.displayRefreshRate * 1000 * 2 / availableChartHeight; + 1 / widget.displayRefreshRate * 1000 * 2 / defaultChartHeight; @override void initState() { super.initState(); - scrollController = ScrollController() - ..addListener(() { - horizontalScrollOffset = scrollController.offset; - }); + linkedScrollControllerGroup = LinkedScrollControllerGroup(); + framesScrollController = linkedScrollControllerGroup.addAndGet(); + frameNumbersScrollController = linkedScrollControllerGroup.addAndGet(); } @override @@ -99,8 +104,9 @@ class _FlutterFramesChartState extends State @override void didUpdateWidget(FlutterFramesChart oldWidget) { super.didUpdateWidget(oldWidget); - if (scrollController.hasClients && scrollController.atScrollBottom) { - scrollController.autoScrollToBottom(); + if (linkedScrollControllerGroup.hasAttachedControllers && + linkedScrollControllerGroup.atScrollBottom) { + linkedScrollControllerGroup.autoScrollToBottom(); } if (!collectionEquals(oldWidget.frames, widget.frames)) { @@ -131,25 +137,26 @@ class _FlutterFramesChartState extends State @override void dispose() { - scrollController.dispose(); + framesScrollController.dispose(); + frameNumbersScrollController.dispose(); super.dispose(); } @override Widget build(BuildContext context) { return Container( - padding: const EdgeInsets.only( + margin: const EdgeInsets.only( left: denseSpacing, right: denseSpacing, - bottom: defaultSpacing, + bottom: denseSpacing, ), - height: defaultChartHeight + defaultScrollBarOffset, + height: defaultChartHeight + frameChartScrollbarOffset, child: Row( children: [ Expanded(child: _buildChart()), const SizedBox(width: defaultSpacing), Padding( - padding: const EdgeInsets.only(bottom: defaultScrollBarOffset), + padding: EdgeInsets.only(bottom: frameChartScrollbarOffset), child: Column( mainAxisAlignment: MainAxisAlignment.spaceBetween, crossAxisAlignment: CrossAxisAlignment.start, @@ -185,12 +192,12 @@ class _FlutterFramesChartState extends State final themeData = Theme.of(context); final chart = Scrollbar( isAlwaysShown: true, - controller: scrollController, + controller: framesScrollController, child: Padding( - padding: const EdgeInsets.only(bottom: defaultScrollBarOffset), + padding: EdgeInsets.only(bottom: frameChartScrollbarOffset), child: RoundedOutlinedBorder( child: ListView.builder( - controller: scrollController, + controller: framesScrollController, scrollDirection: Axis.horizontal, itemCount: widget.frames.length, itemExtent: defaultFrameWidthWithPadding, @@ -200,13 +207,37 @@ class _FlutterFramesChartState extends State selected: widget.frames[index] == _selectedFrame, msPerPx: msPerPx, availableChartHeight: - availableChartHeight - 2 * outlineBorderWidth, + defaultChartHeight - 2 * outlineBorderWidth, displayRefreshRate: widget.displayRefreshRate, ), ), ), ), ); + final frameNumbers = Container( + height: frameNumberSectionHeight, + padding: EdgeInsets.only(left: yAxisUnitsSpace), + child: ListView.builder( + controller: frameNumbersScrollController, + scrollDirection: Axis.horizontal, + itemCount: widget.frames.length, + itemExtent: defaultFrameWidthWithPadding, + shrinkWrap: true, + itemBuilder: (context, index) { + if (index % 2 == 1) { + return const SizedBox(width: defaultFrameWidthWithPadding); + } + return Center( + child: Text( + // TODO(https://github.com/flutter/flutter/issues/94896): stop + // dividing by 2 to get the proper id. + '${widget.frames[index].id ~/ 2}', + style: themeData.subtleChartTextStyle, + ), + ); + }, + ), + ); final chartAxisPainter = CustomPaint( painter: ChartAxisPainter( constraints: constraints, @@ -214,6 +245,7 @@ class _FlutterFramesChartState extends State displayRefreshRate: widget.displayRefreshRate, msPerPx: msPerPx, themeData: themeData, + bottomMargin: frameChartScrollbarOffset, ), ); final fpsLinePainter = CustomPaint( @@ -223,11 +255,16 @@ class _FlutterFramesChartState extends State displayRefreshRate: widget.displayRefreshRate, msPerPx: msPerPx, themeData: themeData, + bottomMargin: frameChartScrollbarOffset, ), ); return Stack( children: [ chartAxisPainter, + Positioned( + top: defaultChartHeight, + child: frameNumbers, + ), Padding( padding: EdgeInsets.only(left: yAxisUnitsSpace), child: chart, @@ -575,6 +612,7 @@ class ChartAxisPainter extends CustomPainter { @required this.displayRefreshRate, @required this.msPerPx, @required this.themeData, + @required this.bottomMargin, }); static const yAxisTickWidth = 8.0; @@ -589,7 +627,7 @@ class ChartAxisPainter extends CustomPainter { final ThemeData themeData; - ColorScheme get colorScheme => themeData.colorScheme; + final double bottomMargin; @override void paint(Canvas canvas, Size size) { @@ -598,7 +636,7 @@ class ChartAxisPainter extends CustomPainter { yAxisUnitsSpace, 0.0, constraints.maxWidth - yAxisUnitsSpace, - constraints.maxHeight - defaultScrollBarOffset, + constraints.maxHeight - bottomMargin, ); _paintYAxisLabels(canvas, chartArea); @@ -608,7 +646,7 @@ class ChartAxisPainter extends CustomPainter { Canvas canvas, Rect chartArea, ) { - const yAxisLabelCount = 6; + const yAxisLabelCount = 5; final totalMs = msPerPx * constraints.maxHeight; // Subtract 1 because one of the labels will be 0.0 ms. @@ -651,20 +689,23 @@ class ChartAxisPainter extends CustomPainter { // Paint a tick on the axis. final tickY = chartArea.height - timeMs / msPerPx; + + // Do not draw the y axis label if it will collide with the 0.0 label or if + // it will go beyond the uper bound of the chart. + if (timeMs != 0 && (tickY > chartArea.height - 10.0 || tickY < 10.0)) + return; + canvas.drawLine( Offset(chartArea.left - yAxisTickWidth / 2, tickY), Offset(chartArea.left + yAxisTickWidth / 2, tickY), - Paint()..color = colorScheme.chartAccentColor, + Paint()..color = themeData.colorScheme.chartAccentColor, ); // Paint the axis label. final textPainter = TextPainter( text: TextSpan( text: labelText, - style: TextStyle( - color: colorScheme.chartSubtleColor, - fontSize: chartFontSizeSmall, - ), + style: themeData.subtleChartTextStyle, ), textAlign: TextAlign.end, textDirection: TextDirection.ltr, @@ -700,6 +741,7 @@ class FPSLinePainter extends CustomPainter { @required this.displayRefreshRate, @required this.msPerPx, @required this.themeData, + @required this.bottomMargin, }); double get fpsTextSpace => scaleByFontFactor(45.0); @@ -714,7 +756,7 @@ class FPSLinePainter extends CustomPainter { final ThemeData themeData; - ColorScheme get colorScheme => themeData.colorScheme; + final double bottomMargin; @override void paint(Canvas canvas, Size size) { @@ -723,7 +765,7 @@ class FPSLinePainter extends CustomPainter { yAxisUnitsSpace, 0.0, constraints.maxWidth - yAxisUnitsSpace, - constraints.maxHeight - defaultScrollBarOffset, + constraints.maxHeight - bottomMargin, ); // Max FPS non-jank value in ms. E.g., 16.6 for 60 FPS, 8.3 for 120 FPS. @@ -733,16 +775,13 @@ class FPSLinePainter extends CustomPainter { canvas.drawLine( Offset(chartArea.left, targetLineY), Offset(chartArea.right, targetLineY), - Paint()..color = colorScheme.chartAccentColor, + Paint()..color = themeData.colorScheme.chartAccentColor, ); final textPainter = TextPainter( text: TextSpan( text: '${displayRefreshRate.toStringAsFixed(0)} FPS', - style: TextStyle( - color: colorScheme.chartSubtleColor, - fontSize: chartFontSizeSmall, - ), + style: themeData.subtleChartTextStyle, ), textAlign: TextAlign.right, textDirection: TextDirection.ltr, diff --git a/packages/devtools_app/lib/src/performance/performance_controller.dart b/packages/devtools_app/lib/src/performance/performance_controller.dart index 234a5d6e3df..d51783176fc 100644 --- a/packages/devtools_app/lib/src/performance/performance_controller.dart +++ b/packages/devtools_app/lib/src/performance/performance_controller.dart @@ -103,7 +103,12 @@ class PerformanceController extends DisposableController if (existingTabForFrame != null) { _selectedAnalysisTab.value = existingTabForFrame; } else { - final newTab = FlutterFrameAnalysisTabData('Frame ${frame.id}', frame); + // TODO(https://github.com/flutter/flutter/issues/94896): stop dividing by + // 2 to get the proper id. + final newTab = FlutterFrameAnalysisTabData( + 'Frame ${frame.id ~/ 2}', + frame, + ); _analysisTabs.add(newTab); _selectedAnalysisTab.value = newTab; } diff --git a/packages/devtools_app/lib/src/theme.dart b/packages/devtools_app/lib/src/theme.dart index 13082faeb57..f9eb05757bf 100644 --- a/packages/devtools_app/lib/src/theme.dart +++ b/packages/devtools_app/lib/src/theme.dart @@ -449,6 +449,11 @@ extension ThemeDataExtension on ThemeData { decoration: TextDecoration.underline, fontSize: defaultFontSize, ); + + TextStyle get subtleChartTextStyle => TextStyle( + color: colorScheme.chartSubtleColor, + fontSize: chartFontSizeSmall, + ); } const extraWideSearchTextWidth = 600.0;