[webkit-reviews] review denied: [Bug 53232] LightElement changes does not require relayout. : [Attachment 80310] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 27 02:41:22 PST 2011
Nikolas Zimmermann <zimmermann at kde.org> has denied Zoltan Herczeg
<zherczeg at webkit.org>'s request for review:
Bug 53232: LightElement changes does not require relayout.
https://bugs.webkit.org/show_bug.cgi?id=53232
Attachment 80310: patch
https://bugs.webkit.org/attachment.cgi?id=80310&action=review
------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=80310&action=review
> Source/WebCore/platform/graphics/filters/LightSource.h:86
> + virtual bool setAzimuth(float) { return false; }
> + virtual bool setElevation(float) { return false; }
> + virtual bool setX(float) { return false; }
> + virtual bool setY(float) { return false; }
> + virtual bool setZ(float) { return false; }
> + virtual bool setPointsAtX(float) { return false; }
> + virtual bool setPointsAtY(float) { return false; }
> + virtual bool setPointsAtZ(float) { return false; }
> + virtual bool setSpecularExponent(float) { return false; }
> + virtual bool setLimitingConeAngle(float) { return false; }
Do we really need all those virtual methods? Is there no way to find out wheter
a LightSource is eg. a PointLightSource, then cast to it? and call the setter
there, instead of having to declare a lot of virtual functions here?
The general concept of the patch seems fine, but I dislike the addition of that
many virtual methods.
Please look into another approach! Thanks in advance!
More information about the webkit-reviews
mailing list