Bug 207048 – Allow different back-ends for ImageBuffer

• Component: Canvas
• Status: RESOLVED FIXED

[Said Abou-Hallawa]

This will decouple the platform specifics and the back-end details from the interface "ImageBuffer". In following patches new types of ImageBuffers backed by new ImageBufferBackends will be added. These new ImageBufferBackends will implement DisplayList drawing and synchronize remote rendering.

[Said Abou-Hallawa]

Created attachment 389356 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389357 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389366 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389414 [details]
Patch

[Said Abou-Hallawa]

Created attachment 389449 [details]
Patch  for review

[Tim Horton]

Comment on attachment 389449 [details]
Patch  for review

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

I think this is fine, except the name.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:834
> -		2DDE1D081F574D0E00D1A365 /* JSVRStageParameters.h in Headers */ = {isa = PBXBuildFile; fileRef = 2DDE1CF61F574BFF00D1A365 /* JSVRStageParameters.h */; };
> +		2DDE1CE31F574AE500D1A365 /* (null) in Headers */ = {isa = PBXBuildFile; };

This all doesn't look good

> Source/WebCore/platform/graphics/ContextualImageBuffer.h:33
> +class ContextualImageBuffer : public ImageBuffer {

I'm not sure I follow the "Contextual" name (but maybe you can explain)

(also, I'm not sure why it exists? Why isn't this just all IN ImageBuffer?)

(reading more... is it so you can hide BackendType from ImageBuffer clients? why not just `using` it away?)

> Source/WebCore/platform/graphics/ContextualImageBuffer.h:145
> +        const_cast<ContextualImageBuffer&>(*this).flushContext();

Maybe we should just bite the bullet and de-const getImageData if it's a lie?

[Said Abou-Hallawa]

Created attachment 391061 [details]
Patch

[Said Abou-Hallawa]

Created attachment 391063 [details]
Patch

[Said Abou-Hallawa]

Comment on attachment 389449 [details]
Patch  for review

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

>> Source/WebCore/WebCore.xcodeproj/project.pbxproj:834
>> +		2DDE1CE31F574AE500D1A365 /* (null) in Headers */ = {isa = PBXBuildFile; };
> 
> This all doesn't look good

This was fixed in the latest patch.

>> Source/WebCore/platform/graphics/ContextualImageBuffer.h:33
>> +class ContextualImageBuffer : public ImageBuffer {
> 
> I'm not sure I follow the "Contextual" name (but maybe you can explain)
> 
> (also, I'm not sure why it exists? Why isn't this just all IN ImageBuffer?)
> 
> (reading more... is it so you can hide BackendType from ImageBuffer clients? why not just `using` it away?)

Contextual here means an ImageBuffer which can provide a GraphicsContext. ContextualImageBuffer is a template class which is derived from ImageBuffer. All the callers can still keep a. pointer to an ImageBuffer which represents the interface of all the ContextualImageBuffers. It is also a bridge design pattern. As you said it hides the backend type from the callers. The creator can choose the backend type but caller which receives an std::unique_ptr<ImageBuffer> does not have to know any details about  this backend.

Other names: BackendedImageBuffer, ImageBufferWithBackend, PhysicalImageBuffer, ConcreteImageBuffer, ImageBufferImpl, ImageBufferBridge..

>> Source/WebCore/platform/graphics/ContextualImageBuffer.h:145
>> +        const_cast<ContextualImageBuffer&>(*this).flushContext();
> 
> Maybe we should just bite the bullet and de-const getImageData if it's a lie?

I think it is okay to keep the function const. flushContext() is an internal function which does nothing expect in the case of IOSurface backed ended ImageBuffer and the remote ImageBuffer.

[Said Abou-Hallawa]

Created attachment 391110 [details]
Patch

[Said Abou-Hallawa]

Created attachment 391114 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 391114 [details]
Patch

Clearing flags on attachment: 391114

Committed r256892: <https://trac.webkit.org/changeset/256892>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/59573099>