Bug 154437 – Web Inspector: Use half-pixel borders for data grids

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Nikita Vasilyev]

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.

[Radar WebKit Bug Importer]

<rdar://problem/24736365>

[Nikita Vasilyev]

Created attachment 272445 [details]
Patch

[Nikita Vasilyev]

Created attachment 272447 [details]
[Image] Before/After

[Blaze Burg]

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?

[Timothy Hatcher]

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.

[Timothy Hatcher]

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

[Nikita Vasilyev]

(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.

[Nikita Vasilyev]

(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.

[Nikita Vasilyev]

Created attachment 273160 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 273160 [details]
Patch

Clearing flags on attachment: 273160

Committed r197659: <http://trac.webkit.org/changeset/197659>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.