1568684 - URL bar looks bad with WebRender on

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[Jeff Muizelaar [:jrmuizel]]

Attachment: no-wr.png

The rounded corners look bad and the box is thicker at the bottom. See screenshots.
I see this with device pixel ratio = 1.5

Comment 1

[Jeff Muizelaar [:jrmuizel]]

Attachment: wr.png

Comment 2

[Jeff Muizelaar [:jrmuizel]]

It would be good to get a reduced test case of this.

Comment 3

[Jeff Muizelaar [:jrmuizel]]

I'm seeing this problem when using a devicePixelRatio of 1.5

I've been debugging this a bunch. It looks like layout is giving us unrounded border widths (1.5px) and that's causing bad.

Layout seems to do this because of set_border_top_width is using a value of 60 for mTwipsPerPixel

https://searchfox.org/mozilla-central/rev/1dfd70469212ef2785d41827c5532c571c784227/servo/components/style/properties/gecko.mako.rs#530

while the bordering rendering code is using a value of 40.

mTwipsPerPixel is getting 60 from https://searchfox.org/mozilla-central/rev/1dfd70469212ef2785d41827c5532c571c784227/layout/style/nsStyleStruct.cpp#326 because it has no prescontext.

This differing values of appunits seems bad. Emilio?

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Hmm, the initial style should be invalidated once we get a pres shell... I'll take a look.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I also think we should do the snapping later generally, but that's a more general problem...

Comment 6

[Emilio Cobos Álvarez (:emilio)]

So when I poke at the border rendering code I get 40 app units, which is the expected thing afaict, right?

In nsCSSBorderRenderer::CreateWebRenderCommands for that border (the one in #about-config-search):

(rr) p aItem->Frame()->StyleBorder()->mTwipsPerPixel
$9 = 40
(rr) p aItem->Frame()->StyleBorder()->mComputedBorder
$10 = {<mozilla::gfx::BaseMargin<int, nsMargin>> = {top = 40, right = 40, bottom = 40, left = 40}, <No data fields>}

Is there something I'm missing? As far as I can tell all that is expected (there are 40 app units per device pixel at with layout.css.devPixelsPerPx=1.5).

Where are you seeing the wrong mTwipsPerPx? It is expected that we start with 60 initially if there's no pres context, but as soon as we get one we end up here, which eventually gets here which recomputes the style.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

Can repro, sorry:

18:09 <emilio> jrmuizel: oh, nvm, I see... The urlbar goes wrong and gets fixed when resized
18:09 <emilio> jrmuizel: the searchbar in about:config remains wrongly snapped regardless
18:10 <emilio> jrmuizel: (and that's what I was debugging, since I don't know how to read bug titles ;_;)

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1568684 - Initial style needs to be invalidated even if we haven't had stylesheets earlier. r=heycam

The test unfortunately passes without the patch because for HTML documents we go
through DoUpdateCharSet which ends up through RebuildAllStyleData. So I think
this bug is not web-observable.

I'll file a bug to try to optimize parts of that away, since I think that we can
be doing a bit of wasteful work there for somewhat common cases...

Comment 9

[Darkspirit]

Could this somehow fix bug 1489463?

Comment 10

[Emilio Cobos Álvarez (:emilio)]

Pretty unlikely.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/21c18dee8d84
Initial style needs to be invalidated even if we haven't had stylesheets earlier. r=heycam

Comment 12

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/21c18dee8d84

Comment 13

[Daniel Cicas [:dcicas], Release QA]

Hello,

I want to verify if this bug is fixed in Fx 70.b10 but it seems i would need a reduced test case or maybe I'm missing something because as far as I can see the only way to have a device pixel ratio of 1.5 is to do it in RDM by making a custom device with that pixel ratio. But my problem is that the search bar in about:config is not accessible in RDM mode. I would be grateful for any help!

Comment 14

[Emilio Cobos Álvarez (:emilio)]

You can set layout.css.devPixelsPerPx to 1.5 instead (and probably needs a browser restart after changing it, or opening a new window to see the bug on bad builds).

Comment 15

[Daniel Cicas [:dcicas], Release QA]

Attachment: 70b10wr-enabled.PNG

Comment 16

[Daniel Cicas [:dcicas], Release QA]

Thank you Emilio for the help!

So on Fx 70b10 with webrender on and a devicePixelRatio of 1.5 the bottom part of the search bar has a small white line/a space 1 pixel wide.

Comment 17

[Emilio Cobos Álvarez (:emilio)]

That is the search bar in about:config. This bug is about the url bar.

Comment 18

[Daniel Cicas [:dcicas], Release QA]

Hello,

Sorry about that, I got confused by the screenshot as both the search bar and the url bar looked pretty bad in the screenshot. I can verify that this issue is fixed on Fx 70.0b13.