KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

OpenGL: laсk of specification for glRotated and 0-vector #41

Closed sergiiromantsov closed 5 years ago

sergiiromantsov commented 6 years ago

Hello, in the Mesa we have got an issue (https://bugs.freedesktop.org/show_bug.cgi?id=100960). Potential reason of that is: application Minecraft uses call glRotated(180, 0, 0, 0). Unfortunately, i haven't found in the specification any definitions how to process 0-vector. Please, could you point me if it exist?

With Windows-drivers and Nvidia's Linux driver application works as author expects. Looks like that drivers uses some workaround and treats 0-vector: (0, 0, 0) as (1, 0, 0).

Is it possible to specify and update specification?

pdaniell-nv commented 5 years ago

glRotated is documented on page 499 of the OpenGL 4.6 compatibility spec (https://www.khronos.org/registry/OpenGL/specs/gl/glspec46.compatibility.pdf). Are you saying the formula doesn't work with (0, 0, 0)?

pdaniell-nv commented 5 years ago

We discussed this in a working group meeting and concluded that this content is attempting to use undefined behavior. Creating a normalized vector between the origin (0,0,0) and (0,0,0) will result in a divide by zero so to be useful for errant content implementations may choose to do something more helpful. We agreed it would be fine for implementation to do something useful for this undefined behavior.

Let the hardware vendor know about this issue if you're stuck and they could address this in their driver.

nvpbrown commented 5 years ago

The fundamental problem here seems to be that glRotate wants to normalize the (x,y,z) vector, and normalizing (0,0,0) is ill-defined. It appears that NVIDIA's implementation has a special case that normalizes zero vectors to (0,0,0). If you don't have special-case logic in your implementation, I presume that you would end up with (NaN, NaN, NaN).

I worked through the math from the specification quickly

R = u u_transpose + cos(angle) (I - u u_transpose) + sin(angle) S

If u is a zero vector, the first term is a zero matrix. The matrix S is also a zero matrix as is sin(angle), to the third term is also a zero matrix, which leaves you with:

R = cos(angle) * I

where I is the identity matrix. When angle is 180, cos(angle) should be -1. That means the call would produce a result identical to glScaled(-1,-1,-1).

It might be worth hacking on Mesa to see if adding special normalization logic produces the desired result.

Of course, it would be better to fix the application code to not do something so ill-defined.

sergiiromantsov commented 5 years ago

Thanks for response. Seems as a general solution it will be good to treat a magnitude as 1 if it was computed as 0. Is it ok?

sergiiromantsov commented 5 years ago

Or really optimize it to cos(angle) * I ?....

nvpbrown commented 5 years ago

@sergiiromantsov: When you are talking about "treat a magnitude as 1" or "optimize it to", are you proposing that the specification guarantee this sort of behavior or that implementations treat corner cases in this manner?

I think the general consensus in the working group is that this behavior is ill-defined, where calling glRotate* with a (0,0,0) vector is an application bug that will have undefined results. As far as I'm aware, the specification does not have any special-case definition for vector normalization for zero-length vectors. The general spec language for floating-point math does not explicitly define what should happen here either:

2.3.4.1 Floating-Point Computation We do not specify how floating-point numbers are to be represented, or the details of how operations on them are performed. ... The special values Inf and -Inf encode values with magnitudes too large to be represented; the special value NaN encodes “Not A Number” values resulting from undefined arithmetic operations such as 0/0 . Implementations are permitted, but not required, to support Infs and NaNs in their floating-point computations.

My previous comments were just explaining how this call should end up being handled by NVIDIA's implementation, with a special case for normalizing (0,0,0). This special case has existed in our driver since before I joined in 2001 and I found no indication why we handle vector normalization this way. My best guess is that it was done this way to avoid NaN values and possible floating-point exceptions when doing vector normalization on the CPU.

I'm not sure there's any particularly useful definition for vector normalization for ill-conditioned inputs. The two cases that jump out to me where vector normalization happens is this sort of rotation and lighting math. For lighting (section 12.2.1.1), normalizing a zero vector to produce a zero vector (NVIDIA's behavior) seems reasonable -- it means that any portion of the lighting equation using such a vector doesn't contribute at all to the final color. But for rotation, I don't think there is a reasonable choice. As I noted in my previous comment, this special case for normalizing a zero vector turns a rotate call into something that is not a rotation at all!

glScaled(cos(angle), cos(angle), cos(angle));

For example, I'm pretty sure that glRotated(90, 0, 0, 0) will produce a zero matrix that will shred all your geometry.

sergiiromantsov commented 5 years ago

@nvpbrown: Hello, Pat. Thanks for your response. I understand that such behavior will not be defined in specification, so mostly i meant treating it in implementation. So would like to make compatible behavior of Mesa with Nvidia and Windows drivers and i'm working on it (at this moment preparing a piglit-test).

ср, 3 жовт. 2018 о 18:54 Pat Brown notifications@github.com пише:

@sergiiromantsov https://github.com/sergiiromantsov: When you are talking about "treat a magnitude as 1" or "optimize it to", are you proposing that the specification guarantee this sort of behavior or that implementations treat corner cases in this manner?

I think the general consensus in the working group is that this behavior is ill-defined, where calling glRotate* with a (0,0,0) vector is an application bug that will have undefined results. As far as I'm aware, the specification does not have any special-case definition for vector normalization for zero-length vectors. The general spec language for floating-point math does not explicitly define what should happen here either:

2.3.4.1 Floating-Point Computation We do not specify how floating-point numbers are to be represented, or the details of how operations on them are performed. ... The special values Inf and -Inf encode values with magnitudes too large to be represented; the special value NaN encodes “Not A Number” values resulting from undefined arithmetic operations such as 0/0 . Implementations are permitted, but not required, to support Infs and NaNs in their floating-point computations.

My previous comments were just explaining how this call should end up being handled by NVIDIA's implementation, with a special case for normalizing (0,0,0). This special case has existed in our driver since before I joined in 2001 and I found no indication why we handle vector normalization this way. My best guess is that it was done this way to avoid NaN values and possible floating-point exceptions when doing vector normalization on the CPU.

I'm not sure there's any particularly useful definition for vector normalization for ill-conditioned inputs. The two cases that jump out to me where vector normalization happens is this sort of rotation and lighting math. For lighting (section 12.2.1.1), normalizing a zero vector to produce a zero vector (NVIDIA's behavior) seems reasonable -- it means that any portion of the lighting equation using such a vector doesn't contribute at all to the final color. But for rotation, I don't think there is a reasonable choice. As I noted in my previous comment, this special case for normalizing a zero vector turns a rotate call into something that is not a rotation at all!

glScaled(cos(angle), cos(angle), cos(angle));

For example, I'm pretty sure that glRotated(90, 0, 0, 0) will produce a zero matrix that will shred all your geometry.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KhronosGroup/OpenGL-API/issues/41#issuecomment-426690631, or mute the thread https://github.com/notifications/unsubscribe-auth/AIOpQ-MzLiLJzpwKzQDRnJii9cCpEE25ks5uhN2tgaJpZM4Wm3lJ .

nvpbrown commented 5 years ago

@sergiiromantsov Sounds good, thanks. Let us know if you discover anything interesting in this experiment. All of my analysis came from a code inspection -- I didn't actually confirm my findings with a specific test.

Thanks, Pat

pdaniell-nv commented 5 years ago

@sergiiromantsov can we close this issue?

sergiiromantsov commented 5 years ago

Probably, yes, at this moment. Thanks