[webkit-reviews] review granted: [Bug 24999] Optimize hit testing with transforms : [Attachment 29191] Revised patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Apr 2 09:16:48 PDT 2009

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 24999: Optimize hit testing with transforms

Attachment 29191: Revised patch

------- Additional Comments from Darin Adler <darin at apple.com>
> -    void move(int x, int y);
> +    void move(int x, int y);    // always flattens

The formatting here is slightly unconventional. We typically use a single space
in cases like this rather than using, say, 4 spaces.

> +    void translate(int x, int y, bool accumulateTransform);

I am uncomfortable with boolean arguments in C++ functions. They're hard to
read at the call site. The "true" doesn't help me understand what the function
is doing. Further, the only caller of this function is passing true. Do we
really need the boolean argument? If the version that flattens is needed, you
could consider the "named enum" design pattern or having two separately named

> +    void flattenWithTransform(const TransformationMatrix& t);

No need to include a name for the argument "t" here.


More information about the webkit-reviews mailing list