Migrating Webcompat whiteboard priorities to project flags. See bug 1547409.

See bug 1547409. Migrating whiteboard priority tags to program flags.

This problem can also occur for the widely used shapes defined with css.
For example the following will result in gaps for Firefox' max zoom of 300%:

<div style="width: 16px; height: 16px; border: 1px solid red">
  <div style="border: 8px solid red; border-color: transparent red; width: 0"></div>
</div>

It is possible to work around the issue by not defining the outer size and using display: inline-block.
I found the behaviour unexpected though.

I could also reproduce this with display: inline-block. A minimal testcase is attached; it originally broke on a real site, with a demo about mazes (https://www.jamisbuck.org/mazes/).

Note that the testcase will render correctly at the default zoom level, but there will be discontinuities in the background as well as in the width of the maze.

So, borders are snapped to device pixels here: https://searchfox.org/mozilla-central/rev/9b99e1d9c6cf83539674cb016c7373f549ba59ca/servo/components/style/properties/gecko.mako.rs#516

It doesn't seem like other browsers do this anymore... We should probably change this. David, is there any good reason for this behavior nowadays? I always found the "round to dev-pixels" pretty weird.

I guess the most useful feature of this is making border: 0.3px and such not round down to 0px...

The most reduced test-case would be data:text/html,<div style="border: 0.1px solid;">Foo, I suspect. getComputedStyle(document.querySelector("div")).borderTopWidth would return 0.1px in other browsers, and 0.5px in Firefox (in a HiDPI display, 1px in a regular display, I guess).

Let graphics snap as it wants before doing so. Rendering of small borders looks
fine in here even with this patch.

This enables further cleanups too.

I sent a patch removing this, and confirmed it fixes both test-cases, but I haven't submitted it for review yet (so take it as a WIP). Let's see what does CI say: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f591313619e690ee25703f5da15fb80a43dc67aa

I think there still is good reason to do this snapping. It produces better visual results; without this, some widths of borders are different widths in device pixels depending on their position, which produces very bad effects. There's a much longer explanation somewhere that I'd like to find later.

I guess the most useful feature of this is making border: 0.3px and such not round down to 0px...

The rounding code to do this at least used to explicitly round anything between 0 and 1 device pixels up to 1.

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁰󠁡󠁿 ⌚UTC-5 from comment #28)

I think there still is good reason to do this snapping. It produces better visual results; without this, some widths of borders are different widths in device pixels depending on their position, which produces very bad effects. There's a much longer explanation somewhere that I'd like to find later.

While that may be true, that's also an issue for a bunch of other stuff like backgrounds and such, right? I think it's mostly graphics' job to snap pixels to device position. Asymmetric borders are not great though.

In any case I think rounding at computed-value time is pretty weird... The fact that the computed style is affected by your DPI has always looked like a bug to me.

(In reply to David Baron :dbaron: 🏴󠁵󠁳󠁰󠁡󠁿 ⌚UTC-5 from comment #29)

The rounding code to do this at least used to explicitly round anything between 0 and 1 device pixels up to 1.

Yeah it still rounds anything non-zero to 1 dev px.

Also, don't you have the same issue anyway with border widths depending on position if you have a subpixel width between them?

From the CI run in comment 27, a bunch of tests fail:

• Vast majority of them testing for our current snapping in particular (not too concerned about those).
• Some of them are minor antialising bits which can be trivially fixed (the fragmentation and pagination-related ones).

All in all those are fixable, but I think if we landed my patch at the very least should not just remove the code, but keep it disabled only on Nightly for quite a bit to watch for regressions...

I am running into this issue when I create buttons with borders that are only enabled when you hover over the button. This is using Firefox 75.0 on Windows 10. Here is a simple test case:

<!doctype html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Firefox border bug</title>
<style>
.button {
	background-color: black;
	font-family: Arial, sans-serif;
	width: 100px;
	height: 50px;
	line-height: 50px;
	text-align: center;
	color: white;
}
.button:hover {
	border: 2px solid black;
	color: black;
	background-color: white;
	width: 96px;
	height: 46px;
	line-height: 46px;
}
a {
	text-decoration: none;
}
td {
	padding: 5px;
}
</style>
</head>
<body>
<table>
<tbody>
<tr>
<td>
<a href="http://www.example.com">
<div class="button">
Click Here
</div>
</a>
</td>
<td>
<a href="http://www.example.com">
<div class="button">
Click Here
</div>
</a>
</td>
</tbody>
</table>
</body>
</html>

If you view this in Firefox 75.0 with the Windows scale set to 125%, then the borders when the button is hovered over will display as 1.6px (instead of 2px). The borders will also show with incorrect sizes if the Windows scale is set to anything other than 100%. As a result, if you hover over the left button in this example, the right button will shift slightly to the left, because the width of the left button (including border) is reduced to 99.2px instead of 100px. If you view this in Firefox with the Windows scale set to 100% it displays correctly, and it displays correctly in all other browsers I have tested (Internet Explorer 11.778.18362.0, Chrome 81.0.4044.122, Edge 44.18362.449.0, Opera 68.0.3618.56).

David, Mats: Phabricator-bugzilla integration seems slow / broken today, but what do you think of https://phabricator.services.mozilla.com/D75360?

It seems closer to what other browsers are doing based on the discussion in the linked Chromium bug, and allows subpixel borders to work as expected.

Daniel, your feedback on the above revision would also be appreciated.

I'm thinking maybe we should ceil rather than floor the border line width, so that we don't ever leave gaps. But on the other hand that can cause borders to overlap.

This seems to match what other browsers do, and seems saner layout-wise,
at least.

So if you're flooring at paint time, which edge are you aligning with (for the cases where the paint-time flooring changes the number of pixels actually covered)? (top/left edge? outer edge?) And what do other browsers do?

It seems to me that aligning with the outer edge would be most desirable since that would avoid having a device pixel of background visible outside the border in those cases.

We align to the outer edges, yeah. I believe other browsers do the same.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #25)

The most reduced test-case would be data:text/html,<div style="border: 0.1px solid;">Foo, I suspect. getComputedStyle(document.querySelector("div")).borderTopWidth would return 0.1px in other browsers, and 0.5px in Firefox (in a HiDPI display, 1px in a regular display, I guess).

Does the approach decided, if decided, i.e "snapping?" to outer edges - does this eliminate the "leak" of the devicePixelRatio (as per email mid January). Or is this still layout-exposed?

(In reply to Simon Mainey from comment #42)

Does the approach decided, if decided, i.e "snapping?" to outer edges - does this eliminate the "leak" of the devicePixelRatio (as per email mid January). Or is this still layout-exposed?

It's still exposed as we round up to at least one device pixel, and that is still exposed in getComputedStyle and other layout APIs. It's a bit less visible though.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #37)

Daniel, your feedback on the above revision would also be appreciated.

I may not have time to look closely at this before you're ready to land it, but I'll bet mats' feedback likely captures any objections/concerns that I might have on the approach.

(I'm happy that we're addressing this compat issue & minor footgun - thanks for tackling it!)

https://hg.mozilla.org/mozilla-central/rev/5df17ecbcaa1
https://hg.mozilla.org/mozilla-central/rev/04543b8ded50

I reverted this change, see https://hg.mozilla.org/integration/autoland/rev/e9818ea993d02a724e56da4dd9167fc1179de68f / bug 1645008.

Though that means that probably we should just WONTFIX this.

Backout merged: https://hg.mozilla.org/mozilla-central/rev/e9818ea993d0

Alert before backout:

== Change summary for alert #26249 (as of Wed, 17 Jun 2020 06:18:53 GMT) ==

Regressions:

10% facebook-cold Similarity2D android-hw-p2-8-0-android-aarch64-shippable opt 0.77 -> 0.69
8% facebook-cold Similarity android-hw-p2-8-0-android-aarch64-shippable opt 0.78 -> 0.71

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=26249