[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