Bug 180141 – [CoordGraphics] Move CoordinatedGraphicsLayer painting behind Nicosia::PaintingEngine

• Component: New Bugs
• Status: RESOLVED FIXED

[Zan Dobersek]

[CoordGraphics] Move CoordinatedGraphicsLayer painting behind Nicosia::PaintingEngine

[Zan Dobersek]

Created attachment 327856 [details]
Patch

[EWS Watchlist]

Attachment 327856 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:977:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Carlos Garcia Campos]

Comment on attachment 327856 [details]
Patch

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

> Source/WebCore/ChangeLog:25
> +        The Nicosia::PaintingEngine::create() function returns a new
> +        PaintingEngine object. For now it defaults to PaintingEngineBasic, but
> +        it could be configured at configure-time or even runtime in the future
> +        to return a more advanced engine implementation.

If possible I would make this configurable at runtime, I'm sure we will want to compare output between engines at some point.

> Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngine.h:44
> +class PaintingEngine {
> +public:

WTF_MAKE_FAST_ALLOCATED?

> Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngineBasic.cpp:62
> +    return true;

Why is it bool if it always returns true? Do you expect other implementations might fail somehow? If that's the case I would probably change it when needed eventually.

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:56
> +    virtual Nicosia::PaintingEngine& getPaintingEngine() = 0;

getPaintingEngine -> paintingEngine

[Zan Dobersek]

Comment on attachment 327856 [details]
Patch

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

>> Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngine.h:44
>> +public:
> 
> WTF_MAKE_FAST_ALLOCATED?

OK.

>> Source/WebCore/platform/graphics/nicosia/NicosiaPaintingEngineBasic.cpp:62
>> +    return true;
> 
> Why is it bool if it always returns true? Do you expect other implementations might fail somehow? If that's the case I would probably change it when needed eventually.

Yes, other implementations will possibly fail here, that's why it returns a boolean value already.

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:56
>> +    virtual Nicosia::PaintingEngine& getPaintingEngine() = 0;
> 
> getPaintingEngine -> paintingEngine

OK.

[Zan Dobersek]

Created attachment 327973 [details]
Patch for landing

[EWS Watchlist]

Attachment 327973 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:977:  Multi line control clauses should use braces.  [whitespace/braces] [4]
Total errors found: 1 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Zan Dobersek]

Comment on attachment 327973 [details]
Patch for landing

Clearing flags on attachment: 327973

Committed r225328: <https://trac.webkit.org/changeset/225328>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/35771883>