From 6560f7ad3c6050cafbd86689a3c3fbd456bcc437 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Thu, 2 Mar 2023 16:33:40 -0800 Subject: [PATCH 1/2] Add tooltips to bottom up row elements --- .../screens/debugger/program_explorer.dart | 2 +- .../screens/profiler/cpu_profile_columns.dart | 84 +++++-------------- .../src/screens/profiler/panes/bottom_up.dart | 55 ++++++++++-- .../src/screens/profiler/panes/call_tree.dart | 10 ++- .../lib/src/shared/table/table.dart | 6 +- .../lib/src/shared/table/table_data.dart | 19 ++++- .../devtools_app/lib/src/shared/theme.dart | 2 +- 7 files changed, 96 insertions(+), 82 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/debugger/program_explorer.dart b/packages/devtools_app/lib/src/screens/debugger/program_explorer.dart index 60061c5c6ee..0071498ed8f 100644 --- a/packages/devtools_app/lib/src/screens/debugger/program_explorer.dart +++ b/packages/devtools_app/lib/src/screens/debugger/program_explorer.dart @@ -50,7 +50,7 @@ class _ProgramExplorerRow extends StatelessWidget { return DevToolsTooltip( message: toolTip ?? node.name, - textStyle: theme.toolTipFixedFontStyle, + textStyle: theme.tooltipFixedFontStyle, child: InkWell( onTap: onTap, child: Row( diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart index d491808ff52..12cf9006006 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart @@ -5,84 +5,40 @@ import 'package:flutter/material.dart'; import '../../shared/globals.dart'; -import '../../shared/primitives/utils.dart'; import '../../shared/routing.dart'; import '../../shared/table/table.dart'; import '../../shared/table/table_data.dart'; -import '../../shared/utils.dart'; import '../debugger/codeview_controller.dart'; import '../debugger/debugger_screen.dart'; import '../vm_developer/vm_developer_common_widgets.dart'; import 'cpu_profile_model.dart'; -const timeColumnWidthPx = 180.0; - -// TODO(kenz): clean up to use TimeAndPercentageColumn. -class SelfTimeColumn extends ColumnData { - SelfTimeColumn({String? titleTooltip}) - : super( - 'Self Time', +class SelfTimeColumn extends TimeAndPercentageColumn { + SelfTimeColumn({ + String? titleTooltip, + InlineSpan? Function(CpuStackFrame, BuildContext)? dataTooltipProvider, + }) : super( + title: 'Self Time', titleTooltip: titleTooltip, - fixedWidthPx: scaleByFontFactor(timeColumnWidthPx), + timeProvider: (stackFrame) => stackFrame.selfTime, + percentAsDoubleProvider: (stackFrame) => stackFrame.selfTimeRatio, + richTooltipProvider: dataTooltipProvider, + secondaryCompare: (stackFrame) => stackFrame.name, ); - - @override - bool get numeric => true; - - @override - int compare(CpuStackFrame a, CpuStackFrame b) { - final int result = super.compare(a, b); - if (result == 0) { - return a.name.compareTo(b.name); - } - return result; - } - - @override - int getValue(CpuStackFrame dataObject) => dataObject.selfTime.inMicroseconds; - - @override - String getDisplayValue(CpuStackFrame dataObject) { - return '${msText(dataObject.selfTime, fractionDigits: 2)} ' - '(${percent2(dataObject.selfTimeRatio)})'; - } - - @override - String getTooltip(CpuStackFrame dataObject) => ''; } -// TODO(kenz): clean up to use TimeAndPercentageColumn. -class TotalTimeColumn extends ColumnData { - TotalTimeColumn({String? titleTooltip}) - : super( - 'Total Time', +class TotalTimeColumn extends TimeAndPercentageColumn { + TotalTimeColumn({ + String? titleTooltip, + InlineSpan? Function(CpuStackFrame, BuildContext)? dataTooltipProvider, + }) : super( + title: 'Total Time', titleTooltip: titleTooltip, - fixedWidthPx: scaleByFontFactor(timeColumnWidthPx), + timeProvider: (stackFrame) => stackFrame.totalTime, + percentAsDoubleProvider: (stackFrame) => stackFrame.totalTimeRatio, + richTooltipProvider: dataTooltipProvider, + secondaryCompare: (stackFrame) => stackFrame.name, ); - - @override - bool get numeric => true; - - @override - int compare(CpuStackFrame a, CpuStackFrame b) { - final int result = super.compare(a, b); - if (result == 0) { - return a.name.compareTo(b.name); - } - return result; - } - - @override - int getValue(CpuStackFrame dataObject) => dataObject.totalTime.inMicroseconds; - - @override - String getDisplayValue(CpuStackFrame dataObject) { - return '${msText(dataObject.totalTime, fractionDigits: 2)} ' - '(${percent2(dataObject.totalTimeRatio)})'; - } - - @override - String getTooltip(CpuStackFrame dataObject) => ''; } class MethodAndSourceColumn extends TreeColumnData diff --git a/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart b/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart index ce8415e28c1..ee17f14b2ae 100644 --- a/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart +++ b/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart @@ -7,6 +7,7 @@ import 'package:flutter/material.dart'; import '../../../shared/primitives/utils.dart'; import '../../../shared/table/table.dart'; import '../../../shared/table/table_data.dart'; +import '../../../shared/theme.dart'; import '../cpu_profile_columns.dart'; import '../cpu_profile_model.dart'; @@ -18,17 +19,27 @@ class CpuBottomUpTable extends StatelessWidget { required bool displayTreeGuidelines, }) { final treeColumn = MethodAndSourceColumn(); - final startingSortColumn = SelfTimeColumn(titleTooltip: selfTimeTooltip); + final selfTimeColumn = SelfTimeColumn( + titleTooltip: selfTimeTooltip, + dataTooltipProvider: (stackFrame, context) => + _bottomUpTimeTooltipBuilder('Self', stackFrame, context), + ); + final totalTimeColumn = TotalTimeColumn( + titleTooltip: totalTimeTooltip, + dataTooltipProvider: (stackFrame, context) => + _bottomUpTimeTooltipBuilder('Total', stackFrame, context), + ); final columns = List>.unmodifiable([ - TotalTimeColumn(titleTooltip: totalTimeTooltip), - startingSortColumn, + totalTimeColumn, + selfTimeColumn, treeColumn, ]); + return CpuBottomUpTable._( key, bottomUpRoots, treeColumn, - startingSortColumn, + selfTimeColumn, columns, displayTreeGuidelines, ); @@ -55,10 +66,38 @@ class CpuBottomUpTable extends StatelessWidget { 'bottom up tree).'; static const selfTimeTooltip = - 'For top-level methods in the bottom-up tree (leaf stack frames in the ' - 'CPU profile),\nthis is the time the method spent executing only its own ' - 'code. For sub nodes (the\ncallers in the CPU profile), this is the self ' - 'time of the callee when being called by\nthe caller. '; + 'For top-level methods in the bottom-up tree (stack frames that were at ' + 'the top of at least one CPU sample), this is the time the method spent ' + 'executing only its own code.\mFor children methods in the bottom-up ' + 'tree (the callers), this is the self time of the top-level method (the ' + 'callee) when called through the child method (the caller).'; + + static InlineSpan? _bottomUpTimeTooltipBuilder( + String type, + CpuStackFrame stackFrame, + BuildContext context, + ) { + // TODO(kenz): consider adding a tooltip for root nodes as well if this is + // a point of confusion for the user. + if (stackFrame.isRoot) { + return null; + } + final fixedStyle = Theme.of(context).tooltipFixedFontStyle; + return TextSpan( + children: [ + TextSpan(text: '$type time for '), + TextSpan( + text: '${stackFrame.root.name}\n', + style: fixedStyle, + ), + const TextSpan(text: 'when called through '), + TextSpan( + text: '${stackFrame.name}', + style: fixedStyle, + ), + ], + ); + } @override Widget build(BuildContext context) { diff --git a/packages/devtools_app/lib/src/screens/profiler/panes/call_tree.dart b/packages/devtools_app/lib/src/screens/profiler/panes/call_tree.dart index 3d51f6eae85..64494a46afb 100644 --- a/packages/devtools_app/lib/src/screens/profiler/panes/call_tree.dart +++ b/packages/devtools_app/lib/src/screens/profiler/panes/call_tree.dart @@ -18,17 +18,19 @@ class CpuCallTreeTable extends StatelessWidget { required bool displayTreeGuidelines, }) { final treeColumn = MethodAndSourceColumn(); - final startingSortColumn = TotalTimeColumn(titleTooltip: totalTimeTooltip); + final selfTimeColumn = SelfTimeColumn(titleTooltip: selfTimeTooltip); + final totalTimeColumn = TotalTimeColumn(titleTooltip: totalTimeTooltip); final columns = List>.unmodifiable([ - startingSortColumn, - SelfTimeColumn(titleTooltip: selfTimeTooltip), + totalTimeColumn, + selfTimeColumn, treeColumn, ]); + return CpuCallTreeTable._( key, dataRoots, treeColumn, - startingSortColumn, + totalTimeColumn, columns, displayTreeGuidelines, ); diff --git a/packages/devtools_app/lib/src/shared/table/table.dart b/packages/devtools_app/lib/src/shared/table/table.dart index dc8f63924d0..da7e853c648 100644 --- a/packages/devtools_app/lib/src/shared/table/table.dart +++ b/packages/devtools_app/lib/src/shared/table/table.dart @@ -1506,9 +1506,11 @@ class _TableRowState extends State> ); final tooltip = column.getTooltip(node); - if (tooltip.isNotEmpty) { + final richTooltip = column.getRichTooltip(node, context); + if (tooltip.isNotEmpty || richTooltip != null) { content = DevToolsTooltip( - message: tooltip, + message: richTooltip == null ? tooltip : null, + richMessage: richTooltip, waitDuration: tooltipWaitLong, child: content, ); diff --git a/packages/devtools_app/lib/src/shared/table/table_data.dart b/packages/devtools_app/lib/src/shared/table/table_data.dart index bb2d98dbccd..cc29e4ac69b 100644 --- a/packages/devtools_app/lib/src/shared/table/table_data.dart +++ b/packages/devtools_app/lib/src/shared/table/table_data.dart @@ -69,10 +69,15 @@ abstract class ColumnData { String? getCaption(T dataObject) => null; - // TODO(kenz): this isn't hooked up to the table elements. Do this. /// Get the cell's tooltip value from the given [dataObject]. String getTooltip(T dataObject) => getDisplayValue(dataObject); + /// Get the cell's rich tooltip span from the given [dataObject]. + /// + /// If both [getTooltip] and [getRichTooltip] are provided, the rich tooltip + /// will take precedence. + InlineSpan? getRichTooltip(T dataObject, BuildContext context) => null; + /// Get the cell's text color from the given [dataObject]. Color? getTextColor(T dataObject) => null; @@ -174,6 +179,8 @@ abstract class TimeAndPercentageColumn extends ColumnData { required String title, required this.percentAsDoubleProvider, this.timeProvider, + this.tooltipProvider, + this.richTooltipProvider, this.secondaryCompare, this.percentageOnly = false, double columnWidth = _defaultTimeColumnWidth, @@ -190,6 +197,10 @@ abstract class TimeAndPercentageColumn extends ColumnData { double Function(T) percentAsDoubleProvider; + String Function(T)? tooltipProvider; + + InlineSpan? Function(T, BuildContext)? richTooltipProvider; + Comparable Function(T)? secondaryCompare; final bool percentageOnly; @@ -221,5 +232,9 @@ abstract class TimeAndPercentageColumn extends ColumnData { } @override - String getTooltip(T dataObject) => ''; + String getTooltip(T dataObject) => tooltipProvider?.call(dataObject) ?? ''; + + @override + InlineSpan? getRichTooltip(T dataObject, BuildContext context) => + richTooltipProvider?.call(dataObject, context); } diff --git a/packages/devtools_app/lib/src/shared/theme.dart b/packages/devtools_app/lib/src/shared/theme.dart index a4a16aaab53..fccbe98a2a7 100644 --- a/packages/devtools_app/lib/src/shared/theme.dart +++ b/packages/devtools_app/lib/src/shared/theme.dart @@ -422,7 +422,7 @@ extension ThemeDataExtension on ThemeData { TextStyle get selectedFixedFontStyle => fixedFontStyle.copyWith(color: colorScheme.devtoolsSelectedLink); - TextStyle get toolTipFixedFontStyle => fixedFontStyle.copyWith( + TextStyle get tooltipFixedFontStyle => fixedFontStyle.copyWith( color: colorScheme.tooltipTextColor, ); From 47761ec1cc0ce87e7aab2d282b7276aa22cdc639 Mon Sep 17 00:00:00 2001 From: Kenzie Schmoll Date: Fri, 3 Mar 2023 09:11:31 -0800 Subject: [PATCH 2/2] review comments --- .../screens/profiler/cpu_profile_columns.dart | 4 +-- .../src/screens/profiler/panes/bottom_up.dart | 29 ++++++++++--------- .../lib/src/shared/table/table_data.dart | 4 ++- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart index 12cf9006006..907a3146f3c 100644 --- a/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart +++ b/packages/devtools_app/lib/src/screens/profiler/cpu_profile_columns.dart @@ -16,7 +16,7 @@ import 'cpu_profile_model.dart'; class SelfTimeColumn extends TimeAndPercentageColumn { SelfTimeColumn({ String? titleTooltip, - InlineSpan? Function(CpuStackFrame, BuildContext)? dataTooltipProvider, + RichTooltipBuilder? dataTooltipProvider, }) : super( title: 'Self Time', titleTooltip: titleTooltip, @@ -30,7 +30,7 @@ class SelfTimeColumn extends TimeAndPercentageColumn { class TotalTimeColumn extends TimeAndPercentageColumn { TotalTimeColumn({ String? titleTooltip, - InlineSpan? Function(CpuStackFrame, BuildContext)? dataTooltipProvider, + RichTooltipBuilder? dataTooltipProvider, }) : super( title: 'Total Time', titleTooltip: titleTooltip, diff --git a/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart b/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart index ee17f14b2ae..0c500e50c17 100644 --- a/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart +++ b/packages/devtools_app/lib/src/screens/profiler/panes/bottom_up.dart @@ -60,17 +60,20 @@ class CpuBottomUpTable extends StatelessWidget { final List bottomUpRoots; final bool displayTreeGuidelines; - static const totalTimeTooltip = - 'Time that a method spent executing its own code as well as the code for ' - 'the\nmethod that it called (which is displayed as an ancestor in the ' - 'bottom up tree).'; + static const totalTimeTooltip = ''' +For top-level methods in the bottom-up tree (stack frames that were at the top of at +least one CPU sample), this is the time the method spent executing its own code, +as well as the code for any methods that it called. - static const selfTimeTooltip = - 'For top-level methods in the bottom-up tree (stack frames that were at ' - 'the top of at least one CPU sample), this is the time the method spent ' - 'executing only its own code.\mFor children methods in the bottom-up ' - 'tree (the callers), this is the self time of the top-level method (the ' - 'callee) when called through the child method (the caller).'; +For children methods in the bottom-up tree (the callers), this is the total time of +the top-level method (the callee) when called through the child method (the caller).'''; + + static const selfTimeTooltip = ''' +For top-level methods in the bottom-up tree (stack frames that were at the top of at +least one CPU sample), this is the time the method spent executing only its own code. + +For children methods in the bottom-up tree (the callers), this is the self time of +the top-level method (the callee) when called through the child method (the caller).'''; static InlineSpan? _bottomUpTimeTooltipBuilder( String type, @@ -87,12 +90,12 @@ class CpuBottomUpTable extends StatelessWidget { children: [ TextSpan(text: '$type time for '), TextSpan( - text: '${stackFrame.root.name}\n', + text: stackFrame.root.name, style: fixedStyle, ), - const TextSpan(text: 'when called through '), + const TextSpan(text: '\nwhen called through '), TextSpan( - text: '${stackFrame.name}', + text: stackFrame.name, style: fixedStyle, ), ], diff --git a/packages/devtools_app/lib/src/shared/table/table_data.dart b/packages/devtools_app/lib/src/shared/table/table_data.dart index cc29e4ac69b..56b71dcd690 100644 --- a/packages/devtools_app/lib/src/shared/table/table_data.dart +++ b/packages/devtools_app/lib/src/shared/table/table_data.dart @@ -166,6 +166,8 @@ extension ColumnDataExtension on ColumnData { } } +typedef RichTooltipBuilder = InlineSpan? Function(T, BuildContext); + /// Column that, for each row, shows a time value in milliseconds and the /// percentage that the time value is of the total time for this data set. /// @@ -199,7 +201,7 @@ abstract class TimeAndPercentageColumn extends ColumnData { String Function(T)? tooltipProvider; - InlineSpan? Function(T, BuildContext)? richTooltipProvider; + RichTooltipBuilder? richTooltipProvider; Comparable Function(T)? secondaryCompare;