Suggestion to change OXY attachment behaviour

Have some feature requests, feedback, cool stuff to share, or want to know where FreeCAD is going? This is the place.
Forum rules
Be nice to others! Read the FreeCAD code of conduct!
Post Reply
User avatar
Jolbas
Posts: 327
Joined: Sat Mar 26, 2022 7:48 am
Location: Sweden

Suggestion to change OXY attachment behaviour

Post by Jolbas »

The wiki for OXY attachment describes how the second direction is guessed when omitted. I don't think the guessing process is very good and have room for improvement but I want your opinion before changing.
I started to look into this after reading a post by @ppemawm and the wiki by @edwilliams16.
I like to suggest that the "hint" direction instead uses the local axis corresponding to the last character. This is true now for OXZ, OYZ and OZY attachments. But not true for OXY, OYX and OZX.

To achieve this I planned to target the rotation constructor Rotation(vX, vY, vZ, orderString) in Rotation.cpp which is used by the Attachment engine. I realize the change in some cases may brake models but I think it will make it more intuitive and beneficial for future works.

If the orderString is for example "YZX" the current implementation works like this to find a rotated CS:
- if vY (first character in "YZX") is not a zero vector, lock the new Y axis to this direction
- else try the same for vZ and then vX
- look for a hint direction in remaining vectors
- if no hint direction is found, use a hard coded local axis as hint as described in the wiki link above

I would like to change the behavior to this:
- if vY is not a zero vector, lock the new Y axis to this direction
- else lock the new Y axis to the local Y
- if vZ (second character in "YZX") is not a zero vector, use as a hint for the new Z axis
- else use the local Z as hint direction
- in case the main and hint directions turns out to be equal. try to use the last vector as hint

I suggest that a `orderString` with only two characters is accepted too.

Related thread: viewtopic.php?t=69662
chrisb
Veteran
Posts: 53920
Joined: Tue Mar 17, 2015 9:14 am

Re: Suggestion to change OXY attachment behaviour

Post by chrisb »

Jolbas wrote: Sat Mar 25, 2023 7:03 am I realize the change in some cases may brake models but I think it will make it more intuitive and beneficial for future works.
If clearly documented, this should be ok. Because the model has not specified the second direction, an arbitrary direction could be considered as ok.
A Sketcher Lecture with in-depth information is available in English, auf Deutsch, en français, en español.
drmacro
Veteran
Posts: 8865
Joined: Sun Mar 02, 2014 4:35 pm

Re: Suggestion to change OXY attachment behaviour

Post by drmacro »

I'm not sure this is necessary. But, that may be because I understand it as is.

Maybe it is really an issue of explaining it correctly. In the video below is the best I've ever seen it explained.

https://youtu.be/5dPL3Cr_Xf0
Star Trek II: The Wrath of Khan: Spock: "...His pattern indicates two-dimensional thinking."
edwilliams16
Veteran
Posts: 3106
Joined: Thu Sep 24, 2020 10:31 pm
Location: Hawaii
Contact:

Re: Suggestion to change OXY attachment behaviour

Post by edwilliams16 »

@Jolbas

If I understand your proposal correctly:

(1) The normal domain of App.Rotation(vx, vy, vz, 'ABC') is vx, vy and vz are directions - non-zero vectors - except since the vC argument is ignored it could be the zero vector. You would like to extend the domain so that a zero vector is interpreted as a unit vector in the corresponding coordinate direction. (All zero would presumably be the identity - currently throws an error)

eg App.Rotation(zero, vy, vz, 'XYZ') would be interpreted as App.Rotation(App.Vector(1, 0, 0), vy, vz, 'XYZ') etc.
This would have no effect on the normal domain of the function. I have not investigated exactly what zero arguments currently do, but I agree that the fall-back defaults in the attachment modes are somewhat arbitrary.

(2) You would like to allow two character strings. Presumably this would mean that
App.Rotation(vx, vy, vz, 'AB') would be the synonymous with App.Rotation(vx, vy, vz, 'ABC') or did you have something else in mind?

See also:
https://blog.freecad.org/2023/01/16/the ... n-freecad/

In the attachment dialog, if the three selections were Vertex1, Vertex1 and Vertex2 and the attachment mode 'OXY' (generating a zero vector), I presume the desired result would be that the origin would attach to Vertex1, its X-direction would align with the local x-direction and its Y-direction would lie in the plane defined by Vertex2? This could be useful when there is not an x-axis reference immediately available.
User avatar
Jolbas
Posts: 327
Joined: Sat Mar 26, 2022 7:48 am
Location: Sweden

Re: Suggestion to change OXY attachment behaviour

Post by Jolbas »

edwilliams16 wrote: Sat Mar 25, 2023 8:31 pm If I understand your proposal correctly
I think you did.
I would like the following to pass:

Code: Select all

vecX = App.Vector(1, 0, 0)
vecY = App.Vector(0, 1, 0)
vecZ = App.Vector(0, 0, 1)
vec0 = App.Vector(0, 0, 0)
vec1 = App.Vector(1, 1, 1)
assert App.Rotation(vec1, vec0, vec0, 'XYZ') == App.Rotation(vec1, vecY, vecZ, 'XYZ')
assert App.Rotation(vec1, vec0, vec0, 'XZY') == App.Rotation(vec1, vecY, vecZ, 'XZY')
assert App.Rotation(vec0, vec1, vec0, 'YXZ') == App.Rotation(vecX, vec1, vecZ, 'YXZ')
assert App.Rotation(vec0, vec1, vec0, 'YZX') == App.Rotation(vecX, vec1, vecZ, 'YZX')
assert App.Rotation(vec0, vec0, vec1, 'ZXY') == App.Rotation(vecX, vecY, vec1, 'ZXY')
assert App.Rotation(vec0, vec0, vec1, 'ZYX') == App.Rotation(vecX, vecY, vec1, 'ZYX')
Currently 2 out of 6 works.
edwilliams16 wrote: Sat Mar 25, 2023 8:31 pm This would have no effect on the normal domain of the function
When using OXY-attachment with only two reference points, a call equivalent to

Code: Select all

App.Rotation(point2-point1, vec0, vec0, 'XYZ')
is made to find the rotation
edwilliams16 wrote: Sat Mar 25, 2023 8:31 pm App.Rotation(vx, vy, vz, 'AB') would be the synonymous with App.Rotation(vx, vy, vz, 'ABC') or did you have something else in mind?
Nothing else in mind. The third character have no effect and this would be in line with the name of the attachment modes: OXY, OXZ, etc.
edwilliams16
Veteran
Posts: 3106
Joined: Thu Sep 24, 2020 10:31 pm
Location: Hawaii
Contact:

Re: Suggestion to change OXY attachment behaviour

Post by edwilliams16 »

@Jolbas
Work like this?

Code: Select all

vecX = App.Vector(1, 0, 0)
vecY = App.Vector(0, 1, 0)
vecZ = App.Vector(0, 0, 1)
vec0 = App.Vector(0, 0, 0)
vec1 = App.Vector(1, 1, 1)
assert App.Rotation(vec1, vec0, vec0, 'XYZ').isSame(App.Rotation(vec1, vecY, vecZ, 'XYZ'), 1e-14)
assert App.Rotation(vec1, vec0, vec0, 'XZY').isSame(App.Rotation(vec1, vecY, vecZ, 'XZY'), 1e-14)
assert App.Rotation(vec0, vec1, vec0, 'YXZ').isSame(App.Rotation(vecX, vec1, vecZ, 'YXZ'), 1e-14)
assert App.Rotation(vec0, vec1, vec0, 'YZX').isSame(App.Rotation(vecX, vec1, vecZ, 'YZX'), 1e-14)
assert App.Rotation(vec0, vec0, vec1, 'ZXY').isSame(App.Rotation(vecX, vecY, vec1, 'ZXY'), 1e-14)
assert App.Rotation(vec0, vec0, vec1, 'ZYX').isSame(App.Rotation(vecX, vecY, vec1, 'ZYX'), 1e-14)
#for consistency - vec0 replaced by corresponding coordinate
assert App.Rotation(vec0, vec0, vec0, 'XYZ').isSame(App.Rotation(vecX, vecY, vecZ, 'XYZ'), 1e-14)
assert App.Rotation(vec0, vec1, vec0, 'XYZ').isSame(App.Rotation(vecX, vec1, vec0, 'XYZ'), 1e-14)


def rotWrapper(vx, vy, vz, str):
    if vx.Length == 0.0:
        vx = App.Vector(1, 0, 0)
    if vy.Length == 0.0:
        vy = App.Vector(0, 1, 0)
    if vz.Length == 0.0: 
        vz = App.Vector(0, 0, 1)
    return App.Rotation(vx, vy, vz, str)


assert rotWrapper(vec1, vec0, vec0, 'XYZ').isSame(rotWrapper(vec1, vecY, vecZ, 'XYZ'), 1e-14)
assert rotWrapper(vec1, vec0, vec0, 'XZY').isSame(rotWrapper(vec1, vecY, vecZ, 'XZY'), 1e-14)
assert rotWrapper(vec0, vec1, vec0, 'YXZ').isSame(rotWrapper(vecX, vec1, vecZ, 'YXZ'), 1e-14)
assert rotWrapper(vec0, vec1, vec0, 'YZX').isSame(rotWrapper(vecX, vec1, vecZ, 'YZX'), 1e-14)
assert rotWrapper(vec0, vec0, vec1, 'ZXY').isSame(rotWrapper(vecX, vecY, vec1, 'ZXY'), 1e-14)
assert rotWrapper(vec0, vec0, vec1, 'ZYX').isSame(rotWrapper(vecX, vecY, vec1, 'ZYX'), 1e-14)
#for consistency - vec0 replaced by corresponding coordinate
assert rotWrapper(vec0, vec0, vec0, 'XYZ').isSame(rotWrapper(vecX, vecY, vecZ, 'XYZ'), 1e-14)
assert rotWrapper(vec0, vec1, vec0, 'XYZ').isSame(rotWrapper(vecX, vec1, vec0, 'XYZ'), 1e-14)
User avatar
Jolbas
Posts: 327
Joined: Sat Mar 26, 2022 7:48 am
Location: Sweden

Re: Suggestion to change OXY attachment behaviour

Post by Jolbas »

@edwilliams16
You are so understanding. :)

Then also add missing third character to order.

Code: Select all

def rotWrapper(vx, vy, vz, order):
    if len(order) == 2:
        order += (set('XYZ') - set(order)).pop()
    if vx.Length == 0.0:
        vx = App.Vector(1, 0, 0)
    if vy.Length == 0.0:
        vy = App.Vector(0, 1, 0)
    if vz.Length == 0.0: 
        vz = App.Vector(0, 0, 1)
    return App.Rotation(vx, vy, vz, order)

assert rotWrapper(vecY, vecZ, vecX, 'ZX').isSame(rotWrapper(vecY, vecZ, vecX, 'ZXY'), 1e-14)
david69
Veteran
Posts: 1773
Joined: Wed Jan 01, 2014 7:48 pm

Re: Suggestion to change OXY attachment behaviour

Post by david69 »

Jolbas wrote: Sat Mar 25, 2023 7:03 am The wiki for OXY attachment describes how the second direction...
from a wiki point of view, don't you think it would be better to directly added these information on https://wiki.freecad.org/Part_EditAttac ... al_to_edge instead of having a separate page?
One can use hidding text ({| class="wikitable mw-collapsible mw-collapsed")
Bance
Veteran
Posts: 4186
Joined: Wed Feb 11, 2015 3:00 pm
Location: London

Re: Suggestion to change OXY attachment behaviour

Post by Bance »

david69 wrote: Sun Mar 26, 2023 11:08 am from a wiki point of view, don't you think it would be better to directly added these information on https://wiki.freecad.org/Part_EditAttac ... al_to_edge
+1 definitely yes.
Post Reply