[webkit-reviews] review granted: [Bug 34548] audio engine: add Vector3 class : [Attachment 48182] move Vector3.h to wtf directory per Oliver's request

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 4 17:10:50 PST 2010


Darin Adler <darin at apple.com> has granted Chris Rogers <crogers at google.com>'s
request for review:
Bug 34548: audio engine: add Vector3 class
https://bugs.webkit.org/show_bug.cgi?id=34548

Attachment 48182: move Vector3.h to wtf directory per Oliver's request
https://bugs.webkit.org/attachment.cgi?id=48182&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    void normalize()
> +    {
> +	   if (isZero())
> +	       return;
> +
> +	   double k = 1.0 / abs();
> +	   m_x *= k;
> +	   m_y *= k;
> +	   m_z *= k;
> +    }

Seems to me it would be more efficient to check the result of abs() for 0.0
rather than separately checking isZero. One equality check against zero instead
of three in the normal case.

> +inline Vector3 operator*(double k, const Vector3& v)
> +{
> +    return Vector3(k * v.x(), k * v.y(), k * v.z());
> +}

I think you should also include the version where the vector is on the left.

> +	Vector3 v = v1 - v2;
> +    return v.abs();

There's a stray tab in here on that first line, that needs to be fixed.

You can just write this without the named temporary as

    return (v1 - v2).abs();

Seems a little cleaner like that to me.

r=me but you can’t land until you get rid of that tab.


More information about the webkit-reviews mailing list