Bug 189579 – Build error in ImageBufferCG when compiled with IOSurfacePool

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Frédéric Wang (:fredw)]

Applying attachment 349536 [details], I get the errors below on macOS and iOS ports, the unified file is:

#include "platform/graphics/cg/IOSurfacePool.cpp"
#include "platform/graphics/cg/ImageBufferCG.cpp"
#include "platform/graphics/cg/ImageBufferDataCG.cpp"
#include "platform/graphics/cg/ImageDecoderCG.cpp"
#include "platform/graphics/cg/IntPointCG.cpp"
#include "platform/graphics/cg/IntRectCG.cpp"
#include "platform/graphics/cg/IntSizeCG.cpp"
#include "platform/graphics/cg/NativeImageCG.cpp"

IOSurfacePool includes IOSurface.h a first time without defining IOSURFACE_CANVAS_BACKING_STORE
ImageBufferCG includes IOSurface.h again but defining IOSURFACE_CANVAS_BACKING_STORE (this second include is ignored because of the #pragma once directive)

This is causing IOSurface::createFromImageBuffer to become undefined:

In file included from /Users/fred/WebKit/WebKitBuild/Debug/DerivedSources/WebCore/unified-sources/UnifiedSource329.cpp:2:
./platform/graphics/cg/ImageBufferCG.cpp:305:48: error: no member named
      'createFromImageBuffer' in 'WebCore::IOSurface'
    return IOSurface::sinkIntoImage(IOSurface::createFromImageBuffer(WTF...
                                    ~~~~~~~~~~~^
./platform/graphics/cg/ImageBufferCG.cpp:348:71: error: no member named
      'createFromImageBuffer' in 'WebCore::IOSurface'
    RetainPtr<CGImageRef> image = IOSurface::sinkIntoImage(IOSurface::create...
                                                           ~~~~~~~~~~~^
2 errors generated.

[Frédéric Wang (:fredw)]

Created attachment 349660 [details]
Quick hack to work around the issue

[Frédéric Wang (:fredw)]

Created attachment 349673 [details]
Patch

A proposed solution. Maybe an alternative is to define USE_IOSURFACE_CANVAS_BACKING_STORE in Source/WTF/wtf/Platform.h?

[Maciej Stachowiak]

Comment on attachment 349673 [details]
Patch

Instead of this change, I think the right fix is to make sure all files have the same value for IOSURFACE_CANVAS_BACKING_STORE. If headers depend on an #ifdef and different files have it set differently, that is a potential problem even without unified builds. Not in this case since the only difference is a static member function, but if the define affected data members, then it would be real bad for different files to have different values.

[Frédéric Wang (:fredw)]

(In reply to Maciej Stachowiak from comment #3)
> Comment on attachment 349673 [details]
> Patch
> 
> Instead of this change, I think the right fix is to make sure all files have
> the same value for IOSURFACE_CANVAS_BACKING_STORE. If headers depend on an
> #ifdef and different files have it set differently, that is a potential
> problem even without unified builds. Not in this case since the only
> difference is a static member function, but if the define affected data
> members, then it would be real bad for different files to have different
> values.

Yeah, I was wondering that too. Does the alternative I propose in comment 2 make sense?

[Frédéric Wang (:fredw)]

Created attachment 349757 [details]
Patch (move definition to Platform.h)

[Frédéric Wang (:fredw)]

cc'ing Simon and Matthew since it looks like the definition was added in bug 58084.

[Tim Horton]

Comment on attachment 349757 [details]
Patch (move definition to Platform.h)

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

> Source/WebCore/platform/graphics/cocoa/IOSurface.h:43
> +#if USE(IOSURFACE_CANVAS_BACKING_STORE)

No real need for the #ifs here but they're fine too.

[WebKit Commit Bot]

Comment on attachment 349757 [details]
Patch (move definition to Platform.h)

Clearing flags on attachment: 349757

Committed r236074: <https://trac.webkit.org/changeset/236074>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44527313>