Bug 62908

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:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
proposed patch	darin: review+
proposed patch	none

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.