Summary: | [CoordGraphics] Track dirty rects that need update in CoordinatedGraphicsLayer | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cgarcia, cmarcelo, ews-watchlist, kondapallykalyan, luiz, noam, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Zan Dobersek
2017-08-09 04:18:16 PDT
Created attachment 317694 [details]
WIP
Created attachment 336128 [details]
Patch
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.
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? 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. 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. Created attachment 336187 [details]
Patch
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.
Comment on attachment 336187 [details] Patch Clearing flags on attachment: 336187 Committed r229804: <https://trac.webkit.org/changeset/229804> All reviewed patches have been landed. Closing bug. |