[Mesa-dev] [PATCH v1] mesa: rotation of 0-vector

[Timothy Arceri]

On 13/9/18 12:44 am, Brian Paul wrote:
> On 09/12/2018 07:30 AM, Sergii Romantsov wrote:
>> Specification doesn't define behaviour for rotation of 0-vector.
>> But khronos.org says that vector has to be normalized.
>> As workaround assumed that for 0-vector x-position will be
>> defined as 1.0f.
>>
>> Bugzilla: 
>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D100960&data=02%7C01%7Cbrianp%40vmware.com%7Cfde139ec0f4448b8090d08d618b3f0ce%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C636723558454036605&sdata=IAlt%2FkpHJyb5JHRYaOTfhf5v9V7Xl5iQBNBdqe2PHQs%3D&reserved=0 
>>
>> Signed-off-by: Sergii Romantsov <sergii.romantsov at globallogic.com>
>> ---
>>   src/mesa/main/matrix.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/src/mesa/main/matrix.c b/src/mesa/main/matrix.c
>> index 8065a83..631b203 100644
>> --- a/src/mesa/main/matrix.c
>> +++ b/src/mesa/main/matrix.c
>> @@ -415,6 +415,11 @@ _mesa_Rotatef( GLfloat angle, GLfloat x, GLfloat 
>> y, GLfloat z )
>>      FLUSH_VERTICES(ctx, 0);
>>      if (angle != 0.0F) {
>> +      /* khronos.org says that ||( x,y,z )|| = 1 (if not, the GL will 
>> normalize this vector)
>> +      * So that is kind of workaround for empty-vectors.
>> +      * */
> 
> Comment style should be:
> 
> /* khronos.org says ...
>   * So that ...
>   */

Thanks for looking into the bug. However the api docs shouldn't be 
quoted. They are unversioned, are at times wrong and should not be used 
as a definative source when making changes or implementing features. 
Please only quote and use the spec for such things.

> 
> You might also note the bug number in the code in case anyone wonders 
> what app would hit this.
> 
> 
>> +      if (x == 0 && y == 0 && z == 0)
>> +         x = 1.0f;

If the spec doesn't define behavior we should probably file a bug 
against the compatibility spec rather than assuming things. Clearly 
recording the problem (including applications it appears in), specifying 
the current behaviors in other drivers and proposing updated wording to 
the spec is usually the fastest way to resolve this type of thing.

Also if this were the correct change it should be made in 
_math_matrix_rotate() this change will make some of the existing code 
there unreachable.

Finally it would be a good idea to have a piglit test for this once any 
spec clarification is done.

> 
> You may as well use 0.0f in the test too, just to be sure no silly 
> int/float/double conversion happens.
> 
>>         _math_matrix_rotate( ctx->CurrentStack->Top, angle, x, y, z);
>>         ctx->NewState |= ctx->CurrentStack->DirtyFlag;
>>      }
>>
> 
> -Brian
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev