Changes to do not hide comments with a custom style when hide constraints enabled (#1070)

[mafdezmoreno]

This PR attempts to achieve the requested changes in the Issue #1070

This is my first contribution to an open source project. Sorry in advance if I'm doing something wrong.

Changes

To evaluate if the comment has to be hidden, a check is made to see that the comment has a custom style and the "show this objects on screen" option is enabled.

This check is done at the beginning of the method responsible for evaluating visibility.

Tests

With "show this objects on screen" enabled:

With "show this objects on screen" disabled:

System information:

• macOS Sonoma 14.2.1 Apple M1

[phkahler]

This leaves the existing code for checking the individual style visibility alone (and extraneous) and I would have expected that to be removed. But then looking at it, there are other considerations. Look further down that function below this line:

if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_SHOW_ALL || isDim ) {

The visibility check there also depends on the group visibility. I think what you might want to do is modify the original early-exit code to only apply to non-custom constraints and leave the existing toggle logic for custom constraints where it is.

Also, give it a minute to see if anyone else has something to say about this. I've never looked at this part of the code and I haven't used custom styles ;-)

[mafdezmoreno]

Thanks @phkahler for your feedback, it has been a great help to me.

After rereading the desired behaviour in #1070 and your comments I have adapted the behaviour of the custom styled comments, with "show these objects on screen" enabled:

• Not shown with "show only dimensions" option active.
• Not shown if the group is disabled.
• Shown in "Hide Constrains and dimensions" and "Show constrains and dimensions" modes.

The behaviour to hide the comment is still maintained with "show these objects on screen" disabled and is not shown in any case.

Below is a recording to show the behaviour achieved:

Toggling "Show constrains and dimensions"/Hide Constrains and dimensions"/"Show constrains and dimensions"

Toggling enable/disable group:

[phkahler]

I'm not sure what the expected behavior of the different things is supposed to be. The toolbar 3-way and the checkbox for the style give 6 possibilities.

I think this part:

    if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_NOSHOW){
        if (type == Type::COMMENT && disp.style.v >= Style::FIRST_CUSTOM) {
            evalShowCustomComment = true;
        } else {
            return false;
        }
    }

Might be better written like this with no new flag:

    if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_NOSHOW){
        if (type == Type::COMMENT && disp.style.v < Style::FIRST_CUSTOM) {
            return false;
        }
    }

In other words the existing behavior of SCM_NOSHOW would simply not apply to custom styled comments. I'm not sure if that's the expected behavior of the request or not so I'll have to wait for more feedback.

[ruevs]

I'll take a look.

[ruevs]

If we decide to keep the functionality as implemented in this PR I would suggest the following implementation:

bool Constraint::IsVisible() const {
    bool keep = false;
    if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_NOSHOW){
        if (type == Type::COMMENT && disp.style.v >= Style::FIRST_CUSTOM) {
            keep = true;
        } else {
            return false;
        }
    }

    if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_SHOW_DIM)
        switch(type) {
        case ConstraintBase::Type::ANGLE:
        case ConstraintBase::Type::DIAMETER:
        case ConstraintBase::Type::PT_PT_DISTANCE:
        case ConstraintBase::Type::PT_FACE_DISTANCE:
        case ConstraintBase::Type::PT_LINE_DISTANCE:
        case ConstraintBase::Type::PT_PLANE_DISTANCE: keep = true; break;
        default:;
        }

    if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_SHOW_ALL || keep) {
        Group *g = SK.GetGroup(group);
        // If the group is hidden, then the constraints are hidden and not

Whether this is the desired behgaviour is harder to say. If we keep the logic above we end up with NO view where ONLY the geometry is visible. In my opinion this is not good.

@symbian9 (ghost) who opened #1070 is not around any more to tell us what he really wanted, but I think his idea was to be able to format some comment constraints with a custom style and keep them when all the "clutter" is hidden - thus allowing to create "good looking" views of drawings for export.

With the above in mind I think that it is better to show the comments in the "Show only dimensions" (SCM_SHOW_DIM) mode but NOT show them in the "Hide constraints and dimensions" mode. In that case the code will become:

bool Constraint::IsVisible() const {
    if(SS.GW.showConstraints != GraphicsWindow::ShowConstraintMode::SCM_NOSHOW){
        return false;
    }

    bool keep = false;
    if(SS.GW.showConstraints == GraphicsWindow::ShowConstraintMode::SCM_SHOW_DIM)
        switch(type) {
        case ConstraintBase::Type::ANGLE:
        case ConstraintBase::Type::DIAMETER:
        case ConstraintBase::Type::PT_PT_DISTANCE:
        case ConstraintBase::Type::PT_FACE_DISTANCE:
        case ConstraintBase::Type::PT_LINE_DISTANCE:
        case ConstraintBase::Type::PT_PLANE_DISTANCE: keep = true; break;
        case ConstraintBase::Type::COMMENT:
            if (type == Type::COMMENT && disp.style.v >= Style::FIRST_CUSTOM) {
                keep = true;
            }
            break;
        default:;
        }

Or we could always show them, but I do not like this for the reason I mentioned above:

bool Constraint::IsVisible() const {
    if(SS.GW.showConstraints != GraphicsWindow::ShowConstraintMode::SCM_NOSHOW){
        return false;
    }

    bool keep = false;
    if (type == Type::COMMENT && disp.style.v >= Style::FIRST_CUSTOM) {
        keep = true;
    }

[ruevs]

Bottom line: we should think about this some more an collect feedback from users (how?).

[mafdezmoreno]

Thank you @ruevs for your review. In my last commit I've implemented the suggested changes (enabled custom style comments are shown in "Show constrains and dimensions" and "Hide constrains a dimensions" modes).

Regarding the functionality requested by @symbian9, I also consider that it is not ideal to show the comment in all modes, but I would like to have an additional "Hide all" mode, in order to avoid distractions during the design of the parts.

I'm open to try to implement the mentioned functionality if the users find it useful.