Bug 62908 - Optimization: avoid call to clearRect() when bgColor is valid when painting the root background in RenderBoxModelObject::paintFillLayerExtended
Summary: Optimization: avoid call to clearRect() when bgColor is valid when painting t...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-06-17 14:08 PDT by Una Sabovic
Modified: 2011-06-19 13:48 PDT (History)
4 users (show)

See Also:

Attachments
proposed patch (4.84 KB, patch)
2011-06-17 14:46 PDT, Una Sabovic 		darin: review+
Details | Formatted Diff | Diff
proposed patch (4.88 KB, patch)
2011-06-18 21:29 PDT, Una Sabovic 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Una Sabovic 2011-06-17 14:08:18 PDT 
When root layers base color is fully transparent backgroundRect was cleared before bgColor is applied.
Instead of clearing the rect we apply CompositeCopy operation when painting the background color.
Comment 1 Una Sabovic 2011-06-17 14:46:03 PDT 
Created attachment 97655 [details]
proposed patch

Adjustment for https://bugs.webkit.org/show_bug.cgi?id=62593 per Darin's suggestion.
Comment 2 Darin Adler 2011-06-17 17:50:15 PDT 
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 3 Eric Seidel (no email) 2011-06-18 20:05:17 PDT 
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?
Comment 4 Eric Seidel (no email) 2011-06-18 20:05:52 PDT 
Una isn't a committer.  Should this be marked r- or cq+?
Comment 5 Una Sabovic 2011-06-18 21:29:29 PDT 
Created attachment 97715 [details]
proposed patch

Fixed variable names patch ready for cq now.
Comment 6 WebKit Review Bot 2011-06-19 13:48:29 PDT 
Comment on attachment 97715 [details]
proposed patch

Clearing flags on attachment: 97715

Committed r89221: <http://trac.webkit.org/changeset/89221>
Comment 7 WebKit Review Bot 2011-06-19 13:48:34 PDT 
All reviewed patches have been landed.  Closing bug.