Bug 148457

Summary: Web Inspector: Rendering Frames timeline pie chart should use SVG instead of 2D canvas
Product: WebKit Reporter: Matt Baker <mattbaker>
Component: Web InspectorAssignee: Matt Baker <mattbaker>
Status: RESOLVED FIXED    
Severity: Normal CC: bburg, commit-queue, graouts, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[Image] Updated UI using SVG
none
[Patch] WIP
none
[Patch] WIP
none
[Patch] Proposed Fix
none
[Image] UI - Empty and populated charts
none
[Patch] WIP mattbaker: commit-queue-

Description Matt Baker 2015-08-25 18:19:43 PDT
* SUMMARY
Rendering Frames timeline pie chart should use SVG instead of 2D canvas.
Comment 1 Radar WebKit Bug Importer 2015-08-25 18:20:02 PDT
<rdar://problem/22431337>
Comment 2 Matt Baker 2015-08-25 18:20:23 PDT
Created attachment 259909 [details]
[Image] Updated UI using SVG
Comment 3 Matt Baker 2015-08-25 22:03:08 PDT
Created attachment 259922 [details]
[Patch] WIP
Comment 4 Matt Baker 2015-08-25 22:05:50 PDT
Created attachment 259923 [details]
[Patch] WIP
Comment 5 Timothy Hatcher 2015-08-26 02:44:23 PDT
Comment on attachment 259923 [details]
[Patch] WIP

I'd be curious if reusing the path elements and just updating "d" when nothing else needs to change would be even better.
Comment 6 Timothy Hatcher 2015-08-26 02:46:29 PDT
Comment on attachment 259923 [details]
[Patch] WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=259923&action=review

> Source/WebInspectorUI/UserInterface/Views/ChartDetailsSectionRow.js:348
> +            let path = createSVGElement("path");

These could get class names and have the stroke, stroke-width and even color be set via a style sheet now.
Comment 7 Matt Baker 2015-08-26 13:07:01 PDT
Comment on attachment 259923 [details]
[Patch] WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=259923&action=review

>> Source/WebInspectorUI/UserInterface/Views/ChartDetailsSectionRow.js:348
>> +            let path = createSVGElement("path");
> 
> These could get class names and have the stroke, stroke-width and even color be set via a style sheet now.

Nice idea! We can definitely set all the stroke properties in CSS, since stroke color isn't based on the item color.
Comment 8 Matt Baker 2015-08-26 17:15:58 PDT
Created attachment 260001 [details]
[Patch] Proposed Fix
Comment 9 Matt Baker 2015-08-26 17:23:18 PDT
Created attachment 260004 [details]
[Image] UI - Empty and populated charts
Comment 10 WebKit Commit Bot 2015-08-26 18:10:01 PDT
Comment on attachment 260001 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 260001

Committed r189006: <http://trac.webkit.org/changeset/189006>
Comment 11 WebKit Commit Bot 2015-08-26 18:10:05 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 BJ Burg 2015-08-27 08:19:26 PDT
Comment on attachment 260001 [details]
[Patch] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=260001&action=review

> Source/WebInspectorUI/UserInterface/Views/ChartDetailsSectionRow.css:50
> +    stroke: rgba(255, 255, 255, 0.7);

I think we are preferring hsla()
Comment 13 Matt Baker 2015-08-28 14:39:16 PDT
Reopening to attach new patch.
Comment 14 Matt Baker 2015-08-28 14:39:19 PDT
Created attachment 260179 [details]
[Patch] WIP

Just need to write a test
Comment 15 Matt Baker 2015-08-28 14:40:03 PDT
(In reply to comment #14)
> Created attachment 260179 [details]
> [Patch] WIP
> 
> Just need to write a test

Attached to wrong bug. Ignore.