WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Patch
(5.51 KB, patch)
2013-04-11 04:18 PDT
,
Arvid Nilsson
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Arvid Nilsson
Comment 1
2013-04-11 04:02:14 PDT
Created
attachment 197569
[details]
Patch
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
Created
attachment 197572
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug