Bug 197836 – Event region computation should respect transforms

• Status: RESOLVED
• Resolution: FIXED
• Component: Scrolling

[Antti Koivisto]

If transformed layers share a compositing layer we miscompute the event region.

[Radar WebKit Bug Importer]

<rdar://problem/50762971>

[Antti Koivisto]

Created attachment 369836 [details]
patch

[Darin Adler]

Comment on attachment 369836 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=369836&action=review

> Source/WebCore/ChangeLog:12
> +        Add support for transforming regions. Non-rectlinear results use enclosing rects.

Is it acceptable to just use enclosing rects?

> Source/WebCore/rendering/EventRegion.h:41
> +    EventRegionContext(EventRegion&);

We want "explicit" here since we wouldn’t want this to be done as a conversion.

> Source/WebCore/rendering/EventRegion.h:58
> +    EventRegionContext makeContext() { return { *this }; }

Not sure why we have this since EventRegionContext also has a public constructor. And it’s only used in one place. I think we could do without it. Not sure, it does seem a little more elegant than invoking a constructor.

> Source/WebCore/rendering/RenderLayer.cpp:4603
> +    auto oldTransfrom = context.getCTM();

Spelling error here: "transfrom".

[Antti Koivisto]

> Is it acceptable to just use enclosing rects?

Event region is used to synchronously determine on UI process side whether swipes should scroll or not. In this use approximation is often unnoticeable and definitely better than getting it completely wrong.

If needed this can be improved later, either by approximating with more rects or by using different data structures (bitmap or multiple Regions with attached transforms).

> Not sure why we have this since EventRegionContext also has a public
> constructor. And it’s only used in one place. I think we could do without
> it. Not sure, it does seem a little more elegant than invoking a constructor.

I like construction function like this because then the client site does not need to spell out uninteresting types:

auto eventRegionContext = eventRegion.makeContext();

[Antti Koivisto]

Created attachment 369867 [details]
patch

[WebKit Commit Bot]

Comment on attachment 369867 [details]
patch

Clearing flags on attachment: 369867

Committed r245293: <https://trac.webkit.org/changeset/245293>