Summary: | Optimization: avoid call to clearRect() when bgColor is valid when painting the root background in RenderBoxModelObject::paintFillLayerExtended | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Una Sabovic <una.sabovic> | ||||||
Component: | Layout and Rendering | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin, eric, simon.fraser, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Una Sabovic
2011-06-17 14:08:18 PDT
Created attachment 97655 [details] proposed patch Adjustment for https://bugs.webkit.org/show_bug.cgi?id=62593 per Darin's suggestion. Comment on attachment 97655 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=97655&action=review > Source/WebCore/rendering/RenderBoxModelObject.cpp:745 > + bool clearRect = false; This local variable name sounds like “a rectangle that is clear” or a verb phrase that means “clear a rectangle”, but it’s actually a boolean flag that indicates whether we need to clear the background. I would name it shouldClearBackground or mustClearBackground. > Source/WebCore/rendering/RenderBoxModelObject.cpp:758 > + CompositeOperator compositeOp = clearRect ? CompositeCopy : context->compositeOperation(); I’d name this “operation” rather than “compositeOp”. Comment on attachment 97655 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=97655&action=review > Source/WebCore/ChangeLog:1 > +2011-06-17 una sabovic <una.sabovic@palm.com> Is your name normally written without capitals? Una isn't a committer. Should this be marked r- or cq+? Created attachment 97715 [details]
proposed patch
Fixed variable names patch ready for cq now.
Comment on attachment 97715 [details] proposed patch Clearing flags on attachment: 97715 Committed r89221: <http://trac.webkit.org/changeset/89221> All reviewed patches have been landed. Closing bug. |