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

[TD] handle equal tolerances correctly #4197

Closed
wants to merge 1 commit into from

Conversation

donovaly
Copy link
Member

@donovaly donovaly commented Dec 27, 2020

the common rule is that if a dimension has equal over- and undertolerance, they are output on the same line as the dimension concatenated using the ± character.

This PR does this.
(It also uses a more self-explaining function name.)

Note that this is not just cosmetics, it is even standardized in the GD&T norms. The issue is also described in the Wiki: https://wiki.freecadweb.org/TechDraw_Geometric_dimensioning_and_tolerancing#Formatting_2

The implementation allows the user to choose

  • if he uses the same value for over- undertolerance, he gets the common ± output
  • if he enters the negated overtolerance for the undertolerance, he gets the double line output that is common for non-equal tolerances

@aapo-aapo
Copy link
Contributor

That's a good improvement, thanks! Just a couple of notes, though:

  1. There should preferably be space characters around the ± character (or, no spaces at all). Currently in the PR, there seems to be space before the ±, but not after, which is inconsistent.
  2. It's a bit hackish to use the sign of the undertolerance to trigger the feature. Maybe add a tolerance Property to use bilateral tolerancing if possible?

@donovaly
Copy link
Member Author

donovaly commented Dec 27, 2020

1. There should preferably be space characters around the ± character (or, no spaces at all).  Currently in the PR, there seems to be space before the ±, but not after, which is inconsistent.

Oops, this was a typo. I'll fix this.

Besides this, we have a general layout issue with tolerances I'll open a forum thread to find a solution. Just as reference, here is what e.g. the ASME norm Y14.5 2018 defines as layout:
AcroRd32_eASeHof6nu

2. It's a bit hackish to use the sign of the undertolerance to trigger the feature.  Maybe add a tolerance Property to use bilateral tolerancing if possible?

That was my first thought but how to achieve this? The norms require bilateral tolerancing if under- and overtolerance is the same. But as user you are lazy and only want to enter one value. But then we need a way to handle the case that e.g. the undertolerance is zero and not because the user is lazy and only input one value.

This is how I came to the implementation of this PR:

  • entering only one value means the zero tolerance is really zero
  • entering the same value triggers automatically the normed bilateral output
  • by entering the same value with different signs you can override the bilateral output (I bet some users want this for some reason)

What would be your proposal to improve this?

@donovaly
Copy link
Member Author

What would be your proposal to improve this?

OK, we could do this:

  • we add a new property just named "Tolerance"
  • if "Tolerance" is not zero and over- and undertolerance are zero, you get the ± output
  • if at least over- or undertolerance is not zero, then "Tolerance" is ignored and you get the two-line tolerance output

This way we enable the user to save time by entering only one value and keep the functionality to get the two-line output if he really wants. If you agree, I'll implement this.

@aapo-aapo
Copy link
Contributor

What would be your proposal to improve this?

Why not just add a boolean "PreferBilaterals" to TechDraw Dimension Preferences, with default value "true". If this boolean would be true, the ± sign would be used if possible (i.e. if the sum of the tolerance values is zero), and the classic printing would be used if ± does not fit. If the boolean in the Prefs would be false, the classic printing would always be used. I would surely always use the new way, but it'd be nice to offer the choice for non-standard users!

@donovaly
Copy link
Member Author

donovaly commented Dec 28, 2020

Why not just add a boolean "PreferBilaterals" to TechDraw Dimension Preferences, with default value "true".

I prefer my proposal because the boolean does not increase the workload. When I have e.g. ± 0.5 as tolerance, I want to just enter the 0.5 once and not 2 values to make sure it is not + 0.5 and 0.0.
In most of ma real-life drawings I have a mixture of equal and non-equal tolerances so a preferences setting would also not help much.

What do you think would be a disadvantage in my proposal?

the classic printing would always be used.

I think there is no such classic. The norms I could access so far use for equal tolerance ±. Only because FC handled this wrong, it doesn't make it a classic :-) With my proposal, users who nevertheless want to use the classical misbehavior, the could do so.

@donovaly
Copy link
Member Author

I prefer my proposal

to make it clear, I refer to this one:
#4197 (comment)
not the one of the PR as it is.

@donovaly
Copy link
Member Author

donovaly commented Dec 28, 2020

Well, I implemented it as I proposed and adapted the PR accordingly:


the common rule is that if a dimension has equal over- and undertolerance, they are output on the same line as the dimension concatenated using the ± character.

This PR does this.
(It also uses a more self-explaining function name.)

Note that this is not just cosmetics, it is even standardized in the GD&T norms. The issue is also described in the Wiki: https://wiki.freecadweb.org/TechDraw_Geometric_dimensioning_and_tolerancing#Formatting_2
also:

  • use one format specifier for tolerances since the norms don't allow a different format for the over- and the undertolerance
  • some code optimizations like a self-explaining function name, remove unnecessary/outdated comments

This is the output (contains the changes from #4203):

FreeCAD_ZouxKGoHKa

The benefits:

  • if the tolerance is equal, you can just input its value to the Tolerance property, if you have unequal tolerance values, you can enter them as always -> we achieve norm-conform output
  • existing documents will stay as they are since an explicit over- undertolerance leads to the two-line output
  • if people prefer the two-line output also for equal tolerances, they can get this by filling the over- undertolerance properties

I also fixed the issue that FC allows different form specifiers for the over- and undertolerance. It would be strange to have the overtolerance with e.g. %.2f and the undertolerance with %.1e. With this PR there is one format specifier used for both tolerances.

I think this is a handy and intuitive solution that minimizes the workload (only on value to input for equal tolerances and no time-consuming additional toggling of options.

@donovaly donovaly force-pushed the TD-equal-tolerances branch 2 times, most recently from 528bddc to 1f83dd3 Compare December 28, 2020 23:07
@wwmayer
Copy link
Contributor

wwmayer commented Dec 30, 2020

This way we enable the user to save time by entering only one value and keep the functionality to get the two-line output if he really wants. If you agree, I'll implement this.

IMO, this is an awkward workflow in some cases. It's fine as long as you only change the Tolerance property directly after creating a dimension. But it increases the work when switching from unilateral to bilateral tolerancing because you have to make sure that Over/UnderTolerance is 0 before the value of Tolerance is considered.

Also, due to the fact of three different tolerance types the logic is IMO not very intuitive for users to understand when which tolerance is active.

If you e.g. have a look at the scale function of GIMP you will see two spin boxes and in between a toggle button with the symbol of a chain link. If the chain link is locked the two scale values are always synchronized. This means if you modify the height value the width value is changed automatically and vice versa. If the chain link is unlocked you can freely change both values.

So, this idea applied to tolerances I suggest to use a boolean property where you can switch between unilateral and bilateral tolerancing.

@donovaly
Copy link
Member Author

IMO, this is an awkward workflow in some cases. It's fine as long as you only change the Tolerance property directly after creating a dimension. But it increases the work when switching from unilateral to bilateral tolerancing because you have to make sure that Over/UnderTolerance is 0 before the value of Tolerance is considered.

But how often do you have in real life that you first specified an equal tolerance and then change to an unequal one, maybe once every few drawings. Then, you need to set 2 values to zero. However, the main use case (about 80 - 90%) in a drawing - are equal, bilateral tolerances.
With my solution, you just have to enter your tolerance to the field that has this name and you are done - as quick as possible, no option can accidentally bet set wrong and existing drawings stay as they are, see also below.
What I should however do to improve the PR is to disable the Tolerance field when there are values in the Over/undertolerance fields.

If you e.g. have a look at the scale function of GIMP you will see two spin boxes and in between a toggle button with the symbol of a chain link. If the chain link is locked the two scale values are always synchronized. This means if you modify the height value the width value is changed automatically and vice versa. If the chain link is unlocked you can freely change both values.

That is what also @aapo proposed. But there is a major issue why I do not support this:

  • how do then get a two-line output for equal, bilateral tolerances? This is the output is what we have at the moment. If we don't provide this feature, all existing drawings would change - this would be a big issue because the tolerance feature with the two fields is in FC for a long time now.
    So we could make the chain option OFF by default, the existing drawings stay as they are, but then you will have in future for every equal tolerance to perform two actions: adding the value + setting the option. But for example some of my real-life drawings have dozens of tolerances and it always annoyed me that I have to spend so much time in getting them by filling 2 fields.

So the question is of we keep existing drawings or not. Changing existing drawings make them more norm-conform that is what I like, but I also know that "automatic changes" that can even not be reverted will cause problems in companies because every change in a drawing has to get a new revision (I think this is normed as well) no matter if it is a pure editorial change or not. So the danger is to open an existing drawing, press save because you just changed something in a model (e.g. a construction sketch) and without noticing you get a changed drawing.

To summarize:

  • having a chain option, when option by default OFF, existing drawings are preserved but all future drawings need 2 actions to get a tolerance
  • when option by default ON, existing drawings will be changed and it won't be possible to get the two-line output for equal tolerances (not-norm conform, you you might have your reasons)

Therefore I think my proposal is more safe and provides the minimal possible workload for future drawings and therefore the better compromise.

@donovaly
Copy link
Member Author

Hmm, I realize now that we already changed existing drawings by the changes we made during the last 2 weeks. Some of them only affect dimensions created with FC 0.19 but some, like the omission of the plus sign for unilateral tolerances. So then we can go on an implement the chain option and set it ON by default. Then exisiting drawings will get norm-conform equal tolerances and one has to perform only 1 action for new equal tolerances.
-> I'll change the PR accordingly the next days.

@wwmayer
Copy link
Contributor

wwmayer commented Jan 2, 2021

if he uses the same value for over- undertolerance, he gets the common ± output

(Cited from your first post.) I have compiled your PR and it does not do it. It still shows the values in two lines.

But how often do you have in real life that you first specified an equal tolerance and then change to an unequal one, maybe once every few drawings.

It's not about how often you do this. It's about when you do it how easy and logical it is to change the values.

Then, you need to set 2 values to zero.

Why? If the equal bilateral mode is active the user has to exactly change one value. With your approach the user has to change up to three values: set Over/UnderTolerance to 0 and then change Tolerance. And as already said for an average user it's not obvious at all when unilateral mode, unequal bilateral mode or equal bilateral mode is active.

Furthermore, Tolerance can accept arbitrary values. So, if it's set to a negative value the text will be: dimension ± -tolerance which I doubt is correct behaviour.

However, the main use case (about 80 - 90%) in a drawing - are equal, bilateral tolerances.

So, the equal bilateral mode can be activated for every newly created dimension. Then the user only has to change one value like with your PR but without the disadvantage of three different tolerance properties. To reduce possible confusions the UnderTolerance can be made read-only in the property editor if equal bilateral mode is active.

With my solution, you just have to enter your tolerance to the field that has this name and you are done - as quick as possible, no option can accidentally bet set wrong and existing drawings stay as they are, see also below.

I disagree. You implicitly expect equal bilateral mode if Over- and UnderTolerance are 0 and Tolerance is set. If for some reason Over- or UnderTolerance becomes non-zero (e.g. if bound to an expression that becomes non-zero due to round-off errors) the dimension automatically switches to unequal bilateral or unilateral mode.
If there was a property to explicitly force equal bilateral mode an accidental change won't be possible.

how do then get a two-line output for equal, bilateral tolerances?

If the boolean property to force equal bilateral mode is off but Over- and UnderTolerance are equal you could handle this situation, too. But I wonder why should this even be allowed as you said yourself that it violates the norm.

So we could make the chain option OFF by default, the existing drawings stay as they are, but then you will have in future for every equal tolerance to perform two actions: adding the value + setting the option. But for example some of my real-life drawings have dozens of tolerances and it always annoyed me that I have to spend so much time in getting them by filling 2 fields.

This can be very easily avoided. There are two methods to achieve this:

  1. First method
  • the constructor of the Dimension feature switches the option off
  • the commands to create a Dimension feature switch the option on
  1. Second method
  • the constructor of the Dimension feature switches the option on
  • the Restore() function of the Dimension feature can be overridden which switches the option off. Old project files lack of that property and it won't be changed when reading them in. For new project files the property will be changed to what's saved there.

@donovaly
Copy link
Member Author

donovaly commented Jan 2, 2021

if he uses the same value for over- undertolerance, he gets the common ± output

(Cited from your first post.) I have compiled your PR and it does not do it. It still shows the values in two lines.

Because I already changed that in favor of the solution to just fill the "Tolerance" field.

However, as I wrote in my last comment, I will change the PR now anyway since we are free to change existing drawings because we already did this lot during the FC 0.19 development cycle.

But how often do you have in real life that you first specified an equal tolerance and then change to an unequal one, maybe once every few drawings.

It's not about how often you do this. It's about when you do it how easy and logical it is to change the values.

I don't agree because let's calculate assuming every action takes e.g. 3 seconds to perform and you have 2 dozen (24) dimensions in a drawing:
0.9 * 24 * 1 action * 3s = 64.8 s
0.1 * 24 * 3 actions * 3s = 21.6 s
total: 86.4 s
but
0.9 * 24 * 2 actions * 3s = 129.6 s
0.1 * 24 * 1 action * 3s = 7.2 s
total: 136.8 s

So it is in my opinion very important that the use case that appears most often needs as less actions as possible.

@donovaly donovaly changed the title [TD] handle equal tolerances correctly [WIP] [TD] handle equal tolerances correctly Jan 2, 2021
@donovaly donovaly force-pushed the TD-equal-tolerances branch 2 times, most recently from b8b6a06 to 8b0bcc2 Compare January 3, 2021 03:40
@donovaly
Copy link
Member Author

donovaly commented Jan 3, 2021

-> I'll change the PR accordingly the next days.

I did so now. Now we have a new option "Equal Tolerance" set by default to true. Existing documents will thus change but as I wrote, we did so already and the benefit is a norm-conform layout also for existing drawings.

In case users still want the old output, they still can force this.

Here is a screenshot of all possible dimension layouts:
FreeCAD_omCppvPBnZ

@donovaly donovaly changed the title [WIP] [TD] handle equal tolerances correctly [TD] handle equal tolerances correctly Jan 3, 2021
@wwmayer
Copy link
Contributor

wwmayer commented Jan 4, 2021

I don't agree because let's calculate assuming every action takes e.g. 3 seconds to perform and you have 2 dozen (24) dimensions

I described two methods how to avoid to change two properties and instead only change one. So, it's rather 86.4 s vs 72.0 s

Now when looking at the latest code changes I am confused. You always said how important it is not to change the drawings when loading a project and now you don't pay any attention.
When I load a project where OverTolerance is set to +1 and UnderTolerance to -2 it suddenly shows +/- 1 in the drawing instead of +1 and -2 (which is clearly a bug).

The EqualProperty is set to True but UnderTolerance is not read-only (another bug) and I am able to change it. However, all the changes are ignored because EqualProperty is True.

This can be fixed with:

void DrawViewDimension::Restore(Base::XMLReader &reader)
{
    // In order to keep old project files working this option will be switchrf off.
    EqualTolerance.setValue(false);
    DrawView::Restore(reader);
}

the common rule is that if a dimension has equal over- and undertolerance, they are output on the same line as the dimension concatenated using the ± character.

This PR does this.

Note that this is not just cosmetics, it is even standardized in the GD&T norms.

also:
* use one format specifier for tolerances since the norms don't allow a different format for the over- and the undertolerance
* some code optimizations like a self-explaining function name, remove unnecessary/outdated comments
@donovaly
Copy link
Member Author

donovaly commented Jan 5, 2021

Now when looking at the latest code changes I am confused. You always said how important it is not to change the drawings when loading a project and now you don't pay any attention.

Yes, changing drawings is an issue in terms of QA. Every change deserves a new drawing revision. (I did not know this few weeks ago).

The point is that TD was full of small bugs here and there and the many fixes in the 0.19 development cycle changed existing drawings already a lot. OK, FC 0.19 was not released as being stable, but we had to introduce changes nevertheless. For example a machining company reported that my clearance hole diameters were wrong. That was when I realized that the Hole dialog did not apply norm-conform hole diameters. I hope I fixed the hole dialog but of course the new diameters (only slight changes, however) change existing drawings as well. So I already broke existing drawings.

When I load a project where OverTolerance is set to +1 and UnderTolerance to -2 it suddenly shows +/- 1 in the drawing instead of +1 and -2 (which is clearly a bug).

Yes.

void DrawViewDimension::Restore(Base::XMLReader &reader)

Thanks.
I thought, since existing drawings were changed already anyway, we can convert existing drawings but I see now that this is not possible for unequal, existing tolerances.
So I now added the void as you proposed.

@donovaly
Copy link
Member Author

donovaly commented Jan 5, 2021

A question: since tolerances are in many cases in sub-millimeter range, the stepsize for the tolerances should be 0.1 mm instead of 1.0mm. How can this be achieved without the need to set a range constraint for the App::Property?

By the way, when this PR is in, I will add a dialog to change dimensions. Surprisingly dimensions are the only TD feature without a dialog and while learning GD&T I noticed that a dialog would speed op the workflow a bit.

@donovaly
Copy link
Member Author

donovaly commented Jan 7, 2021

Just as reference, here is what e.g. the ASME norm Y14.5 2018 defines as layout:
AcroRd32_eASeHof6nu

Just for information, this is what the ISO 129-1 allows:
firefox_UzZ9Bg6ote

@donovaly
Copy link
Member Author

donovaly commented Jan 7, 2021

Just for information since this was also new to me:

UT and OT are supposed to define a range for a given value V which can be expressed as:
V + UT <= V <= V + OT

That can but not need to be. It is valid to have e.g. 8.0 +0.4 +0.1 Then the resulting value can be in the range 8.4 - 8.1. While googling around, I found sch drawings and apparently one sometimes don't like to dimension 8.25 +- 0.15.
Here is for example forum thread saying this is ISO-conform: https://www.pcdmisforum.com/forum/pc-dmis-enterprise-metrology-software/pc-dmis-for-cmms/1272-double-plus-tolerance-why
I read the ISO and they simply don't forbid this kind of tolerancing. However, we support this already, it is just for information.

@wwmayer
Copy link
Contributor

wwmayer commented Jan 8, 2021

The point is that TD was full of small bugs here and there and the many fixes in the 0.19 development cycle changed existing drawings already a lot.

If a drawing violates the standard then IMO it's OK to make changes that automatically fix the drawing. But if over and under tolerance is not equal bilateral then it should not change the drawing to show them as equal bilateral.

A question: since tolerances are in many cases in sub-millimeter range, the stepsize for the tolerances should be 0.1 mm instead of 1.0mm. How can this be achieved without the need to set a range constraint for the App::Property?

You can't. But one way is to set the lower and upper limit to DBL_MIN/DBL_MAX.

By the way, when this PR is in, I will add a dialog to change dimensions. Surprisingly dimensions are the only TD feature without a dialog and while learning GD&T I noticed that a dialog would speed op the workflow a bit.

Yes, I realized this as well. It would be a nice feature.

That can but not need to be. It is valid to have e.g. 8.0 +0.4 +0.1 Then the resulting value can be in the range 8.4 - 8.1.

But isn't this very odd? I mean you say the length dimension is 8mm but the value can be in the range 8.1-8.4 mm. Sounds a bit contradictory to me.

Here is for example forum thread saying this is ISO-conform: https://www.pcdmisforum.com/forum/pc-dmis-enterprise-metrology-software/pc-dmis-for-cmms/1272-double-plus-tolerance-why
I read the ISO and they simply don't forbid this kind of tolerancing. However, we support this already, it is just for information.

So, if the standard allows it then I don't mind (but still find it strange).

@wwmayer
Copy link
Contributor

wwmayer commented Jan 8, 2021

Merged.

@wwmayer wwmayer closed this Jan 8, 2021
@donovaly
Copy link
Member Author

donovaly commented Jan 8, 2021

If a drawing violates the standard then IMO it's OK to make changes that automatically fix the drawing. But if over and under tolerance is not equal bilateral then it should not change the drawing to show them as equal bilateral.

Yes, of course.
I learned now a lot and I will take care in future that existing drawings will not be changed without a very good reason.

You can't. But one way is to set the lower and upper limit to DBL_MIN/DBL_MAX.

Thanks, I will make a new PR to do this.

But isn't this very odd?

Yes. I won't do this by myself since as a physicist this is illogical. However, I read to learn.

@donovaly
Copy link
Member Author

donovaly commented Jan 9, 2021

You can't. But one way is to set the lower and upper limit to DBL_MIN/DBL_MAX.

Thanks, I will make a new PR to do this.

Here it is: #4263

@donovaly donovaly deleted the TD-equal-tolerances branch January 9, 2021 23:33
@donovaly
Copy link
Member Author

By the way, when this PR is in, I will add a dialog to change dimensions. Surprisingly dimensions are the only TD feature without a dialog and while learning GD&T I noticed that a dialog would speed op the workflow a bit.

Yes, I realized this as well. It would be a nice feature.

Here is the PR that adds the dialog: #4271

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants