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

[FEM] Add reference highlighting for displacement constraints #5391

Closed

Conversation

AjinkyaDahale
Copy link
Contributor

@AjinkyaDahale AjinkyaDahale commented Jan 17, 2022

This PR when ready is intended to highlight the geometries on which a particular FEM constraint is applied, when in the constraint's taskview.

Forum post: https://forum.freecadweb.org/viewtopic.php?f=18&t=65412.

@github-actions github-actions bot added the WB FEM Related to the FEM Workbench label Jan 17, 2022
@AjinkyaDahale
Copy link
Contributor Author

@wwmayer tagging you since you recently created the class ReferenceHighlighter that I am using here, and which I'm not able to import properly.

@AjinkyaDahale
Copy link
Contributor Author

Possibly fixed the issue by adding PartDesignGui to the libraries in /Mod/Fem/Gui/CMakelists.txt.

@AjinkyaDahale AjinkyaDahale force-pushed the fem-constrain-qol-2 branch 3 times, most recently from 310966b to e38a27a Compare January 18, 2022 06:20
@wwmayer
Copy link
Contributor

wwmayer commented Jan 18, 2022

wwmayer tagging you since you recently created the class ReferenceHighlighter that I am using here, and which I'm not able to import properly.

Making FemGui to depend on PartDesignGui is not unproblematic because the more dependencies we add between modules the longer the build process will take. This is because the build system would have to wait for PartDesignGui to be finished before it can start with FemGui.

Now when looking at the implementation of ReferenceHighlighter there is actually nothing that would forbid it to move it to the Part module. And since it does not even use any particular GUI stuff it could be even moved to Part/App.
I once put the files to PartDesign/Gui because I only needed it there for several view providers.

@AjinkyaDahale
Copy link
Contributor Author

Now when looking at the implementation of ReferenceHighlighter there is actually nothing that would forbid it to move it to the Part module.

That thought did occur to me trying to fix the issue. Do you want to make the move yourself? Otherwise I can make a PR to precede this one.

And since it does not even use any particular GUI stuff it could be even moved to Part/App.

I would vote against that, since this appears to be a very GUI-centric item and the dependency on Part/Gui already exists.

@wwmayer
Copy link
Contributor

wwmayer commented Jan 18, 2022

OK I will move it to Part/Gui, then.

@AjinkyaDahale
Copy link
Contributor Author

OK I will move it to Part/Gui, then.

Thank you. Could you also add the methods to highlight vertices? Some FEM constraints can work on vertices.

@wwmayer
Copy link
Contributor

wwmayer commented Jan 18, 2022

See af868ed

@wwmayer
Copy link
Contributor

wwmayer commented Jan 18, 2022

Could you also add the methods to highlight vertices? Some FEM constraints can work on vertices.

What exactly do you mean?

@AjinkyaDahale
Copy link
Contributor Author

Could you also add the methods to highlight vertices? Some FEM constraints can work on vertices.

What exactly do you mean?

Like say ReferenceHighlighter::getVertexColors(subSet.second, colors).

@wwmayer
Copy link
Contributor

wwmayer commented Jan 18, 2022

Ah, OK.

@AjinkyaDahale
Copy link
Contributor Author

I think this is ready for review apart from a few kinks, which I can add as additional commits.

@AjinkyaDahale AjinkyaDahale marked this pull request as ready for review January 18, 2022 18:39
@wwmayer
Copy link
Contributor

wwmayer commented Jan 18, 2022

Here we go: 74fe1fa

But note the code is not tested yet because I didn't have a sandbox to test it. The function getVertexColors() is supposed to accept the sub-elements Vertex, Edge, Wire and Face. So the array of string can e.g. be: {Vertex1, Vertex2, Edge4, Wire1, Face7}

@AjinkyaDahale
Copy link
Contributor Author

Here we go: 74fe1fa

Thanks. Rebased.

But note the code is not tested yet because I didn't have a sandbox to test it.

I should be able to try it with this PR's branch.

The function getVertexColors() is supposed to accept the sub-elements Vertex, Edge, Wire and Face. So the array of string can e.g. be: {Vertex1, Vertex2, Edge4, Wire1, Face7}

Sounds useful, albeit I can't try it with this PR. Why are we not including Shells and Solids, though?

Following constraints affected:

`ViewProviderFemConstraint`
`ViewProviderFemConstraintDisplacement`
`ViewProviderFemConstraintFixed`
`ViewProviderFemConstraintFluidBoundary`
`ViewProviderFemConstraintForce`
`ViewProviderFemConstraintHeatflux`
`ViewProviderFemConstraintPressure`
`ViewProviderFemConstraintSpring`
`ViewProviderFemConstraintTemperature`
@wwmayer
Copy link
Contributor

wwmayer commented Jan 19, 2022

Why are we not including Shells and Solids, though?

Because shells and solids are not supported by the selection mechanism.

@wwmayer
Copy link
Contributor

wwmayer commented Jan 19, 2022

Merged as ec00d35

@wwmayer wwmayer closed this Jan 19, 2022
@AjinkyaDahale
Copy link
Contributor Author

Merged as ec00d35

Thanks. I see you made some changes in PROPERTY_SOURCE for the cpp's. I tried the same, but I think I was having errors opening any file after that. Were you able to fix that? If yes how?

@wwmayer
Copy link
Contributor

wwmayer commented Jan 22, 2022

but I think I was having errors opening any file after that

I have opened the project files from the examples directory without problems.

Were you able to fix that? If yes how?

Changing the inheritance of the types shouldn't cause any problems w.r.t. existing project files because this information is not stored inside the Document.xml/GuiDocument.xml there.

In case you can reproduce problems then please open a forum topic and ping me there.

@AjinkyaDahale
Copy link
Contributor Author

@wwmayer thanks for the reply

In case you can reproduce problems then please open a forum topic and ping me there.

I sure will. So far I cannot reproduce the issue but you'll be the first to know if I do.

@AjinkyaDahale AjinkyaDahale deleted the fem-constrain-qol-2 branch February 26, 2022 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WB FEM Related to the FEM Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants