Bug 180141 - [CoordGraphics] Move CoordinatedGraphicsLayer painting behind Nicosia::PaintingEngine
Summary: [CoordGraphics] Move CoordinatedGraphicsLayer painting behind Nicosia::Painti...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-29 08:13 PST by Zan Dobersek
Modified: 2017-11-30 07:14 PST (History)
4 users (show)

See Also:


Attachments
Patch (20.27 KB, patch)
2017-11-29 08:38 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch for landing (20.23 KB, patch)
2017-11-30 06:32 PST, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2017-11-29 08:13:50 PST
[CoordGraphics] Move CoordinatedGraphicsLayer painting behind Nicosia::PaintingEngine
Comment 1 Zan Dobersek 2017-11-29 08:38:00 PST
Created attachment 327856 [details]
Patch
Comment 2 EWS Watchlist 2017-11-29 08:40:14 PST
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.
Comment 3 Carlos Garcia Campos 2017-11-29 22:43:55 PST
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
Comment 4 Zan Dobersek 2017-11-30 06:24:05 PST
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.
Comment 5 Zan Dobersek 2017-11-30 06:32:24 PST
Created attachment 327973 [details]
Patch for landing
Comment 6 EWS Watchlist 2017-11-30 06:33:31 PST
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.
Comment 7 Zan Dobersek 2017-11-30 07:13:01 PST
Comment on attachment 327973 [details]
Patch for landing

Clearing flags on attachment: 327973

Committed r225328: <https://trac.webkit.org/changeset/225328>
Comment 8 Zan Dobersek 2017-11-30 07:13:05 PST
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-11-30 07:14:22 PST
<rdar://problem/35771883>