largestDimension() does not return the largest dimension

[greyltc]

Why is it the diagonal of the bounding box times five?

cadquery/cadquery/cq.py

Line 2020 in 74573fc

return s.BoundingBox().DiagonalLength * 5.0

[jmwright]

I think that code has been around since the first commit of CadQuery 1.x, so I'm not sure why it's that way. The test seems to account for it being that way.

@dcowden Do you remember why it's that way? Was it a work-around for something in FreeCAD?

[adam-urbanczyk]

@jmwright I think it was related to how cuts used to be make (i.e. with CSG ).

[adam-urbanczyk]

Do you want to use largestDimension for something @greyltc ? Otherwise I'd like to close the issue.

[greyltc]

Yeah. I wanted to use it to find the largest dimension of an object 😉
I ended up writing my own that's pretty similar. It returns an accurate value (barring some corner cases that I didn't bother to address) and lets the user decide to measure along X, Y or Z:

https://github.com/greyltc/mechanical/blob/bc8b0bbcb578ae39a1a7801aeee27fcc613fa3c3/toolbox/utilities.py#L144-L161

It's not a drop in replacement, but it gets the job done for me.

I'm not really sure why this should be closed though, besides it being annoyoing to have issues open 😉. The function as it is today doesn't do what its namesake says it does so I'd imagine it could cause someone else problems down the road if it's left like this.

It also has a magic number in it so that's certianly a bad thing. I mean, why shouldn't 5.0 be 2.5 or 50.0?

[jmwright]

I think at one time I tried using longestDistance to set camera position, but ended up using the bounding box, and combining the extents of the bounding boxes of all the objects in the scene when there was more than one.

https://github.com/jmwright/cadquery-gui/blob/master/assets/js/viewer.js#L157

My preference would be to leave this open and determine an example case that shows how it should work. If we can determine the expected behavior I'll put together a PR when I get a chance.

[greyltc]

We could mesh the objects and calculate the distance between all possible pairs of verticies and return the largest value of that list...

[adam-urbanczyk]

@greyltc this sounds rather slow. There is OBB in opencascade https://www.opencascade.com/doc/occt-7.4.0/refman/html/class_b_rep_bnd_lib.html#aed61fe542529ccd72e7e8fdad61a0bbe we could use it.
The main question here what is the desired largest dimension definition OBB,AABB,minimum enclosing sphere...

NB : the original question you had was answered (why diag*5 --> historical reasons), that is why I wanted to close the issue.

[greyltc]

the original question you had was answered (why diag*5 --> historical reasons), that is why I wanted to close the issue.

You're right!

[greyltc]

I personally think the diameter of the minimum enclosing sphere would be good!

[jmwright]

Corrected in #317