modelica
Documentation of Surface visualizer is misleading
There is a wording mismatch in the documentation of the Surface visualizer for the representation of surfaces.
Just for reference, inputs and parameters of Surface are defined here, the replaceable function surfaceCharacteristic has its interface here, and the documentation of the Surface model can be found here.
Documentation talks about control points of the surface, but as far as I know, this terminology is only widely used in the context of Bézier/B-spline curves and surfaces. In this context, however, the points described in the documentation do certainly not refer to control points (which generally do not lie on the surface), and while they may refer to knot points or knot vectors this would not account for the multiplicity of knots.
Instead, it seems that the most widespread word defining the kind of point mentioned in the documentation is vertex (vertices in plural), as this is the atomic element used for instance by OpenGL and Vulkan to actually draw surfaces on screen.
This word is well suited in this context because it refers not only to the geometric coordinates of the point (X, Y, Z) but also to visual attributes like the color (C).
I suppose the problem is because Surface was originally (intended to be) defined as a tensor product spline, but for sure this is no longer the case.
Of course, a modeler is free to first design a tensor-product spline surface and then compute a set of vertices (by evaluating the spline at nu*nv points) in order to make use of the Surface visualizer.
IMHO, problematic lines that should be updated are:
-
here
nuandnvare not sets of parameters but simple parameters that define the number of vertices in each direction, - here "control point" should be changed to "vertex", and "A time-varying position" to "Time-varying coordinates" since a vertex is already defining a position in that sense,
- here it is a combination of the above two amendments,
- here again "control point" should be changed to "vertex" in two occurrences, better use "in the form of" rather than "in form of", and the reading would benefit from a line break before "and an optional color array",
- here, here and here, maybe "surface point" should be changed to "vertex" as well,
- here and here (two occurrences in each) "points" could be changed to "vertices",
- here, here, and here, "positions of points" should be changed to "coordinates of vertices".
By the way, it is a bit unfortunate that it is impossible to set the normals (N) in the characteristic function of the surface (together with an enabler boolean hasProvidedNormals)—similarly to the colors—as they can usually be computed analytically at very low cost, which would provide better rendering than if approximated using vertices.
Additionally, it may be worth documenting that vertices (coordinates X, Y, Z) should be given in a contiguous order such as to avoid drawing invalid polygons (see Figure 2-3 here) and such that taking three/four vertices in a specific pattern iteratively (by the Modelica tool) allows to draw the surface as expected by the user.
Maybe this is always guaranteed when evaluating the surface at monotonic values of u and v, meaning by successively increasing/decreasing indices in both u- and v- directions in two nested for-loops, like the existing surface characteristics are actually done?
I am not sure how to express it correctly and for a generic surface, I should think about it, but you certainly know better than I do.
Of course, it would be possible to circumvent the problem by going through tessellation, but this seems too much hard work while the data could be guaranteed correct upstream.
There is a wording mismatch in the documentation of the Surface visualizer ...
@anotheruserofgithub Please feel free to improve the documentation on your own and set a pull request to be reviewed. (In wavefront format, geometric vertex is used, wikipedia.)
Regarding the other topics:
- normals: the implementation would be straight forward but IMO the tool vendors shall be involved.
- contiguous order of vertices: it implies from the indexing of
X,YandZbyuandv, so we deal here with a parametric surface. Well, maybe this is not that obvious and can be better documented as well.
There is a wording mismatch in the documentation of the Surface visualizer ...
@anotheruserofgithub Please feel free to improve the documentation on your own and set a pull request to be reviewed.
The guide for contributing is found in https://github.com/modelica/ModelicaStandardLibrary/blob/master/CONTRIBUTING.md - in particular please ensure that you or your department (etc) have signed the CLA.
Thank you for the answers!
One last detail (I hope!): For correct back face culling and coloring, vector $\vec{w}=\vec{u}\times\vec{v}$ shall turn counterclockwise (CCW) when facing any vertex $\vec{V}_{u,v}=\left(X_{u,v},Y_{u,v},Z_{u,v}\right)$ from outside the surface (i.e., $\vec{w}$ shall point towards the eye when looking directly in front of the top face), where $u,v$ are indices incremented along $\vec{u}$ and $\vec{v}$ directions respectively, $\vec{u}$ is defined as $\vec{V}_{u+1,v}-\vec{V}_{u,v}$ and $\vec{v}$ as $\vec{V}_{u,v+1}-\vec{V}_{u,v}$. Thus normals shall be outer-pointing normals.
So I will try to explain all those things in a very concise way, and open a PR in the coming days/weeks.
- normals: the implementation would be straight forward but IMO the tool vendors shall be involved.
I think this would be really worth it if you could make this feature in, both in terms of rendering and efficiency.
Note that many surfaces are rendered as double-sided; i.e. GL_LIGHT_MODEL_TWO_SIDE=true. With normals it becomes important to say whether this is intended or not - and possibly have a flag for it.
Could add a doubleSidedSurface parameter to the partial surface model.
Although, am I right in saying that it should already be the case now? Meaning, whether normals are explicitly provided by the user or implicitly approximated by the tool does not matter, because the tool might currently render the surface as double-sided (by default) whilst the user might want to be able to actually render it as single-sided.
(Sorry for my ignorance, I'm very new to this topic.)
Could add a
doubleSidedSurfaceparameter to the partial surface model. Although, am I right in saying that it should already be the case now?
Yes, and no.
The current surface as just a set of points doesn't say which side is front and which side is back so you must see it as doubleSidedSurface. One could add some additional logic for it - but we haven't yet. However, as soon as there is a normal it is clearer.