[webkit-reviews] review granted: [Bug 179967] [CoordGraphics] Replace CoordinatedSurface, ThreadSafeCoordinatedSurface with CoordinatedBuffer : [Attachment 327485] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Zan Dobersek
<zan at falconsigh.net>'s request for review:
Bug 179967: [CoordGraphics] Replace CoordinatedSurface,
ThreadSafeCoordinatedSurface with CoordinatedBuffer
https://bugs.webkit.org/show_bug.cgi?id=179967

Attachment 327485: Patch

https://bugs.webkit.org/attachment.cgi?id=327485&action=review




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 327485
  --> https://bugs.webkit.org/attachment.cgi?id=327485
Patch

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

> Source/WebKit/ChangeLog:13
> +
> +	   
> +

Remove extra empty lines here, please.

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp:53
> +void CoordinatedBuffer::paintToSurface(const IntRect& rect, Client& client)

I'm confused about this, what surface is this painting to? Maybe we could
rename it to paintToBuffer now, or simply paint() since buffer is this, no?

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedBuffer.cpp:59
> +    client.paintToSurfaceContext(context);

I would also rename this to paintToContext()

> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp:105
>      UpdateAtlasSurfaceClient surfaceClient(client, size, supportsAlpha());
> -    m_surface->paintToSurface(rect, surfaceClient);
> +    m_buffer->paintToSurface(rect, surfaceClient);

Wow, this CoordinatedBuffer::Client is a bit messy. Here we have
UpdateAtlasSurfaceClient which is a CoordinatedBuffer::Client but receives
another CoordinatedBuffer::Client as parameter. Could we avoid this client mess
by simply using lambdas to render into the context? CoordinatedBuffer::paint()
could receive a Function <void (GraphicsContext&)>. I'm not suggesting to do
that in this patch, just wondering if that would be possible.