[CoordGraphics] Replace CoordinatedSurface, ThreadSafeCoordinatedSurface with CoordinatedBuffer
Created attachment 327485 [details] Patch
Comment on attachment 327485 [details] 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.
Comment on attachment 327485 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=327485&action=review >> 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? I have this and other "paint to surface" methods removed in the stashed work. That's why I didn't bother renaming them. >> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp:105 >> + 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. This will also be simplified to the point where CoordinatedBuffer::Client will be possible to remove.
Committed r225108: <https://trac.webkit.org/changeset/225108>
<rdar://problem/35674166>
(In reply to Zan Dobersek from comment #3) > Comment on attachment 327485 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=327485&action=review > > >> 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? > > I have this and other "paint to surface" methods removed in the stashed > work. That's why I didn't bother renaming them. > > >> Source/WebKit/WebProcess/WebPage/CoordinatedGraphics/UpdateAtlas.cpp:105 > >> + 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. > > This will also be simplified to the point where CoordinatedBuffer::Client > will be possible to remove. \o/