Bug 154437 - Web Inspector: Use half-pixel borders for data grids
Summary: Web Inspector: Use half-pixel borders for data grids
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: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on: 150383
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-18 22:25 PST by Nikita Vasilyev
Modified: 2016-03-06 22:41 PST (History)
8 users (show)

See Also:


Attachments
Patch (2.92 KB, patch)
2016-02-27 23:39 PST, Nikita Vasilyev
timothy: review-
Details | Formatted Diff | Diff
[Image] Before/After (128.69 KB, image/png)
2016-02-27 23:46 PST, Nikita Vasilyev
no flags Details
Patch (2.72 KB, patch)
2016-03-06 21:40 PST, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2016-02-18 22:25:29 PST
Since Bug 146619 Web Inspector: Use "hairline" borders on retina screen,
half-pixel borders, aka "hairlines", are extensively used on retina screens.

They are, however, not used in data grids because WebKit doesn't support
them for table cells (Bug 154435).

The solution would be to either:
* Fix Bug 154435;
* Come up with a workaround.
Comment 1 Radar WebKit Bug Importer 2016-02-18 22:25:52 PST
<rdar://problem/24736365>
Comment 2 Nikita Vasilyev 2016-02-27 23:39:12 PST
Created attachment 272445 [details]
Patch
Comment 3 Nikita Vasilyev 2016-02-27 23:46:25 PST
Created attachment 272447 [details]
[Image] Before/After
Comment 4 BJ Burg 2016-02-28 07:34:54 PST
Comment on attachment 272445 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:702
> +            left += this._headerTableBodyElement.rows[0].cells[i].getBoundingClientRect().width;

Is this more expensive to compute than offsetWidth? Does it matter?

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:706
> +                resizer.element.style.left = Math.round(left) + "px";

Why do we need to round this, if we computed subpixel precision widths for columns?
Comment 5 Timothy Hatcher 2016-02-28 09:59:13 PST
Comment on attachment 272445 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DataGrid.css:303
> +        border-spacing: 0.5px;

Can we define --border-width and get rid of the extra @media (-webkit-min-device-pixel-ratio: 2) rules we have?

Define --border-width as 1px and have a @media (-webkit-min-device-pixel-ratio: 2) rule to define it as 0.5px.
Comment 6 Timothy Hatcher 2016-03-04 09:09:59 PST
Comment on attachment 272445 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Views/DataGrid.css:302
> +        border-collapse: separate;

You should not need this after this change: http://trac.webkit.org/changeset/197524
Comment 7 Nikita Vasilyev 2016-03-06 21:29:14 PST
(In reply to comment #4)
> Comment on attachment 272445 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272445&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DataGrid.js:702
> > +            left += this._headerTableBodyElement.rows[0].cells[i].getBoundingClientRect().width;
> 
> Is this more expensive to compute than offsetWidth? Does it matter?

I profiled DataGrid in Timelines. It was about the same as offsetWidth.

For initial DataGrid render, it usually takes 0.1-0.5ms for every
offsetWidth or getBoundingClientRect() call.
When resizing, it takes <0.01ms.

> 
> > Source/WebInspectorUI/UserInterface/Views/DataGrid.js:706
> > +                resizer.element.style.left = Math.round(left) + "px";
> 
> Why do we need to round this, if we computed subpixel precision widths for
> columns?

Actually, we don't. Thanks for pointing out!
I thought 2.5px on retina (device pixel ratio of 2) would be rounded down to 2px,
but it actually renders it as 2.5px, e.g. 5 physical pixels.
Comment 8 Nikita Vasilyev 2016-03-06 21:40:05 PST
(In reply to comment #6)
> Comment on attachment 272445 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272445&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/DataGrid.css:302
> > +        border-collapse: separate;
> 
> You should not need this after this change:
> http://trac.webkit.org/changeset/197524

After http://trac.webkit.org/changeset/197627, to be exact.
Comment 9 Nikita Vasilyev 2016-03-06 21:40:36 PST
Created attachment 273160 [details]
Patch
Comment 10 WebKit Commit Bot 2016-03-06 22:41:42 PST
Comment on attachment 273160 [details]
Patch

Clearing flags on attachment: 273160

Committed r197659: <http://trac.webkit.org/changeset/197659>
Comment 11 WebKit Commit Bot 2016-03-06 22:41:45 PST
All reviewed patches have been landed.  Closing bug.