Bug 153892 - [GTK] Web Inspector: Add new GTK+ icons for instrument icons
Summary: [GTK] Web Inspector: Add new GTK+ icons for instrument icons
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Fujii Hironori
URL:
Keywords: InRadar
Depends on: 153884
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-04 13:19 PST by Joseph Pecoraro
Modified: 2017-04-27 08:53 PDT (History)
12 users (show)

See Also:


Attachments
Patch (92.05 KB, patch)
2017-04-14 02:01 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Apple images and proposed ones (31.03 KB, image/png)
2017-04-14 02:09 PDT, Fujii Hironori
no flags Details
screenshot of before and after this fix (139.36 KB, image/png)
2017-04-14 02:49 PDT, Fujii Hironori
no flags Details
Patch (92.05 KB, patch)
2017-04-14 03:04 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
Patch (68.91 KB, patch)
2017-04-26 23:12 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff
screenshot of before and after this fix (92.49 KB, image/png)
2017-04-26 23:22 PDT, Fujii Hironori
no flags Details
Apple images and proposed ones (51.37 KB, image/png)
2017-04-26 23:33 PDT, Fujii Hironori
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2016-02-04 13:19:51 PST
* SUMMARY
Add new GTK+ icons for instrument icons.

Trunk is getting single SVG icons for the different Timelines / Instruments. It would be best for the GTK images to follow suit!

See bug: 153884
<https://webkit.org/b/153884> Web Inspector: New timeline images for instruments
Comment 1 Radar WebKit Bug Importer 2016-02-04 13:20:23 PST
<rdar://problem/24510460>
Comment 2 Joseph Pecoraro 2016-02-04 13:20:47 PST
Expected paths:
Images/NetworkInstrument.svg
Images/ScriptsInstrument.svg
Images/MemoryInstrument.svg
Images/LayoutInstrument.svg
Images/RenderingFramesInstrument.svg
Comment 3 Joseph Pecoraro 2016-02-04 13:22:55 PST
> Images/MemoryInstrument.svg

Note this is only needed when linux ports start getting ENABLE(RESOURCE_USAGE), but it might be worth adding an icon in the meantime.
Comment 4 Fujii Hironori 2017-04-14 01:56:40 PDT
HeapAllocationsInstrument.svg has been added (Bug 155731)
Comment 5 Fujii Hironori 2017-04-14 02:01:36 PDT
Created attachment 307106 [details]
Patch
Comment 6 Fujii Hironori 2017-04-14 02:09:16 PDT
Created attachment 307107 [details]
Apple images and proposed ones
Comment 7 Fujii Hironori 2017-04-14 02:49:26 PDT
Created attachment 307109 [details]
screenshot of before and after this fix
Comment 8 Fujii Hironori 2017-04-14 03:04:49 PDT
Created attachment 307111 [details]
Patch
Comment 9 Michael Catanzaro 2017-04-14 05:22:49 PDT
Andres, is it OK?
Comment 10 Joseph Pecoraro 2017-04-14 11:14:07 PDT
Comment on attachment 307111 [details]
Patch

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

r=me, but consider simplifying the svg a bit!

> Source/WebInspectorUI/UserInterface/Images/gtk/NetworkInstrument.svg:12
> +<svg
> +   xmlns:dc="http://purl.org/dc/elements/1.1/"
> +   xmlns:cc="http://creativecommons.org/ns#"
> +   xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"
> +   xmlns:svg="http://www.w3.org/2000/svg"
> +   xmlns="http://www.w3.org/2000/svg"
> +   xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"
> +   xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"
> +   sodipodi:docname="NetworkInstrument.svg"

There is a pretty good chance that you can reduce the size of these SVGs significantly by eliminating unnecessary references and simplifying a bit.

  • the metadata is not necessary for rendering (dc, cc, rdf)
  • inkscape / sodipodi attributes appear to be for editors only, they won't affect rendering
  • docname is not necessary, this file already has a name
  • some style attributes have unnecessary properties (especially for fonts)
  • some values can be simplified as the large number of decimals are unlikely to be necessary in some cases (.0002 and .9997)

I can understand if you want to leave some of the attributes to make future editing easier, but we've found that is often not the case with our images.
Comment 11 Joseph Pecoraro 2017-04-14 11:15:37 PDT
You may want to get a review from GTK folks (Andres / Michael). The patch looks good from an inspector point of view.
Comment 12 Fujii Hironori 2017-04-16 21:19:22 PDT
Thank you for reviewing, Joseph.
I didn't know about Andres's script to optimize SVG.

  Bug 143476 - [GTK] Web Inspector: Optimize SVG images
Comment 13 Fujii Hironori 2017-04-17 01:09:56 PDT
This script can not reproduce the same SVG files at the moment because root elements of all SVG files are added id="root" in Bug 150602.
Comment 14 Fujii Hironori 2017-04-26 23:12:42 PDT
Created attachment 308345 [details]
Patch
Comment 15 Fujii Hironori 2017-04-26 23:22:57 PDT
Created attachment 308348 [details]
screenshot of before and after this fix
Comment 16 Fujii Hironori 2017-04-26 23:33:13 PDT
Created attachment 308350 [details]
Apple images and proposed ones
Comment 17 Michael Catanzaro 2017-04-27 08:11:31 PDT
Comment on attachment 308345 [details]
Patch

We really have to remove the nonfree Apple images, it's just getting ridiculous at this point.
Comment 18 WebKit Commit Bot 2017-04-27 08:53:39 PDT
Comment on attachment 308345 [details]
Patch

Clearing flags on attachment: 308345

Committed r215869: <http://trac.webkit.org/changeset/215869>
Comment 19 WebKit Commit Bot 2017-04-27 08:53:40 PDT
All reviewed patches have been landed.  Closing bug.