Top sites search tiles' magnifier icon looks weird

RESOLVED FIXED in Firefox 64

Status

()

defect
P1
normal
RESOLVED FIXED
10 months ago
9 months ago

People

(Reporter: mak, Assigned: jrmuizel)

Tracking

(Blocks 2 bugs, {regression})

Trunk
mozilla64
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox62 disabled, firefox63 disabled, firefox64 fixed)

Details

Attachments

(4 attachments)

Reporter

Description

10 months ago
Posted image 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.
We're having trouble reproducing this on Windows 10 and Mac, marco.  Any more specific STRs?
Flags: needinfo?(mak77)
Reporter

Comment 2

10 months ago
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.
Component: Activity Streams: Newtab → Graphics: WebRender
Flags: needinfo?(mak77)
Product: Firefox → Core
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.
Priority: -- → P1
See Also: → 1452337
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
Summary: Top search tiles' magnifier icon looks weird → Top sites search tiles' magnifier icon looks weird
Assignee: nobody → emilio
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
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.
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.
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.
Stealing from Emilio. I should be able to implement this today.
Assignee: emilio → jmuizelaar
My first patch was garbage. Will need to do something more nuanced.
Comment on attachment 9007268 [details]
Bug 1486377. Avoid repeat sampling when not wanted. r=mstange

Markus Stange [:mstange] has approved the revision.
Attachment #9007268 - Flags: review+

Comment 17

9 months ago
Pushed by jmuizelaar@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c1583f2f382b
Avoid repeat sampling when not wanted. r=mstange

Comment 18

9 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/c1583f2f382b
Status: NEW → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.