Closed Bug 1531170 Opened 8 months ago Closed 7 months ago

Can't scroll on https://codyhouse.co/ds/components/app/smooth-scrolling (APZ hit testing broken?)

Categories

(Core :: Graphics: WebRender, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox65 --- unaffected
firefox66 --- unaffected
firefox67 --- fixed

People

(Reporter: mstange, Assigned: gw)

References

Details

(Keywords: regression)

Attachments

(3 files)

Scrolling the content frame on https://codyhouse.co/ds/components/app/smooth-scrolling does not work on my macOS Firefox Nightly build with WebRender enabled.

This is a regression from bug 1524385.

 7:14.94 INFO: Last good revision: 8c235a9b55a2b2d89115a7cc0f56a58c8883a980
 7:14.94 INFO: First bad revision: 2853480ed90d0a995593e66fcd68a8bff1ad8d96
 7:14.94 INFO: Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=8c235a9b55a2b2d89115a7cc0f56a58c8883a980&tochange=2853480ed90d0a995593e66fcd68a8bff1ad8d96

(Don't be distracted by the "smooth-scrolling" part in the URL - this page does not override scrolling. This bug can also be observed on other example pages such as https://codyhouse.co/ds/components/app/text-media that have nothing to do with scrolling - I just picked the "smooth-scrolling" one because it has the most text and reliably causes scrollbars.)

Summary: Can't scroll on https://codyhouse.co/ds/components/app/text-media (APZ hit testing broken?) → Can't scroll on https://codyhouse.co/ds/components/app/smooth-scrolling (APZ hit testing broken?)

I think the clip:rect(1px,1px,1px,1px); property on the .modal div isn't being respected during hit-testing somehow, so hit-testing ends up hitting the modal that's sitting on top of the content.

If I set background-color:green on the .modal__body it is correctly clipped away, so the visual rendering is correctly accounting for that clip. Seems like it's just the hit-testing that's not.

Glenn, per description, this is a regression from bug 1524385. Can you please look into this for the upcoming 67 soft freeze (Mar 11)?

Flags: needinfo?(gwatson)

Sure, I'll investigate this today.

Flags: needinfo?(gwatson)
Assignee: nobody → gwatson
Attached file scene-1-0.ron

I loaded the reduced test case, entered full screen mode (to reduce the size of the capture) and captured the attached scene.ron file.

I think this may indicate that the problem is in the display list itself, but I'm not certain yet.

If we look at the Clip/ClipChain with id (6, (1, 2)), that has a clip rect at (1,1) with size (0,0), which is the relevant clip we are concerned with.

Looking at item [43] in the ron file, it is a hit testing rectangle with clip_id: ClipChain((6, (1, 2))) and hit testing tag Some((2, 1)). This does appear to result in this item failing the hit test.

However, directly after that, item [44] is another hit testing rectangle with clip_id: Clip(0, (1, 2)) and hit testing tag Some((2, 1)). Since this doesn't contain the 0x0 clip rect, it gets detected as a valid hit test and returned.

I suspect that [44] shouldn't be present in the display list, and is causing the bug. kats, does this sound right to you? I'll keep digging a bit further in the meantime.

Flags: needinfo?(kats)

(In reply to Glenn Watson [:gw] from comment #7)

If we look at the Clip/ClipChain with id (6, (1, 2)), that has a clip rect at (1,1) with size (0,0), which is the relevant clip we are concerned with.

Agreed

Looking at item [43] in the ron file, it is a hit testing rectangle with clip_id: ClipChain((6, (1, 2))) and hit testing tag Some((2, 1)). This does appear to result in this item failing the hit test.

However, directly after that, item [44] is another hit testing rectangle with clip_id: Clip(0, (1, 2)) and hit testing tag Some((2, 1)). Since this doesn't contain the 0x0 clip rect, it gets detected as a valid hit test and returned.

But both of these items are inside a stacking context (item [42]) which has a clip_id: Clip((6, (1, 2))). So I would expect that everything inside the stacking context (which includes both items 43 and 44) gets clipped by this. Or am I misunderstanding?

Flags: needinfo?(kats)

For reference, items 43 and 44 are generated from CompositorHitTestInfo items like so:

      CompositorHitTestInfo p=0x112f96120 f=0x112f2a020(HTMLScroll(div)(1) class:modal__content) key=28 bounds(0,0,0,0) layerBounds(0,0,0,0) visible(0,0,76800,44220) building(0,0,76800,44220) componentAlpha(0,0,0,0) clip() asr() clipChain() ref=0x112db6020 agr=0x112f2a020 hitTestInfo(0x1) hitTestArea(0,0,76800,44220)
        GenericDisplayItem { item: Rectangle(RectangleDisplayItem { color: ColorF { r: 0.0, g: 0.0, b: 0.0, a: 0.0 } }), layout: PrimitiveInfo { rect: TypedRect(2560.0×1474.0 at (0.0,0.0)), clip_rect: TypedRect(2560.0×1474.0 at (0.0,0.0)), is_backface_visible: true, tag: Some((2, 1)) }, space_and_clip: SpaceAndClipInfo { spatial_id: SpatialId(1, PipelineId(1, 2)), clip_id: ClipChain(ClipChainId(6, PipelineId(1, 2))) } }
      CompositorHitTestInfo p=0x112f96020 f=0x112f2a9f0(HTMLScroll(div)(1) class:modal__body) key=28 bounds(0,0,0,0) layerBounds(0,0,0,0) visible(0,0,76800,44220) building(0,0,76800,44220) componentAlpha(0,0,0,0) clip(0,0,76800,44220) asr() clipChain(0x112f96220 <0,0,76800,44220> [root asr]) ref=0x112db6020 agr=0x112f2a020 hitTestInfo(0x1) hitTestArea(0,0,76800,44220)
        GenericDisplayItem { item: Rectangle(RectangleDisplayItem { color: ColorF { r: 0.0, g: 0.0, b: 0.0, a: 0.0 } }), layout: PrimitiveInfo { rect: TypedRect(2560.0×1474.0 at (0.0,0.0)), clip_rect: TypedRect(2560.0×1474.0 at (0.0,0.0)), is_backface_visible: true, tag: Some((2, 1)) }, space_and_clip: SpaceAndClipInfo { spatial_id: SpatialId(1, PipelineId(1, 2)), clip_id: Clip(0, PipelineId(1, 2)) } }

The modal__body div (item 44) has a separate clip set on it which is why it doesn't get the same clip as item 43. But I still think the enclosing stacking context should be clipping them both.

Recently, semantics for clips on stacking contexts were changed
such that primitives inherit the clip chain from all enclosing
stacking contexts. However, the hit testing code was not updated
to handle this change.

As each hit testing primitive is added, the current stack of
active stacking contexts is now scanned. Any valid clip chain
roots from the primitive and/or the stacking context stack are
added to the list of clip chain roots for this hit testing
primitive.

This patch also applies some optimizations and other cleanups
of the hit-testing code, specifically:

  • Instead of cloning the hit testing runs Vec every time a
    frame is built, store these in the new HitTestingScene. The
    HitTestingScene is built once per scene, and then shared by
    any hit tester instances via an Arc. This reduces a lot of
    memory allocations and copying during scrolling.
  • When creating a new HitTestingScene, pre-allocate the size
    of the arrays, based on the size of the previous hit testing
    structure. This works similarly to how arrays are sized
    in the PrimitiveStore.
  • Pre-calculate and cache a number of inverse transform matrices
    that were previously being calculated for each hit testing run.
  • Store hit testing primitives in a flat array, instead of runs,
    since there is no longer a single clip chain id per primitive.
  • Fix an apparent (?) bug in the existing hit testing code, where
    clipping out a single hit test primitive would break out of the
    loop for the current run of hit test items.
Pushed by gwatson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/96e78962a053
Fix WR hit testing breakage caused by stacking context clip changes. r=kvark
Status: NEW → RESOLVED
Closed: 7 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
You need to log in before you can comment on or make changes to this bug.