[webkit-reviews] review denied: [Bug 34548] audio engine: add Vector3 class : [Attachment 48086] change filename from VectorTypes.h to Vector3.h

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has denied 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 48086: change filename from VectorTypes.h to Vector3.h
https://bugs.webkit.org/attachment.cgi?id=48086&action=review

------- Additional Comments from Sam Weinig <sam at webkit.org>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 54315)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,30 @@
> +2010-02-03  Chris Rogers  <crogers at google.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   audio engine: add Vector3 class
> +	   https://bugs.webkit.org/show_bug.cgi?id=34548
> +
> +	   No tests since no javascript API yet
> +
> +	   * platform/audio: Added.
> +	   * platform/audio/Vector3.h: Added.
> +	   (WebCore::Vector3::Vector3):
> +	   (WebCore::Vector3::abs):
> +	   (WebCore::Vector3::isZero):
> +	   (WebCore::Vector3::normalize):
> +	   (WebCore::Vector3::x):
> +	   (WebCore::Vector3::y):
> +	   (WebCore::Vector3::z):
> +	   (WebCore::operator+):
> +	   (WebCore::operator-):
> +	   (WebCore::operator*):
> +	   (WebCore::dot):
> +	   (WebCore::cross):
> +	   (WebCore::Vector3::dot):
> +	   (WebCore::Vector3::cross):
> +	   (WebCore::Vector3::distance):
> +
>  2010-02-03  Adele Peterson  <adele at apple.com>
>  
>	   Reviewed by Brady Eidson.
> Index: WebCore/platform/audio/Vector3.h
> ===================================================================
> --- WebCore/platform/audio/Vector3.h	(revision 0)
> +++ WebCore/platform/audio/Vector3.h	(revision 0)
> @@ -0,0 +1,165 @@
> +/*
> + * Copyright (C) 2010 Google Inc. All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1.  Redistributions of source code must retain the above copyright
> + *	  notice, this list of conditions and the following disclaimer.
> + * 2.  Redistributions in binary form must reproduce the above copyright
> + *	  notice, this list of conditions and the following disclaimer in the
> + *	  documentation and/or other materials provided with the distribution.
> + * 3.  Neither the name of Apple Computer, Inc. ("Apple") nor the names of
> + *	  its contributors may be used to endorse or promote products derived
> + *	  from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED

> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY

> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef Vector3_h
> +#define Vector3_h
> +
> +#include <math.h>
> +
> +namespace WebCore {
> +
> +//
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> +// Basic 3D vector type

I don't think this comment adds much.

> +
> +    double dot(const Vector3& v) const;
> +
> +    Vector3 cross(const Vector3& v) const;
> +
> +    double distance(const Vector3& v) const;

The parameter names here should be removed.

> +private:
> +    friend double dot(const Vector3& v1, const Vector3& v2);
> +    friend Vector3 cross(const Vector3& v1, const Vector3& v2);
> +    friend Vector3 operator*(double k, const Vector3& v);
> +    friend Vector3 operator+(const Vector3& v1, const Vector3& v2);
> +    friend Vector3 operator-(const Vector3& v1, const Vector3& v2);

Here too.

> +inline Vector3 cross(const Vector3& v1, const Vector3& v2)
> +{
> +    double x3 = v1.m_y * v2.m_z  -  v1.m_z * v2.m_y;
> +    double y3 = v1.m_z * v2.m_x  -  v1.m_x * v2.m_z;
> +    double z3 = v1.m_x * v2.m_y  -  v1.m_y * v2.m_x;
> +    return Vector3(x3, y3, z3);
> +}

Some double spaces here.

r- to fix these nits.