# [v1] mesa: rotation of 0-vector

Submitted by Sergii Romantsov on Sept. 12, 2018, 1:30 p.m.

## Details

Message ID 1536759035-30661-1-git-send-email-sergii.romantsov@globallogic.com New show "mesa: rotation of 0-vector" ( rev: 1 ) in Mesa

Not browsing as part of any series.

## Commit Message

Sergii Romantsov Sept. 12, 2018, 1:30 p.m.
```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://bugs.freedesktop.org/show_bug.cgi?id=100960
Signed-off-by: Sergii Romantsov <sergii.romantsov@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.
+      * */
+      if (x == 0 && y == 0 && z == 0)
+         x = 1.0f;
_math_matrix_rotate( ctx->CurrentStack->Top, angle, x, y, z);
ctx->NewState |= ctx->CurrentStack->DirtyFlag;
}

```

```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.
>
> Signed-off-by: Sergii Romantsov <sergii.romantsov@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 ...
*/

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;

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
```
```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:
>>
>> Signed-off-by: Sergii Romantsov <sergii.romantsov@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
```
```Hello,
thanks for looking on that.
Will update after any answer on
https://github.com/KhronosGroup/OpenGL-API/issues/41
Hope that is correct place for such things.

On Thu, Sep 13, 2018 at 4:57 AM, Timothy Arceri <tarceri@itsqueeze.com>
wrote:

> 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.
>>>
>>> 2F%2Fbugs.freedesktop.org%2Fshow_bug.cgi%3Fid%3D100960&
>>> amp;data=02%7C01%7Cbrianp%40vmware.com%7Cfde139ec0f4448b
>>> 8090d08d618b3f0ce%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%
>>> 7C0%7C636723558454036605&amp;sdata=IAlt%2FkpHJyb5JHRYaOTfhf5
>>> v9V7Xl5iQBNBdqe2PHQs%3D&amp;reserved=0
>>> Signed-off-by: Sergii Romantsov <sergii.romantsov@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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
```