Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

mesh importer: at least one method to offset the origin should become a permanent non-breakable standard #109

Closed
Yichard opened this issue Aug 14, 2020 · 9 comments
Labels
backwards-compat Either working towards or away from backwards compatibility enhancement New feature or request viewer Related to viewers wontfix Core team has decided that this isn't a feature they want at this time.

Comments

@Yichard
Copy link

Yichard commented Aug 14, 2020

When importing a mesh, the center of coordinate is automatically changed to the geometric center of the mesh. However it makes sense to have it differently, for instance that a rotating door has its center in the hinge (which simplifies scripting).
There are currently four possibilities to "cheat" and obtain this result:

1 Add extra unconnected vertices. This works, since the geometric center is calculated with vertices info only.

2 Add extra unconnected edges. Same as 1, but allow to draw a "bounding box" around the object (in Collada), much more visible (in Blender) than isolated vertices.

3 Add extra triangles. If they are small enough (0.1mm) they raise red errors in the importer, but in fact they are ignored and not accounted in the bounding box, which is good. But I recently found out that they cause some errors in the physical shape, even if the later is imported as a separate file (having the same vertices). It is as if they still formed extra physics faces, not visible in the build window. (I have a reproducible demo of this bug)

4 Use the collada origin. This is what should be done from the start, and everybody else does.

Solution 1 and 2 need no action.
Debugging 3 is not estimated.
Solution 4 probably needs rewriting of the whole importer thing.

But still 1 and 2 arise a problem: if in the future the importer removes "unused" vertices or edges, breaking collada files relying on them. (it would make sense to remove unused vertices which are often inadvertently left in meshes, unintentionally offsetting the center)
So my request is
To be warranted that one of these methods (unused vertices, edges or triangles in the calculation of the mesh center) becomes a non-changeable standard. (at least for one method, preferably 2. This was the solution preferred by Mike Chase)
This just needs that this rule is written somewhere, including in the code, so that future developers find this rule and know why it exists. (yes I request to add comments in the code, lol)

@Yichard Yichard changed the title mesh importer: how to set the origin elsewhere than center (4 methods) mesh importer: at least one method to offset the origin should become a permanent non-breakable standard Aug 14, 2020
@mdickson
Copy link
Contributor

Of the options above only option 3 will likely work reliably. The uploader (the viewer) removes isolated verts and edges and only retains faces. So the current way to do what you're discussing is to add a single tri which isnt visible but sets the center and hence the origin of the object.

It might have been nice to preserve the origin in the collada file but in truth the receiving system (SL) sets the origin for all objects to the object center.

So since the viewer and the target system its built for are forcing origin to object center we're left with option 3. I dont see an easy way to implement different behavior in OpenSim/Halcyon.

@Yichard
Copy link
Author

Yichard commented Aug 14, 2020

1 is currently working well, and probably 2, even if isolated vertices and edges lead to no rendering. So my point is that at least one of these ways is preserved in the future.

@Yichard
Copy link
Author

Yichard commented Sep 3, 2020

Ok rethinking it a bit.

Solution 2 (edges) is not a solution: edges exist only in Blender, not in Collada. So extra edges are importd as extra vertices, which brings us to solution 1.

Solution 1 (vertices) is not stable in time: somebody may think that extra vertices must be culled, as they may cause mesh to be off-center, what some people may see as a bug. Technically it is possible to remove them, but at the cost of some complex operations. Which probably explains why it was never done. So if we want this solution to remain useful, we need to keep the principle of not removing unused vertices, and provide a way that future devs don't do it. (ascending compatibility)

Solution 3 (small triangles) works, but poses problems for physical objects, an even for physical shapes. If the object is rendered as hull, the hull accounts with the extra face. But I saw problems even if rendered as a prim

Only solution 4 is safe. But we need to know for which reason the importer re-centers the mesh. Is there a compelling reason to do so? If not, it is enough to remove the part which does that. Ideally this should be an option, but then we are dependent of the viewers people.

For reasons of learning and facilitating work, solution 1 and solution 4 are equivalent.

@Yichard
Copy link
Author

Yichard commented Sep 3, 2020

Added: checking in the Collada spec, there are edges. But they are not imported in Halcyon. Adding an edge awareness to the importer would solve the problem, by providing a way to distinguish edges from lone vertices. But we have the same problem as with solution 1: in the future a non-informed dev may decide to cull unused edges.

@Ana-Green
Copy link

See: #111

Regards,
Ana Green

@kf6kjg kf6kjg added backwards-compat Either working towards or away from backwards compatibility enhancement New feature or request labels Apr 21, 2021
@kf6kjg
Copy link
Contributor

kf6kjg commented Apr 21, 2021

This issue raises a concern with SL compat. Though ATM I don't know whether or not it's the VIEWER or the SERVER that converts the mesh from collada to the asset format. If it's the viewer, then that is currently out of scope. It'd be good if someone could locate which does which.

@mdickson
Copy link
Contributor

The viewer does the conversion from the Collada/DAE format to the internal SL asset format. To implement this suggestion would require changes to the viewer's conversion process which I suspect is unlikely. Regardless it should be a feature request there.

@kf6kjg kf6kjg added viewer Related to viewers wontfix Core team has decided that this isn't a feature they want at this time. labels Apr 22, 2021
@kf6kjg
Copy link
Contributor

kf6kjg commented Apr 22, 2021

Thank you for that detail, I suspected that was the case, but I've never dug into that area of the code in either.

@kf6kjg kf6kjg closed this as completed Apr 22, 2021
@Yichard
Copy link
Author

Yichard commented Apr 22, 2021

ok if I understand well this is a feature request for firestorm, which will likely send us to the SL Jira.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards-compat Either working towards or away from backwards compatibility enhancement New feature or request viewer Related to viewers wontfix Core team has decided that this isn't a feature they want at this time.
Projects
None yet
Development

No branches or pull requests

4 participants