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
Patch (1.44 KB, patch)
2014-03-27 12:27 PDT, Adenilson Cavalcanti Silva
no flags
Adenilson Cavalcanti Silva
Comment 1 2014-03-27 11:02:40 PDT
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
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.