#1818 closed defect (wontfix)
Normals incorrect for instanced objects different scaling along each axis
Reported by: | Tristan Croll | Owned by: | Tom Goddard |
---|---|---|---|
Priority: | minor | Milestone: | |
Component: | Graphics | Version: | |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Notify when closed: | Platform: | all | |
Project: | ChimeraX |
Description
Context is a new "adaptive" distance restraint scheme (see https://twitter.com/CrollTristan/status/1113122594616741890). To represent the restraints, I'm using a cylinder (surface.shapes.cylinder_geometry()
) for the target distance, connected at each end to the restrained atoms with cones (surface.shapes.cone_geometry()
). Zooming in on a set of stretched restraints, the cone normals are all wrong (see attached picture). Looks to me like they're being transformed in the same way as the vertices themselves, which is only correct when the transformation is strictly rotation/scale or when there is no skew component parallel to the normal. From what I gather, the generally correct transformation for normals is the transpose of the inverse of the vertex transform. I can see it would be a horrible waste of resources to use that everywhere, but would it be possible to provide an optional switch on Drawing
to turn on the strict treatment?
Attachments (1)
Change History (4)
by , 7 years ago
Attachment: | weird_normals.jpg added |
---|
comment:1 by , 7 years ago
Resolution: | → wontfix |
---|---|
Status: | assigned → closed |
You are right the graphics rendering assumes the Model position is assumed to be a rotation so that the normals transform by just multiplying the rotation. If there are instead different scale factors along different axes then the proper transformation of the normals requires multiply by the inverse transpose of the 3x3 transform. That is a very slow computation and adds complexity to the code which is almost never needed. OpenGL shaders do not handle matrix inverse. Even if they did you likely would not want to compute the inverse on every frame, since just rotating would not change any model position matrices. So the inverse should probably be computed on the CPU. That will be painfully slow.
The best solution is probably to use a cylinder instead of a cone. Even though it has different scaling along its length and radius the normals are correct because each normal sees just one scale factor.
If you really wanted to use the cones you can set the normal vectors for the cone yourself every time you change their matrix to produce the correct transformed direction. It wouldn't require a matrix inverse since you have special case where you are just stretching the cone, so you just need to scale the normal cone axis component (z-axis), and scale xy components according to the cone aspect ratio.
comment:2 by , 7 years ago
That's more-or-less what I expected the issue to be. No problem - too minor an issue to really spend a whole lot of time on it (I like the cones and will probably keep them regardless). Pre-adjusting the normals per-transform doesn't really look like a viable option without a huge amount of work - under typical circumstances there will be thousands of these things, and unless I'm missing something this approach would sidestep all your optimised code. An alternative would be to allow an optional array of pre-calculated transforms for the normals (defaulting to the vertex transforms if not provided) - but again, probably too minor to justify the effort. The only other application I can think of off-hand would be in visualisation of anisotropic B-factors (where the sphere for each atom is stretched to represent the shape of its thermal ellipsoid). On 2019-04-11 01:48, ChimeraX wrote:
follow-up: 2 comment:3 by , 6 years ago
Summary: | Normals incorrect for non-rigid transforms → Normals incorrect for instanced objects different scaling along each axis |
---|
This problem is primarily of concern with instanced objects. For Drawings with a single position matrix I think it is reasonable to require a rotation and translation with no scaling. For instanced objects like spheres for atomsdifferent scale factors are used for different instances, and for cylinder instances used to draw bonds different scale factors are used along the cylinder axis and perpendicular to that axis. Those are the most widely used cases of instancing and the normal vectors are properly transformed in those cases because of the special geometry. For instanced cones (used for ribbon tethers and ISOLDEs distance restraints) the normals won't be transformed correctly and lighting will look wrong. For instanced ellipsoids (we don't have any case of this currently, but has been used in Chimera for anisotropic b-factor display) the normals also won't be right.
The way to handle this would be for Drawing to compute an array of normal vector transforms (inverse transpose of position matrix) and the OpenGL vertex shader would need to support that array and apply it to normals. Probably the user would also specify if this is needed since for the common molecular cases of spheres and cylinders it is not needed, and it will slow down rendering and especially changes in position if normal transforms have to be computed. While this solution is sensible, the complexity and limited used cases and too many other priorities mean I am not going to add it.
Cylinder and cone colours are identical for a given bond.