1642526 - Non-root pinch-zoomed scrollbars look odd; thin scrollbar thumb in wide zoomed scrollbar track

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect, P2)

Description

[Markus Stange [:mstange]]

Attachment: Screen Shot 2020-06-01 at 8.57.22 PM.png

On pages with nested scroll frames, zooming the page seems to zoom the scrollbar track but not the scrollbar thumb.

This looks fairly odd - everything is enlarged, except scrollbar thumbs which draw at their original screen pixel thickness and look comically thin compared to the rest of the page.

Comment 1

[Timothy Nikkel (:tnikkel)]

Affects non-overlay as well.

Comment 2

[Botond Ballo [:botond]]

I'm guessing this is an artifact of us asking the native widget code to draw the thumb?

Comment 3

[Markus Stange [:mstange]]

That seems likely.

Comment 4

[Timothy Nikkel (:tnikkel)]

This seems to be the code responsible for it

https://searchfox.org/mozilla-central/rev/8ccea36c4fb09412609fb738c722830d7098602b/widget/cocoa/nsNativeThemeCocoa.mm#2446

Naively expanding the rect doesn't work, which is not surprising at all.

So I guess we'll need to apply a transform to the context if the pass in rect doesn't have the expected width (or height depending on which scrollbar it is)?

Comment 5

[Kartikaya Gupta (email:[e-mail])]

This only happens with non-WR

Comment 6

[Kartikaya Gupta (email:[e-mail])]

So my impression of how this is supposed to work is that gecko sets a transform on the draw target, then requests core graphics to draw the scroll thumb, and the scroll thumb automatically gets scaled by the transform on the draw target. But it's not clear to me how that transform is actually taken into account.

I see the scale transform is on the draw target when we create the gfxQuartzNativeDrawing here but then to get the CGContextRef from that, the BeginNativeDrawing function runs this code which takes the raw data buffer from the skia draw target and wraps it in a CG bitmap context. So when CG draws, it bypasses the transform completely. The confusing part is how any of other widgets manage to work.

Comment 7

[Kartikaya Gupta (email:[e-mail])]

Incidentally this also happens if you set a transform: scale(3) on the element. Digging into the code a bit more it seems like most in-content widgetry get drawn via an NSCell codepath (e.g. DrawCellWithScaling) but the scrollbars and some other things get drawn via a CUIDraw codepath. Maybe that makes a difference, in that the core UI drawing API just always draws things at a fixed size.

I also tried setting a scale transform on the CGContext but that doesn't seem to have the desired effect. The position of the scrollbar gets scaled but the size does not, which is quite odd. I might have to get core UI to draw it into a separate CGContext and then copy that back into the target CG context with a scale factor. That will result in blurry scrollbars but I can't think of other options, unless we draw them ourselves without core UI.

Comment 8

[Markus Stange [:mstange]]

Interesting finds! We are planning to draw scrollbars without CoreUI anyways, so I can just make a patch to do that here.

Comment 9

[Kartikaya Gupta (email:[e-mail])]

The existing "custom" scrollthumb path here seems to do scaling (which makes sense, since it's drawing primitives into the CGContext). So forcing all scrollthumbs down that path is probably the simplest path to success. Just need to set the right colors on the scrollbar parameters, probably.

Comment 10

[Kartikaya Gupta (email:[e-mail])]

Markus, do you plan on working on this in the next week or so? We'd like to fix it in the 83 nightly cycle since it's currently marked as a desktop zoom blocker. If not I have a small patch to force us into the custom scrollbar draw path and that should probably be fine as a short-term fix.

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=8dc62d2bb55d1ab354eed88c3edf179fb7f4e21d

Comment 11

[Markus Stange [:mstange]]

Let's land your patch first then. I am planning to work on it this week but let's reduce risk.

Comment 12

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1642526 - Force the macOS scrollthumbs to be drawn via the custom drawing codepath. r?mstange

The default drawing codepath requests the OS to draw it, but the OS seems to
ignore the scaling factor of the transform. So when drawing scrollbars after
APZ-zooming, the scrollthumbs appear abnormally thin. This patch forces us into
the custom drawing codepath which gets scaled properly.

Comment 13

[Kartikaya Gupta (email:[e-mail])]

Attachment: Bug 1642526 - Update fuzz numbers for existing reftests. r?mstange

The change perturbs the drawing of the scrollbar endcaps for a handful of
APZ-related reftests, mostly with WR enabled. This just updates the fuzz numbers
to match the new values.

Depends on D92677

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44eecd5815e1
Force the macOS scrollthumbs to be drawn via the custom drawing codepath. r=mstange
https://hg.mozilla.org/integration/autoland/rev/d73ecb0ee0c4
Update fuzz numbers for existing reftests. r=mstange

Comment 15

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/44eecd5815e1
https://hg.mozilla.org/mozilla-central/rev/d73ecb0ee0c4