Bug 179972 – [CoordGraphics] Simplify CoordinatedGraphicsLayer's content buffer updates

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

[CoordGraphics] Simplify CoordinatedGraphicsLayer's content buffer updates

[Zan Dobersek]

Created attachment 327494 [details]
Patch

[Carlos Garcia Campos]

Comment on attachment 327494 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.h:-56
> -    class Client {
> -    public:
> -        virtual ~Client() = default;
> -        virtual void paintToSurfaceContext(GraphicsContext&) = 0;
> -    };

\o/

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:-859
> -void CoordinatedGraphicsLayer::didUpdateTileBuffers()
> -{
> -    if (!isShowingRepaintCounter())
> -        return;
> -
> -    m_layerState.repaintCount = incrementRepaintCount();
> -    m_layerState.repaintCountChanged = true;
> -}

Why did you move this?

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:976
> +            if (!coordinatedBuffer)
> +                continue;

Dow we consider the tile to be still dirty in this case?

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h:33
> +#include <WebCore/CoordinatedBuffer.h>

Can we forward declare this?

[Zan Dobersek]

Comment on attachment 327494 [details]
Patch

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

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:-859
>> -}
> 
> Why did you move this?

Because it's also moved in the class declaration, and it's not an override of the TiledBackingStoreClient interface anymore, so it best fits with other didChange*() methods.

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:976
>> +                continue;
> 
> Dow we consider the tile to be still dirty in this case?

Yes. It might be possible to always retrieve a non-null CoordinatedBuffer simply because CoordinatedBuffer::create() in UpdateAtlas is a Ref<>. I'll look into it.

>> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/CompositingCoordinator.h:33
>> +#include <WebCore/CoordinatedBuffer.h>
> 
> Can we forward declare this?

No, because we need the CoordinatedBuffer::Flags type. This just includes the CoordinatedBuffer header explicitly, I assume it was until now included through UpdateAtlas.h or some other header.

[Zan Dobersek]

Comment on attachment 327494 [details]
Patch

Clearing flags on attachment: 327494

Committed r225115: <https://trac.webkit.org/changeset/225115>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35676071>