[Webkit-unassigned] [Bug 40629] GraphicsLayer: Allow more layers to be opaque when applicable

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=40629


Simon Fraser (smfr) <simon.fraser at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #60201|review?                     |review-
               Flag|                            |




--- Comment #9 from Simon Fraser (smfr) <simon.fraser at apple.com>  2010-07-06 13:08:30 PST ---
(From update of attachment 60201)
> index d531bb5d0a0e29abdfe930ec7bf2166d3b55b422..fcee1962310652d679e6977f1b587b9581242dff 100644
> +        No new tests: this is an optimization.        

You can and should add tests that dump the layer tree (layoutTestController.layerTreeAsText()). You should also add a test or two to make sure that you don't hit this optimization when you don't intend to.

>  void GraphicsLayerQtImpl::paint(QPainter* painter, const QStyleOptionGraphicsItem* option, QWidget* widget)
>  {
> -    if (m_currentContent.backgroundColor.isValid())
> -        painter->fillRect(option->rect, QColor(m_currentContent.backgroundColor));
> +//     if (m_currentContent.backgroundColor.isValid())
> +//         painter->fillRect(option->rect, QColor(m_currentContent.backgroundColor));

Don't check in commented-out code.

> diff --git a/WebCore/rendering/RenderLayerBacking.cpp b/WebCore/rendering/RenderLayerBacking.cpp

> +    bool contentsOpaque = false;
> +    if (rendererHasBackground()) {
> +        const Color &bgColor = rendererBackgroundColor();

Color is sizeof(int), so no point in using a reference there.

> +        if (bgColor.alpha() == 255) {

Use !bgColor.hasAlpha().

> +            const IntRect elementBounds = m_owningLayer->localBoundingBox(),
> +                            layerBounds = compositedBounds();

We don't use multiple declarations like this. Also no need for the IntRects to be 'const'. Also elementBounds is a bad name; that's really layer bounds. So use layerBounds and compositedBounds or something.

> +            if (elementBounds.width() >= layerBounds.width() && elementBounds.height() >= layerBounds.height())
> +                contentsOpaque = true;

You should just test for rect equality. The composited bounds will never be smaller than m_owningLayer->localBoundingBox().

You should file a follow-up bug for other obvious enhancements (e.g. non-alpha background image).

r- for lack of tests.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.