Summary: | Refactor canvas drawing to be more data driven | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Tim Dresser <tdresser> | ||||||||
Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | cc-bugs, enne, jamesr, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Tim Dresser
2012-01-19 07:28:57 PST
Created attachment 123126 [details]
Patch
Comment on attachment 123126 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123126&action=review > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:687 > + if (!quad->hasAlpha()) { > + // Even though the WebGL layer's texture was likely allocated > + // as GL_RGB, disable blending anyway for better robustness. > + context()->disable(GraphicsContext3D::BLEND); > + } else { > + GC3Denum sfactor = quad->premultipliedAlpha() ? GraphicsContext3D::ONE : GraphicsContext3D::SRC_ALPHA; > + GLC(context(), context()->blendFunc(sfactor, GraphicsContext3D::ONE_MINUS_SRC_ALPHA)); > + } Enabling/disabling alpha is already done by the drawQuad routine above because all quads need that commonly, so no need to do that here. Can you also make sure blendFunc gets reset properly, since it only is currently set at the beginning of the frame? > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:695 > + if (!quad->hasAlpha()) > + context()->enable(GraphicsContext3D::BLEND); Not needed either for the above reason. > Source/WebCore/platform/graphics/chromium/cc/CCDrawQuad.h:53 > + const IntSize& bounds() const { return m_sharedQuadState->bounds(); } I don't think this function needs to be added to either CCDrawQuad or the shared quad state. Isn't bounds() just quad->quadRect().size()? Created attachment 123191 [details]
Patch
Comment on attachment 123191 [details]
Patch
Thanks! This looks good to me. jamesr?
Comment on attachment 123191 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=123191&action=review R=me, one nit > Source/WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:685 > + drawTexturedQuad(quad->layerTransform(), quad->quadRect().size().width(), quad->quadRect().size().height(), quad->opacity(), sharedGeometryQuad(), can I get a temporary const reference to size in this piece? this line's pretty long, even by webkit standards Created attachment 123316 [details]
Patch for landing
Comment on attachment 123316 [details] Patch for landing Rejecting attachment 123316 [details] from commit-queue. tdresser@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/committers.py. - If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags. - If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/committers.py by adding yourself to the file (no review needed). The commit-queue restarts itself every 2 hours. After restart the commit-queue will correctly respect your committer rights. Comment on attachment 123316 [details]
Patch for landing
Submitting to commit queue on behalf of Tim Dresser.
Comment on attachment 123316 [details] Patch for landing Clearing flags on attachment: 123316 Committed r105529: <http://trac.webkit.org/changeset/105529> All reviewed patches have been landed. Closing bug. |