WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(6.02 KB, patch)
2010-06-30 23:15 PDT
,
Sam Magnuson
simon.fraser
: review-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2010-06-15 10:31:12 PDT
<
rdar://problem/5698198
>
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
Created
attachment 60200
[details]
Patch
Early Warning System Bot
Comment 4
2010-06-30 23:09:12 PDT
Attachment 60200
[details]
did not build on qt: Build output:
http://webkit-commit-queue.appspot.com/results/3370100
Sam Magnuson
Comment 5
2010-06-30 23:15:59 PDT
Created
attachment 60201
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug