WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
130848
Remove comment from Filter.h
https://bugs.webkit.org/show_bug.cgi?id=130848
Summary
Remove comment from Filter.h
Adenilson Cavalcanti Silva
Reported
2014-03-27 10:55:11 PDT
As noticed in the TODO in #130779, it was possible to make the parameters in applyHorizontalScale()/VerticalScale() const refs if small changes were made on the derived classes (e.g. SVGFilter).
Attachments
Patch
(4.46 KB, patch)
2014-03-27 11:02 PDT
,
Adenilson Cavalcanti Silva
no flags
Details
Formatted Diff
Diff
Patch
(1.44 KB, patch)
2014-03-27 12:27 PDT
,
Adenilson Cavalcanti Silva
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Adenilson Cavalcanti Silva
Comment 1
2014-03-27 11:02:40 PDT
Created
attachment 227960
[details]
Patch
Dirk Schulze
Comment 2
2014-03-27 11:51:32 PDT
Comment on
attachment 227960
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=227960&action=review
Not sure why you are doing that. passing a reference to a float is not more or less expecive than passing the value itself. All the float references should be turned back to normal float arguments.
> Source/WebCore/platform/graphics/filters/Filter.h:50 > + virtual float applyHorizontalScale(const float& value) const { return value * m_filterResolution.width(); } > + virtual float applyVerticalScale(const float& value) const { return value * m_filterResolution.height(); }
there is no need to make the float a reference, it doesn't save anything.
> Source/WebCore/svg/graphics/filters/SVGFilter.cpp:39 > +float SVGFilter::applyHorizontalScale(const float& value) const
Ditto.
Adenilson Cavalcanti Silva
Comment 3
2014-03-27 11:56:17 PDT
Dirk Thanks for the review. I have one question: const float would be ok?
Simon Fraser (smfr)
Comment 4
2014-03-27 12:26:59 PDT
Comment on
attachment 227960
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=227960&action=review
>> Source/WebCore/platform/graphics/filters/Filter.h:50 >> + virtual float applyVerticalScale(const float& value) const { return value * m_filterResolution.height(); } > > there is no need to make the float a reference, it doesn't save anything.
Nor a const float. The const doesn't add anything.
Adenilson Cavalcanti Silva
Comment 5
2014-03-27 12:27:51 PDT
Created
attachment 227964
[details]
Patch
Adenilson Cavalcanti Silva
Comment 6
2014-03-27 12:35:33 PDT
Gentlemen Thanks for the review. I agree that it won't fly, so I'm just renaming the bug title and simply removing the TODO comment.
WebKit Commit Bot
Comment 7
2014-03-27 13:08:09 PDT
Comment on
attachment 227964
[details]
Patch Clearing flags on attachment: 227964 Committed
r166368
: <
http://trac.webkit.org/changeset/166368
>
WebKit Commit Bot
Comment 8
2014-03-27 13:08:17 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