Summary: | [BlackBerry] Accelerated compositing debug rectangle incorrectly drawn for layers with surfaces | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Arvid Nilsson <anilsson> | ||||||
Component: | WebKit BlackBerry | Assignee: | Arvid Nilsson <anilsson> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | anilsson, anlo, cgarcia, commit-queue, jpetsovits, mifenton, rwlbuis, tonikitoo, xan.lopez | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Arvid Nilsson
2013-04-09 06:11:09 PDT
Created attachment 197569 [details]
Patch
Comment on attachment 197569 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=197569&action=review Looks good. > Source/WebCore/platform/graphics/blackberry/LayerRenderer.cpp:598 > + // If we're rendering to a surface, don't include debug border inside the surface Finish comments with a period. ;-) > Source/WebCore/platform/graphics/blackberry/LayerRendererSurface.cpp:67 > +FloatQuad LayerRendererSurface::getTransformedBounds() const getFoo is typically used for methods that return the value as an out parameter. Sou you could either pass the FloatQuad as a reference and keep the name or rename it to transformedBounds(). I this case passing it as a reference we safe a copy, I think. (In reply to comment #2) > (From update of attachment 197569 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=197569&action=review > > Looks good. > > > Source/WebCore/platform/graphics/blackberry/LayerRenderer.cpp:598 > > + // If we're rendering to a surface, don't include debug border inside the surface > > Finish comments with a period. ;-) > > > Source/WebCore/platform/graphics/blackberry/LayerRendererSurface.cpp:67 > > +FloatQuad LayerRendererSurface::getTransformedBounds() const > > getFoo is typically used for methods that return the value as an out parameter. Sou you could either pass the FloatQuad as a reference and keep the name or rename it to transformedBounds(). I this case passing it as a reference we safe a copy, I think. I copied LayerCompositingThread::getTransformedBounds() which does return a reference and I guess thus deserves the "get" prefix. The LayerRendererSurface doesn't bother with caching the transformed bounds and can't return a reference. It's not worth caching them either, so let's just drop the "get" bit. Created attachment 197572 [details]
Patch
I was under the impression that WebKit coding style guidelines sometimes allows using the "get" prefix when some more computation is required than just returning the value of a data member, for example when a hash map lookup or other extra work is needed. Maybe that's just something I imagined =) (In reply to comment #5) > I was under the impression that WebKit coding style guidelines sometimes allows using the "get" prefix when some more computation is required than just returning the value of a data member, for example when a hash map lookup or other extra work is needed. Maybe that's just something I imagined =) That was just my imagination, the coding style guidelines only talks about using "get" when the method has out parameters. Comment on attachment 197572 [details]
Patch
Thanks for the review, I should have a look at the code here and see what other getters should have their "get" prefix removed.
Comment on attachment 197572 [details] Patch Clearing flags on attachment: 197572 Committed r148200: <http://trac.webkit.org/changeset/148200> All reviewed patches have been landed. Closing bug. |