1486377 - Top sites search tiles' magnifier icon looks weird

Status: RESOLVED FIXED

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

Description

[Marco Bonardo [:mak]]

Attachment: Screenshot (42).png

I see a line and a dot below the magnifier icon, that looks part of the icon but shifted. The screenshot is self explanatory.
I think it's some kind of rounding problem, due to the 26px height, any other value seems to work properly. I'm at 125% DPI setting on Windows 10.

Comment 1

[Tim Spurway [:tspurway]]

We're having trouble reproducing this on Windows 10 and Mac, marco.  Any more specific STRs?

Comment 2

[Marco Bonardo [:mak]]

I can reproduce just by setting the dpi to 125% and resizing the browser window vertically.
BUT, you made me think it could be specific to my setup, and indeed it's a webrender bug, if I disable webrender it works correctly.

Comment 3

[Aria Beingessner [:Gankra]]

I also noticed this on my mac yesterday, I expect this is a dupe but I'm behind on bugmail so I'll assume this is the unique one until proven otherwise.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

I can reproduce this on Mac. The icon in question is just a single svg icon. It's not part of a sprite sheet. This suggest that this bug is different from bug 1452337

Comment 5

[Jeff Muizelaar [:jrmuizel]]

Attachment: Standalone, somewhat minified, version of the problem

Comment 6

[Darkspirit]

mozregression --good 2018-01-15 --bad 2018-08-27 --pref gfx.webrender.all:true -a https://bug1486377.bmoattachments.org/attachment.cgi?id=9007038
> 12:39.53 INFO: Last good revision: bd889e36b9370ff2e2faafb25c5db56a8f05a6da
> 12:39.53 INFO: First bad revision: dd37857e52a137fd89bbe721fa610ec97a558a2d
> 12:39.53 INFO: Pushlog:
> https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bd889e36b9370ff2e2faafb25c5db56a8f05a6da&tochange=dd37857e52a137fd89bbe721fa610ec97a558a2d

> dd37857e52a1	Andrew Osmond — Bug 1435291 - Part 2. Make background SVGs use WebRender instead of fallback. r=jrmuizel

Comment 7

[Jeff Muizelaar [:jrmuizel]]

So it looks like this is caused by WebRender not supporting no-repeat. Here's the relevant part of the scene:

                    item: Image((
                        image_key: ((3), 62),
                        stretch_size: (62.400001525878906, 62.400001525878906),
                        tile_spacing: (0, 0),
                        image_rendering: Auto,
                        alpha_type: PremultipliedAlpha,
                        color: (
                            r: 1,
                            g: 1,
                            b: 1,
                            a: 1,
                        ),
                    )),

Afaict we don't have anything to prevent the image from repeating other than the stretch_size.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

https://github.com/servo/webrender/issues/1797#issuecomment-412546370

Comment 9

[Jeff Muizelaar [:jrmuizel]]

The take away here is that WebRender will give no-repeat semantics if stretch_size matches the destination rect size:

i.e. if instead of:
                    item: Image((
                        image_key: ((3), 62),
                        stretch_size: (62.400001525878906, 62.400001525878906),
                    ...
                    info: (
                        rect: ((38, 38), (63, 63)),
                        clip_rect: ((38, 38), (63, 63)),
                        is_backface_visible: true,
                        tag: None,
                    ),

we had 
                    item: Image((
                        image_key: ((3), 62),
                        stretch_size: (63, 63),
                        tile_spacing: (0, 0),
                    ...
                    info: (
                        rect: ((38, 38), (63, 63)),
                        clip_rect: ((38, 38), (63, 63)),
                        is_backface_visible: true,
                        tag: None,
                    ),

then we'd get what we want.

This doesn't seem like an entirely unreasonable thing for Gecko to provide.

Comment 10

[Markus Stange [:mstange]]

I agree. So it sounds like Gecko needs to either snap both rects or none of them. I think snapping both is the less risky approach for now.

Comment 11

[Jeff Muizelaar [:jrmuizel]]

Stealing from Emilio. I should be able to implement this today.

Comment 12

[Jeff Muizelaar [:jrmuizel]]

Attachment: Potentential patch

Comment 13

[Jeff Muizelaar [:jrmuizel]]

Attachment: Bug 1486377. Avoid repeat sampling when not wanted. r=mstange

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Lots of failure: https://treeherder.mozilla.org/#/jobs?repo=try&revision=25835910802ef665c65582e573e300757f552bde&selectedJob=198083469

Comment 15

[Jeff Muizelaar [:jrmuizel]]

My first patch was garbage. Will need to do something more nuanced.

Comment 16

[Markus Stange [:mstange]]

Comment on attachment 9007268 [details]
Bug 1486377. Avoid repeat sampling when not wanted. r=mstange

Markus Stange [:mstange] has approved the revision.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1583f2f382b
Avoid repeat sampling when not wanted. r=mstange

Comment 18

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/c1583f2f382b