Bug 198998 - [CoordinatedGraphics] support bounds origin
Summary: [CoordinatedGraphics] support bounds origin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 198475
  Show dependency treegraph
 
Reported: 2019-06-19 06:18 PDT by Philippe Normand
Modified: 2019-09-27 03:34 PDT (History)
15 users (show)

See Also:


Attachments
screenshot (231.95 KB, image/png)
2019-06-19 06:21 PDT, Philippe Normand
no flags Details
WIP (10.28 KB, patch)
2019-06-19 10:50 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
WIP (9.33 KB, patch)
2019-06-20 01:47 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (9.62 KB, patch)
2019-09-26 05:29 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Normand 2019-06-19 06:18:15 PDT
It seems that now, only fixed elements of the page scroll. The others remain stuck, I'll attach a screenshot.
Comment 1 Philippe Normand 2019-06-19 06:21:57 PDT
Created attachment 372458 [details]
screenshot

The header bar moves when scrolling (it should remain fixed on top of the page) while the rest of the page doesn't move (it should)
Comment 2 Philippe Normand 2019-06-19 06:23:00 PDT
Simon, would you have any hint? Reverting your patch fixes the regression.
Comment 3 Simon Fraser (smfr) 2019-06-19 09:16:47 PDT
Have GTK take GraphicsLayer boundsOrigin into account when positioning descendant layers.
Comment 4 Zan Dobersek 2019-06-19 10:50:12 PDT
Created attachment 372478 [details]
WIP
Comment 5 Simon Fraser (smfr) 2019-06-19 11:31:29 PDT
Comment on attachment 372478 [details]
WIP

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

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:919
> -        layerState.mainBackingStore->invalidate({ { }, IntSize { m_size } });
> +        layerState.mainBackingStore->invalidate({ IntPoint { m_boundsOrigin }, IntSize { m_size } });

Invalidation always needs to round out (expand). So enclosingIntRect()

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:926
> +        layerState.mainBackingStore->createTilesIfNeeded(transformedVisibleRect(), IntRect(m_boundsOrigin.x(), m_boundsOrigin.y(), m_size.width(), m_size.height()));

enclosingIntRect()?
Comment 6 Carlos Garcia Campos 2019-06-20 00:35:04 PDT
Is this GTK specific? Or coordinated graphics? isn't WPE affected?
Comment 7 Zan Dobersek 2019-06-20 01:13:14 PDT
Comment on attachment 372478 [details]
WIP

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

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:919
>> +        layerState.mainBackingStore->invalidate({ IntPoint { m_boundsOrigin }, IntSize { m_size } });
> 
> Invalidation always needs to round out (expand). So enclosingIntRect()

This actually has to go unchanged as it relates to the content rectangle, not the layer one.

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:926
>> +        layerState.mainBackingStore->createTilesIfNeeded(transformedVisibleRect(), IntRect(m_boundsOrigin.x(), m_boundsOrigin.y(), m_size.width(), m_size.height()));
> 
> enclosingIntRect()?

Same here.

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:1112
> -    m_layerTransform.setPosition(m_adjustedPosition);
> +    m_layerTransform.setPosition({ m_adjustedPosition.x() - m_boundsOrigin.x(), m_adjustedPosition.y() - m_boundsOrigin.y() });

Main requirement is to have the bounds origin properly complement the position value. This was a good start, but further changes are needed in computePixelAlignment().
Comment 8 Zan Dobersek 2019-06-20 01:47:56 PDT
Created attachment 372548 [details]
WIP
Comment 9 Adrian Perez 2019-06-20 02:20:32 PDT
(In reply to Zan Dobersek from comment #8)
> Created attachment 372548 [details]
> WIP

I have tried this patch locally and so far the few websites
that I tested so far (e.g. GitHub, Twitter igalia.com) are
working well \o/
Comment 10 Simon Fraser (smfr) 2019-06-20 08:27:28 PDT
I reverted most of the change that caused this, so page scrolling on trunk is probably OK again. If GTK ever wants accelerated overflow scroll, then it will need this patch.
Comment 11 Alice Mikhaylenko 2019-06-22 23:50:11 PDT
I don't know if it's related, but kinetic scrolling is broken in trunk. It jumps up a bit instead of triggering kinetic scrolling.
Comment 12 Zan Dobersek 2019-09-26 03:23:27 PDT
Repurposing.
Comment 13 Zan Dobersek 2019-09-26 05:29:29 PDT
Created attachment 379633 [details]
Patch
Comment 14 Zan Dobersek 2019-09-27 03:34:35 PDT
Comment on attachment 379633 [details]
Patch

Clearing flags on attachment: 379633

Committed r250418: <https://trac.webkit.org/changeset/250418>
Comment 15 Zan Dobersek 2019-09-27 03:34:40 PDT
All reviewed patches have been landed.  Closing bug.