Bug 148108

Summary: Web Inspector: New rule button in the style sidebar is blurry and too dark
Product: WebKit Reporter: Timothy Hatcher <timothy>
Component: Web InspectorAssignee: Devin Rousso <hi>
Status: RESOLVED FIXED    
Severity: Normal CC: agomez, bburg, cgarcia, commit-queue, graouts, hi, joepeck, mattbaker, nvasilyev, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Screenshot
none
Patch
none
Patch
none
Patch timothy: review+, commit-queue: commit-queue-

Timothy Hatcher
Reported 2015-08-17 16:59:49 PDT
Created attachment 259211 [details] Screenshot It looks like it is scaled and full black.
Attachments
Screenshot (4.44 KB, image/png)
2015-08-17 16:59 PDT, Timothy Hatcher
no flags
Patch (1.54 KB, patch)
2015-08-17 18:23 PDT, Devin Rousso
no flags
Patch (1.63 KB, patch)
2015-08-18 17:39 PDT, Devin Rousso
no flags
Patch (14.82 KB, patch)
2015-08-19 17:35 PDT, Devin Rousso
timothy: review+
commit-queue: commit-queue-
Radar WebKit Bug Importer
Comment 1 2015-08-17 17:00:01 PDT
Devin Rousso
Comment 2 2015-08-17 18:23:48 PDT
Timothy Hatcher
Comment 3 2015-08-17 19:16:21 PDT
Comment on attachment 259220 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259220&action=review > Source/WebInspectorUI/UserInterface/Views/CSSStyleDetailsSidebarPanel.css:85 > + margin: 5.5px 0 0 6px; Will this work on non-retina? Are we scaling the SVG? Why 5.5 and 6 mixed?
Devin Rousso
Comment 4 2015-08-17 19:17:57 PDT
(In reply to comment #3) > > Source/WebInspectorUI/UserInterface/Views/CSSStyleDetailsSidebarPanel.css:85 > > + margin: 5.5px 0 0 6px; > > Will this work on non-retina? Are we scaling the SVG? Why 5.5 and 6 mixed? Oh crap I totally forgot about non-retina. I'll need to check that.
Timothy Hatcher
Comment 5 2015-08-17 20:21:11 PDT
We should just make the SVG be correct and use whole pixel positions.
Devin Rousso
Comment 6 2015-08-18 17:39:10 PDT
Devin Rousso
Comment 7 2015-08-18 21:09:03 PDT
Comment on attachment 259331 [details] Patch This still does not look nice on non-retina. I will need to fix this, but I am not quite sure how yet...
Devin Rousso
Comment 8 2015-08-19 17:35:47 PDT
Timothy Hatcher
Comment 9 2015-08-20 17:15:58 PDT
Comment on attachment 259435 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=259435&action=review > Source/WebInspectorUI/ChangeLog:15 > + * UserInterface/Images/Plus13.svg: Copied from Source/WebInspectorUI/UserInterface/Images/Plus.svg. > + * UserInterface/Images/Plus15.svg: Copied from Source/WebInspectorUI/UserInterface/Images/Plus.svg. Maybe PlusSmall and PlusLarge? The numbers bother me. > Source/WebInspectorUI/UserInterface/Images/Minus.svg:4 > - <line class="stroked" fill="none" stroke="black" x1="1.5" y1="6.5" x2="11.5" y2="6.5"/> > + <line class="stroked" fill="none" stroke="black" x1="1" y1="6.5" x2="12" y2="6.5"/> This seems like it would be blurry now. > Source/WebInspectorUI/UserInterface/Images/NewTabPlus.svg:4 > + <path d="M 7.5 2 V 13 M 2 7.5 H 13" class="stroked" fill="none" stroke="black"/> Ditto.
Devin Rousso
Comment 10 2015-08-20 17:22:54 PDT
(In reply to comment #9) > Maybe PlusSmall and PlusLarge? The numbers bother me. I went with Plus13 and Plus15 at Joe's suggestion because we also have a NewTabPlus.svg, which has slightly thicker lines. Also, since Plus is so common, appending the square size will let us easily identify how big it is (we may need a Plus17 in the future, so PlusLarger doesn't make a lot of sense there). > This seems like it would be blurry now. So I actually talked with Dean Jackson and he explained that SVG elements have their pixel values on a 0.5 unit, so if you set a line on x=1.5, the line will draw from 1 to 2. It's similar to what we have been doing to avoid blurry edges. I tested it on both retina and non and it looked fine.
Timothy Hatcher
Comment 11 2015-08-20 18:05:23 PDT
I knew it was half pixel aligned. I just didn't expect to see whole pixel values in the path.
WebKit Commit Bot
Comment 12 2015-08-20 18:41:59 PDT
Comment on attachment 259435 [details] Patch Rejecting attachment 259435 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 259435, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: /git.webkit.org/WebKit 8bcad24..e91f121 master -> origin/master Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ... Currently at 188731 = 8bcad240f15aaad40b4b85766086c1f4ebd2ccb2 r188732 = e91f1219ec679745f8073ad53af0dcee40aa8a20 Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc First, rewinding head to replay your work on top of it... Fast-forwarded master to refs/remotes/origin/master. Full output: http://webkit-queues.webkit.org/results/83640
Joseph Pecoraro
Comment 13 2015-09-01 11:13:24 PDT
This was reviewed but hasn't landed yet! I'll look into manually landed it.
Joseph Pecoraro
Comment 14 2015-09-01 11:39:15 PDT
Note You need to log in before you can comment on or make changes to this bug.