Bug 175376 – [CoordGraphics] Track dirty rects that need update in CoordinatedGraphicsLayer

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

[CoordGraphics] Track dirty rects that need update in CoordinatedGraphicsLayer

[Zan Dobersek]

Created attachment 317694 [details]
WIP

[Zan Dobersek]

Created attachment 336128 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:561:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 3 files


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

[Carlos Garcia Campos]

Comment on attachment 336128 [details]
Patch

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

> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:220
> +        bool fullRect { false };

I find this name a bit confusing, I've had to read the patch several times to understand it. I guess it means the full rect is dirty, so we do nothing when it's false and dirty rects is empty right? I think that instead of using a boolean for this we could make the dirty rects vector std::optional. What do you think?

[Zan Dobersek]

Comment on attachment 336128 [details]
Patch

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

>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:220
>> +        bool fullRect { false };
> 
> I find this name a bit confusing, I've had to read the patch several times to understand it. I guess it means the full rect is dirty, so we do nothing when it's false and dirty rects is empty right? I think that instead of using a boolean for this we could make the dirty rects vector std::optional. What do you think?

In addition to doing nothing when this is false and no other dirty rect was registered, in setNeedsDisplay() this is set to true, and any recorded rect is dropped, with the intention of redrawing the whole layer during the flush. After this is set to true, any other call to setNeedsDisplay() or setNeedsDisplayInRect() is ignored, because we don't have to bother recording anything anymore if we know the layer will be redrawn in full.

Corresponding to that logic, during flush the whole backing store is invalidated if fullRect is true, otherwise we do invalidation rect-by-rect.

[Zan Dobersek]

Comment on attachment 336128 [details]
Patch

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

>>> Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.h:220
>>> +        bool fullRect { false };
>> 
>> I find this name a bit confusing, I've had to read the patch several times to understand it. I guess it means the full rect is dirty, so we do nothing when it's false and dirty rects is empty right? I think that instead of using a boolean for this we could make the dirty rects vector std::optional. What do you think?
> 
> In addition to doing nothing when this is false and no other dirty rect was registered, in setNeedsDisplay() this is set to true, and any recorded rect is dropped, with the intention of redrawing the whole layer during the flush. After this is set to true, any other call to setNeedsDisplay() or setNeedsDisplayInRect() is ignored, because we don't have to bother recording anything anymore if we know the layer will be redrawn in full.
> 
> Corresponding to that logic, during flush the whole backing store is invalidated if fullRect is true, otherwise we do invalidation rect-by-rect.

I can find a better name though.

[Zan Dobersek]

Created attachment 336187 [details]
Patch

[EWS Watchlist]

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


ERROR: Source/WebCore/platform/graphics/texmap/coordinated/CoordinatedGraphicsLayer.cpp:561:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 1 in 3 files


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

[Zan Dobersek]

Comment on attachment 336187 [details]
Patch

Clearing flags on attachment: 336187

Committed r229804: <https://trac.webkit.org/changeset/229804>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/38706952>