RESOLVED DUPLICATE of bug 70634 40629
GraphicsLayer: Allow more layers to be opaque when applicable
https://bugs.webkit.org/show_bug.cgi?id=40629
Summary GraphicsLayer: Allow more layers to be opaque when applicable
Noam Rosenthal
Reported 2010-06-15 10:02:50 PDT
This would include layers with a 100% solid background color.
Attachments
Patch (6.13 KB, patch)
2010-06-30 23:02 PDT, Sam Magnuson
no flags
Patch (6.02 KB, patch)
2010-06-30 23:15 PDT, Sam Magnuson
simon.fraser: review-
Simon Fraser (smfr)
Comment 1 2010-06-15 10:31:12 PDT
Noam Rosenthal
Comment 2 2010-06-28 14:26:55 PDT
*** Bug 40380 has been marked as a duplicate of this bug. ***
Sam Magnuson
Comment 3 2010-06-30 23:02:54 PDT
Early Warning System Bot
Comment 4 2010-06-30 23:09:12 PDT
Sam Magnuson
Comment 5 2010-06-30 23:15:59 PDT
Noam Rosenthal
Comment 6 2010-07-01 08:39:01 PDT
The Qt-specific content LGTM!
Noam Rosenthal
Comment 7 2010-07-01 09:14:00 PDT
question - if we use contentsRect() for the texture instead of size(), can't we set the contents to opaque if the bgcolor is opaque, regardless of the layer size? I'm not really sure how contentsRect is supposed to work, but if it corresponds to the element size, this should be feasible and would also save us some texture-memory.
Noam Rosenthal
Comment 8 2010-07-04 20:50:33 PDT
ignore my last comment; contentsRect is only for directly composited image/media.
Simon Fraser (smfr)
Comment 9 2010-07-06 13:08:30 PDT
Comment on attachment 60201 [details] Patch > 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.
Noam Rosenthal
Comment 10 2010-08-14 08:13:25 PDT
This one is still applicable even though it's with GraphicsLayerQt, though it doesn't give us a huge performance benefit.
Jesus Sanchez-Palencia
Comment 11 2011-12-01 12:08:28 PST
Noam, Igor, is this a duplicate of https://bugs.webkit.org/show_bug.cgi?id=70634 ? Thanks!
Simon Fraser (smfr)
Comment 12 2011-12-01 12:09:28 PST
Yep. *** This bug has been marked as a duplicate of bug 70634 ***
Note You need to log in before you can comment on or make changes to this bug.