(Geo)Group behavior changes #2759

realthunder · 2019-12-05T07:52:22Z

Thess patches are split out from #2723

Please refer to this post for description of group changes.

haraldg · 2020-06-09T11:33:22Z

Hi, I'd like to test this, as is supposedly fixes a rather important issue in Arch. (Easy to work around but very confusing to new people.)

Can you rebase and resolve the merge conflicts?

realthunder · 2020-06-09T13:24:14Z

Can you rebase and resolve the merge conflicts?

Done. Please try.

haraldg · 2020-06-17T22:32:34Z

Hi @realthunder, I'm seeing an issue that I believe is related to this PR:
In the Arch WB, when drawing a wall directly (ie just click the wall tool and draw the base line), I get the following error message:
410.371 <App> Document.cpp(3500): Unbenannt#WallTrace still touched after recompute

This PR is the only change I have on top of master, so I guess it is responsible.

realthunder · 2020-06-17T22:35:22Z

Is it possible to attach a file showing the problem?

haraldg · 2020-06-17T22:42:45Z

Github tells me "We don't support that file type" for an .FCStd, so I have renamed it to .zip, but it is utterly trivial anyway: Just an empty file with a single wall.

Wall.zip

realthunder · 2020-06-17T23:18:32Z

The problem is caused by not fixing direction of the line segment in WallTrace. It behaves the same in upstream.

realthunder · 2020-06-17T23:20:37Z

BTW, ArchWall is not a group. So this patch shouldn't have any effect on it. It is more relevant to ArchBuildingPart, I guess.

haraldg · 2020-06-18T09:06:46Z

That's strange, I never saw this before. I need to compile current master again ...
Thanks for your input.

haraldg · 2020-07-19T19:04:45Z

I have used this branch now for a all my work for a full month. I can confirm that it fixes a nasty problem for the Arch WB. I didn't run into any regressions. I'm switching back to master now, as the branches have conflicts again and it is unlikely that my testing will uncover any regressions in the future.

I'm sorry that I don't have the insight into the code base to do a code review, but I'm looking forward to when this gets merged.

berndhahnebach · 2020-08-04T05:21:05Z

link to the forum post with the nasty bug described ...
https://forum.freecadweb.org/viewtopic.php?f=23&t=38779&start=20#p340174

link to this PR as a fix:
https://forum.freecadweb.org/viewtopic.php?f=23&t=38779&start=50#p369903

pwnorbitals · 2021-02-01T19:41:26Z

Is there a blocking issue with this PR ? Not sure why we don't merge (after rebase/resolve)

matthijskooijman · 2021-05-19T20:23:18Z

src/App/GroupExtension.cpp

            }
        }
    }

    App::Extension::extensionOnChanged(p);
 }

-void GroupExtension::slotChildChanged(const DocumentObject &obj, const Property &prop) {
-    if(&prop == &obj.Visibility && !_togglingVisibility)


Reading this code, I wonder: Shouldn't this &prop == &obj.Visibility be kept? I'm not entirely sure, but it looks like the below handler only has code for when a child's visibility changes? Or is this changed handler somehow limited to only Visibility changes now?

Yes, it is limited to Visibility. A new signalChanged has been added to every property. This slotChildChanged is now hooked to child objects' Visibility.signalChanged, so no need to check for property anymore.

Ah, I see it was in an earlier commit (57e2164), I was looking just in the same commit. Ok, looks good, thanks for clarifying!

matthijskooijman · 2021-05-20T14:06:38Z

src/App/ExtensionContainer.h

+            }
+        }
+        return false;
+    }


I really like these helpers, great for readability of the other code. However, I would personally put these helpers, along with modifications to use them in existing code, in a separate commit, which would then make no functional changes, just optimize the existing code, making both resulting commits easier to review. It's of course not up to me and I believe that the form/separation of these PRs has already been discussed a lot, but just wanted to add my 2c here :-)

berndhahnebach · 2021-09-24T11:50:42Z

Following a link to the branch on the CI-repository:

https://gitlab.com/freecad/FreeCAD-CI/-/commits/PR_2759

The CI-status is available on the latest commit of the branch.
If there is no status available the PR should be rebased on latest master.
Check pipeline branches to see if your PR has been run by the CI.

https://gitlab.com/freecad/FreeCAD-CI/-/pipelines?scope=branches

berndhahnebach · 2021-09-25T11:07:51Z

Would it be possible to rebase on latest Master? Cheers bernd

realthunder · 2021-09-25T12:19:04Z

Yes, sure. I'll rebase all of my PRs. But since there are quite a few of them. It'll take some time.

donovaly · 2022-01-06T23:41:12Z

can you please rebase to merge the PR?

approved once by @yorikvanhavre .
@chennes , @wwmayer any concerns from your side?

donovaly · 2022-02-19T18:40:31Z

Will this PR become part of the planned Toponaming inclusion branch?

pwnorbitals · 2022-03-20T14:50:23Z

@realthunder pinging for rebase as I think it's a very interesting PR. Thanks ! :)

berndhahnebach · 2022-07-05T20:17:34Z

for feature branch PR_2759. Pipeline 574892903 was triggered at 193d88e. All CI branches and pipelines.

realthunder · 2022-11-03T22:25:57Z

PR suspended for now, and will resume after topo naming merge.

maxwxyz · 2024-04-27T05:51:51Z

Is this PR also ready to be resumed after the recent toponaming merges?

realthunder mentioned this pull request Dec 5, 2019

Plain/Geo group behavior change and related 3D visualization/selection enhancement #2723

Closed

realthunder force-pushed the GroupChange branch from 3f25965 to d7e4996 Compare December 31, 2019 03:58

yorikvanhavre approved these changes Feb 5, 2020

View reviewed changes

realthunder force-pushed the GroupChange branch from 3d73812 to d7701d0 Compare February 18, 2020 00:49

realthunder force-pushed the GroupChange branch from d7701d0 to 8dae676 Compare February 26, 2020 12:20

realthunder force-pushed the GroupChange branch from 8dae676 to 5b0aad5 Compare March 9, 2020 12:17

realthunder force-pushed the GroupChange branch from 5b0aad5 to 658ed53 Compare March 26, 2020 06:38

realthunder force-pushed the GroupChange branch from 658ed53 to 7ea00b2 Compare June 9, 2020 13:23

wwmayer added the For 0.20 label Feb 11, 2021

matthijskooijman reviewed May 19, 2021

View reviewed changes

matthijskooijman reviewed May 20, 2021

View reviewed changes

berndhahnebach added Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD and removed For 0.20 labels Sep 17, 2021

FreeCAD deleted a comment from freecadci Jul 5, 2022

luzpaz added the ✋ On hold This PR must not be merged before some condition is met label Nov 15, 2022

luzpaz marked this pull request as draft November 15, 2022 03:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

(Geo)Group behavior changes #2759

(Geo)Group behavior changes #2759

realthunder commented Dec 5, 2019

haraldg commented Jun 9, 2020

realthunder commented Jun 9, 2020

haraldg commented Jun 17, 2020

realthunder commented Jun 17, 2020

haraldg commented Jun 17, 2020

realthunder commented Jun 17, 2020

realthunder commented Jun 17, 2020

haraldg commented Jun 18, 2020

haraldg commented Jul 19, 2020

berndhahnebach commented Aug 4, 2020 •

edited

pwnorbitals commented Feb 1, 2021

matthijskooijman May 19, 2021

realthunder May 20, 2021

matthijskooijman May 20, 2021 •

edited

matthijskooijman May 20, 2021

berndhahnebach Sep 25, 2021

berndhahnebach commented Sep 24, 2021 •

edited

berndhahnebach commented Sep 25, 2021

realthunder commented Sep 25, 2021

donovaly commented Jan 6, 2022

donovaly commented Feb 19, 2022

pwnorbitals commented Mar 20, 2022

berndhahnebach commented Jul 5, 2022

realthunder commented Nov 3, 2022

maxwxyz commented Apr 27, 2024

(Geo)Group behavior changes #2759

Are you sure you want to change the base?

(Geo)Group behavior changes #2759

Conversation

realthunder commented Dec 5, 2019

haraldg commented Jun 9, 2020

realthunder commented Jun 9, 2020

haraldg commented Jun 17, 2020

realthunder commented Jun 17, 2020

haraldg commented Jun 17, 2020

realthunder commented Jun 17, 2020

realthunder commented Jun 17, 2020

haraldg commented Jun 18, 2020

haraldg commented Jul 19, 2020

berndhahnebach commented Aug 4, 2020 • edited

pwnorbitals commented Feb 1, 2021

matthijskooijman May 19, 2021

Choose a reason for hiding this comment

realthunder May 20, 2021

Choose a reason for hiding this comment

matthijskooijman May 20, 2021 • edited

Choose a reason for hiding this comment

matthijskooijman May 20, 2021

Choose a reason for hiding this comment

berndhahnebach Sep 25, 2021

Choose a reason for hiding this comment

berndhahnebach commented Sep 24, 2021 • edited

berndhahnebach commented Sep 25, 2021

realthunder commented Sep 25, 2021

donovaly commented Jan 6, 2022

donovaly commented Feb 19, 2022

pwnorbitals commented Mar 20, 2022

berndhahnebach commented Jul 5, 2022

realthunder commented Nov 3, 2022

maxwxyz commented Apr 27, 2024

berndhahnebach commented Aug 4, 2020 •

edited

matthijskooijman May 20, 2021 •

edited

berndhahnebach commented Sep 24, 2021 •

edited