-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Core / Measure: Introduce QuickMeasure #12217
Core / Measure: Introduce QuickMeasure #12217
Conversation
a89f30f
to
bcd20fb
Compare
There seem to be a couple of small bugs here: in my testing, parallel edges don't show their distance, nor do two selected points. |
bcd20fb
to
5d24167
Compare
Bugs are fixed. Order in status bar is fixed too. |
5d24167
to
06ec5c7
Compare
@chennes I fixed the end of files and changed to enum class. |
The CI is failing for MacOS but I can't understand why. Any idea @chennes ? |
Don't worry about it, that CI has been giving us problems quite frequently due to some sort of timeout. I believe @adrianinsaval in aware of the issue. |
I'll see if I can manage to reproduce the CI problem on the m1 mac once I get home, for now first just try re triggering the CI if you see mac failing every now and then |
@hlorus - comments re umf work in progress? |
UMF doesn't utilize the Measurement class in Measurement.h, so conflicts should be minimal. However, there might be some code duplication, which would probably be nice to avoid. |
Ah indeed we are bound to have conflicts here, as for a start we both introduce the Measure/Gui module. |
I would like to comment on putting the measurements on the bottom or top of the screen. I wrote the QuickMeasure Add-on and had the output going to the status bar. I found it awkward to move my eyes from what I was measuring to the status line and then back up to the part, I found it easier to create the existing text box and put it by what I was measuring, thereby keeping my eyes on the part and on the output. This is copied from pull request #12049 which was closed before I wrote to it. |
bfe8826
to
0986925
Compare
|
@DanMiel I am commenting on the code submitted in this pull request, naturally :) |
ad232f4
to
fd12675
Compare
The unified measurement tool is merged now, could this PR be completed now? |
fd12675
to
ea74f6c
Compare
Rebased on main after the Unified measurement tool merged. |
I could see a slowdown when changing selection in big documents with complex geometry if always the volume/area is calculated (just selecting/organizing items in the tree view). Maybe selection in the tree view could be excluded? |
I think the problem is that the function that gets the volume/area from the shape is a bit slow because its computing them everytime its called. This also causes some annoying issues in assembly and MBD as well. I'll open an issue about this. |
@PaddleStroke could we comment out or deactivate COM and Volume for now? Most of the time when selecting objects in the tree view the user is not interested in these measures. When it's needed, the new measurement tool can provide them. COM would also be more relevant if a coordinate system or datum point could be created at it's location. |
Yes I think we are going to disable area / volume for solids because there are no obvious way to solve this. And as you say volume/area are not so often needed. When it is the UMT can be used. |
…use it's slowing down selection.
@maxwxyz can you please test and feedback if everything is fine now? |
looks good! |
Looks to me as this code makes no use or almost of the new measurement framework. Am I correct ? |
Well, the framework was merged to main only last week. The Quick Measure feature developed over main is months old. |
Framework dev is available as a PR for months so doesn't look as a sensible argument. |
Depending on unmerged patch sounds risky, but now that both are in main, code can be deduplicated |
Why are square millimeters written as mm^2 and not as mm²? |
Depending on self-coded things is even more. Trust me on this. 😄 |
I based QuickMeasure on the pre-existing Measurement class and extended it. It seems UMT is introducing measurement document objects for each type of measurement. Quickmeasure does not need to create objects. So it can hardly reuse UMT framework. However it stands that there is duplication of the functions calculating the values (length, area...). But then I would say they do not belong to the measure document objects, as one (such as quickmeasure) can need to calculate measures without needing to create document objects. So those would belong to the pre-existing measurement class, where quickMeasure is using them. |
I don't think so. When you use the tool, the result is displayed in the Task panel without any object created. 😉 EDIT: @PaddleStroke I'm wrong here, I just checked and an object is created, then discarded if the measurement isn't validated. |
The tool is causing some severe problems: https://forum.freecad.org/viewtopic.php?t=87511 |
Introduce quick measure. This adds a small label on the status bar on the right side that gives basic information about selection :
Also as a small thing, it makes the text of the status bar label selectable so you can select it and copy it.
Fixes #12049