RESOLVED FIXED 114275
[BlackBerry] Accelerated compositing debug rectangle incorrectly drawn for layers with surfaces
https://bugs.webkit.org/show_bug.cgi?id=114275
Summary [BlackBerry] Accelerated compositing debug rectangle incorrectly drawn for la...
Arvid Nilsson
Reported 2013-04-09 06:11:09 PDT
PR 323746
Attachments
Patch (5.58 KB, patch)
2013-04-11 04:02 PDT, Arvid Nilsson
no flags
Patch (5.51 KB, patch)
2013-04-11 04:18 PDT, Arvid Nilsson
no flags
Arvid Nilsson
Comment 1 2013-04-11 04:02:14 PDT
Carlos Garcia Campos
Comment 2 2013-04-11 04:10:11 PDT
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.
Arvid Nilsson
Comment 3 2013-04-11 04:13:07 PDT
(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.
Arvid Nilsson
Comment 4 2013-04-11 04:18:38 PDT
Arvid Nilsson
Comment 5 2013-04-11 04:21:40 PDT
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 =)
Arvid Nilsson
Comment 6 2013-04-11 04:23:42 PDT
(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.
Arvid Nilsson
Comment 7 2013-04-11 06:54:28 PDT
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.
WebKit Commit Bot
Comment 8 2013-04-11 07:26:33 PDT
Comment on attachment 197572 [details] Patch Clearing flags on attachment: 197572 Committed r148200: <http://trac.webkit.org/changeset/148200>
WebKit Commit Bot
Comment 9 2013-04-11 07:26:35 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.