1618432 - Move the scrollbars during the dynamic toolbar moving for WebRender

Status: RESOLVED FIXED

(Core :: Panning and Zooming, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

The issue Botond found in bug 1598978 comment 4 is specific for WebRender. For non-WebRender we do it already.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1618432 - Move the root horizontal scrollbar in response to the dynamic toolbar transitions. r?botond

Comment 2

[Hiroyuki Ikezoe (:hiro)]

I'd think I am on the right track but still the patch I wrote doesn't work properly.

It looks that the horizontal scrollbar container is moving properly (i.e. its transform value seems to be applied properly), but it visually clipped by something.

Botond, can you please point out me what I am still missing?

Comment 3

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

Haven't looked at the patch yet but the clipping might be the same as what I filed in https://github.com/mozilla-mobile/fenix/issues/8784

Comment 4

[Botond Ballo [:botond]]

I had a look at the patch and suggested a way we could simplify it. I'm not sure if that will fix the clipping issue, but I suppose it's worth a try.

Comment 5

[Hiroyuki Ikezoe (:hiro)]

I should have explained what I did. Actually I did initially try to move the scroll thumb instead of the container, the result was also clipped, so I started thinking, oh maybe this is just a thumb, the container must be clipping the thumb area, but...

Anyways, I was thinking the clipping issue kats mentioned is a different issue, but yeah it might be caused by the same real issue. I will dig into detail. Thanks for the feedbacks by both of you!

Comment 6

[Botond Ballo [:botond]]

Yeah, that's a good point. With non-WR the container does in fact clip the translated thumb, and we work around it by expanding the container's clip here.

It probably makes sense to investigate the existing clipping issue and get the thumb rendering, and then consider my suggestion as a follow-up if it doesn't break things.

Comment 7

[Hiroyuki Ikezoe (:hiro)]

What I've noticed/found so far;

1. The clip in question is applied in AppendScrollPartsTo
2. With expanding the clip the patch (D64726) works properly
3. Even with expanding the clip, if we translate the scroll thumb (instead of the scroll container), the thumb is still clipped (probably it's clipped by the scroll container)
4. The clip was introduced in bug 1050096 to fix some issues on non WebRender

So, there are two options to fix this issue, one is to expand the clip on WebRender, the other is to change the clip dynamically.

To do the latter, in my understandings, we need to make clip animatable in WebRender just like transforms by APZ. This sounds a tough work at least for me.

If expanding the clip doesn't cause side effects, I'd prefer it.

There may be other options though.

Comment 8

[Hiroyuki Ikezoe (:hiro)]

Pushed the updated patch which including the expanding code.

Comment 9

[Hiroyuki Ikezoe (:hiro)]

Oops, I forgot about mentioning the other clipping issue kats noticed.

Though I haven't found the clip for the issue, but it seems a different issue. There seems places that we don't factor zoom scale into.

Comment 10

[Hiroyuki Ikezoe (:hiro)]

(In reply to Hiroyuki Ikezoe (:hiro) from comment #9)

Oops, I forgot about mentioning the other clipping issue kats noticed.

Though I haven't found the clip for the issue, but it seems a different issue. There seems places that we don't factor zoom scale into.

Filed bug 1619169 for this issue.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7590e9379bd
Move the root horizontal scrollbar in response to the dynamic toolbar transitions. r=botond

Comment 12

[Dorel Luca [:dluca]]

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