Closed Bug 1164552 Opened 9 years ago Closed 9 years ago

New Graph colors for performance tool

Categories

(DevTools :: Performance Tools (Profiler/Timeline), defect)

41 Branch
defect
Not set
normal

Tracking

(firefox40 verified, firefox41 fixed)

VERIFIED FIXED
Firefox 41
Tracking Status
firefox40 --- verified
firefox41 --- fixed

People

(Reporter: jsantell, Assigned: jsantell)

References

Details

Attachments

(3 files)

Right now we're using the theme colors defined on MDN and bug 947242, the colors dont lend themselves well to the graphs drawn in performance tools, especially in light theme. Some extra history in bug 1150578.

We need a range of colors to use in non-text settings (which most of the highlight colors seemed to be geared towards that), and since all of the perf tool's components use the theme, and parse the dark/light themes, we'll need to add some variables, like --theme-graph-red or something.
Assignee: nobody → jsantell
Blocks: perf-tool-v2
Status: NEW → ASSIGNED
Created new theme variables --theme-graphs-{COLOR}. For dark theme, these are pretty much the same as highlight colors (some renaming of colors), but the light theme graph colors are way less bold than they were before, and more subtle but saturated now. I think it looks pretty good.
Attachment #8608305 - Flags: review?(vporof)
Attached image Dark Theme markers
Not much different here
Comment on attachment 8608305 [details] [diff] [review]
1164552-perf-colors.patch

Review of attachment 8608305 [details] [diff] [review]:
-----------------------------------------------------------------

So pretty.

::: browser/devtools/shared/timeline/global.js
@@ +48,5 @@
>  const TIMELINE_BLUEPRINT = {
>    /* Group 0 - Reflow and Rendering pipeline */
>    "Styles": {
>      group: 0,
> +    colorName: "graphs-purple",

s/graphs/markers/ maybe?
Attachment #8608305 - Flags: review?(vporof) → review+
Comment on attachment 8608305 [details] [diff] [review]
1164552-perf-colors.patch

Review of attachment 8608305 [details] [diff] [review]:
-----------------------------------------------------------------

"graphs-COLOR" refers to the variable name though, which is used for things like FPS and mem graphs too; not sold on "graphs" bit I don't think "markers" is general enough
https://hg.mozilla.org/mozilla-central/rev/7bcf45a157f1
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 41
Flags: qe-verify+
Comment on attachment 8608305 [details] [diff] [review]
1164552-perf-colors.patch


Approval Request Comment
[Feature/regressing bug #]: 1167252, the new performance tool
[User impact if declined]: Won't ship the performance tool
[Describe test coverage new/current, TreeHerder]: There are try pushes in Bug 1167252 with all patches needing uplift
[Risks and why]: Requesting uplift for the accumulated changes in the performance tool since the 40 merge date, so these changes haven't had the full 6 weeks to bake.  Risks are generally contained within devtools, specifically within the performance panel.
[String/UUID change made/needed]: None
Attachment #8608305 - Flags: approval-mozilla-aurora?
Note: I had verbal confirmation for these uplifts from Sylvestre even before he's flagged them as a+.  See https://bugzilla.mozilla.org/show_bug.cgi?id=1167252#c26
Comment on attachment 8608305 [details] [diff] [review]
1164552-perf-colors.patch

Change approved to skip one train as part of the spring campaign.
Attachment #8608305 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Confirmed that the colors available in the Performance tool's UI are the same as the ones specified in the MDN article [1]. Testing was done on Aurora 40.0a2 (2015-06-09), using Windows 7 (x64), Ubuntu 14.04 (x64) and Mac OS X 10.9.5.

A breakdown of all the item colors verified for this fix is available below.

+--------------------------+------------+-------------+
|                          | Dark theme | Light theme |
+--------------------------+------------+-------------+
| Toolbars background      | #343C45    | #F0F1F2     |
+--------------------------+------------+-------------+
| Selection background     | #1D4F73    | #4C9ED9     |
+--------------------------+------------+-------------+
| Selection text           | #F5F7FA    | #F5F7F5     |
+--------------------------+------------+-------------+
| Splitters                | #000000    | #AAAAAA     |
+--------------------------+------------+-------------+
| Sidebar background       | #181D20    | #F7F7F7     |
+--------------------------+------------+-------------+
| Body background          | #14171A    | #FCFCFC     |
+--------------------------+------------+-------------+
| Body text                | #8FA1B2    | #18191A     |
+--------------------------+------------+-------------+
| Recalculate style marker | #DF80FF    | #B693EB     |
+--------------------------+------------+-------------+
| Layout marker            | #DF80FF    | #B693EB     |
+--------------------------+------------+-------------+
| Paint marker             | #70BF53    | #85D175     |
+--------------------------+------------+-------------+
| DOM event marker         | #D99B28    | #EFC052     |
+--------------------------+------------+-------------+
| Function call marker     | #D99B28    | #EFC052     |
+--------------------------+------------+-------------+
| Parse HTML marker        | #D99B28    | #EFC052     |
+--------------------------+------------+-------------+
| Parse XML marker         | #D99B28    | #EFC052     |
+--------------------------+------------+-------------+
| GC event marker          | #EB5368    | #E57180     |
+--------------------------+------------+-------------+
| Console marker           | #757873    | #CCCCCC     |
+--------------------------+------------+-------------+
| Timestamp marker         | #46AFE3    | #83B7F6     |
+--------------------------+------------+-------------+


[1] https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.