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

TechDraw: Smart dimension tool #13525

Merged
merged 9 commits into from
May 13, 2024

Conversation

PaddleStroke
Copy link
Contributor

@PaddleStroke PaddleStroke commented Apr 18, 2024

Implement a smart dimension tool similar to Sketcher_Dimension. Fixes #12370
Please test and feedback.

By default it is set to both smart dimension + old tools. See preference in TechDraw/Dimension. If you select 'single tool' then it looks like this :
image

  • also improve the 'arc length' tool so that it works with any kind of geometries : Arc of ellipse / bspline ... making ExtensionArcLengthAnnotation kind of useless. Though that tool works with multiple edges but as it does not show where it starts and end that wouldn't be very helpful.

  • Implement Area dimension. (current tool is balloon not a real dimension)

@github-actions github-actions bot added Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD WB TechDraw Related to the TechDraw Workbench WB Sketcher Related to the Sketcher Workbench labels Apr 18, 2024
@luzpaz

This comment was marked as resolved.

@maxwxyz
Copy link
Collaborator

maxwxyz commented Apr 19, 2024

Does it need a preselection?
You could use the dimension icon from sketcher in yellow for this tool.

@PaddleStroke
Copy link
Contributor Author

PaddleStroke commented Apr 19, 2024

It works like sketcher tool, you activate it then you have a continuous mode. However it actually currently do not work with preselection. Let me fix that.
And yes good idea for the icon I'll do that.

Note @WandererFan , this introduce 'Tool handlers' class in techdraw which are same as the sketcher tools. This is good to handle tools that have "tool->selection" workflow such as the balloon annotation. When I was doing this PR I noticed baloon annotation is hardcoding things in the QGVPage.cpp, such as setting balloon cursor. I think after this PR merges, balloon can be changed to use the new tool handler.

@PaddleStroke
Copy link
Contributor Author

PaddleStroke commented Apr 19, 2024

I have also integrated arc length now.
Could also be added :

  • landmark?
  • Area ?
  • Edge length ?

@WandererFan
Copy link
Contributor

If you mean "landmark dimension", I'm not sure if that is widely used. It was an early attempt to mitigate TNP for dimensions. I'd leave it out for now.

Would like a proper Area Dimension. The one in the Extension tools is a static annotation that doesn't get updated when the face changes. I don't think it handles holes properly either.

I don't have a need for edge length, but some might.

@WandererFan
Copy link
Contributor

Note that the chain & ordinate dimensions in the Extension tools (red icons) have issues with proper alignment and updating. Making a proper "dimension group" for related dims is on my todo list and has been for a long time.

@PaddleStroke
Copy link
Contributor Author

Ok, I'm creating an area dimension now then.

@PaddleStroke
Copy link
Contributor Author

image

@PaddleStroke PaddleStroke marked this pull request as ready for review April 22, 2024 13:30
@PaddleStroke PaddleStroke force-pushed the td_dimension branch 2 times, most recently from 95a3a38 to 4e6e9c9 Compare April 22, 2024 15:00
@pierreporte
Copy link

@PaddleStroke Arrows pointing in an area should end by a bullet by default. Pointed arrows are only for lines and points.

@WandererFan
Copy link
Contributor

Missing an include for QTimer? Also some warnings about unused variables.

@PaddleStroke
Copy link
Contributor Author

@WandererFan thanks I fixed those. I'll check the CI when its done to make sure there are no more things.

@pierreporte that would need (potentially) a substantial amount of additional work. So can you discuss with DWG to make sure every one agree and offer a mockup for the shape. Thanks

@pierreporte
Copy link

@PaddleStroke This is standard drawing practice, not UI/UX. In Guide du dessinateur industriel (A. Chevalier), there is the following example in the common mistakes section. TechDraw already has an acceptable dot terminal available (it could be a bit smaller though to match arrow heads width).

image

@maxwxyz
Copy link
Collaborator

maxwxyz commented Apr 23, 2024

can confirm @pierreporte

@maxwxyz
Copy link
Collaborator

maxwxyz commented Apr 24, 2024

@PaddleStroke Point and arrow are defined in ISO 129-1
grafik

@PaddleStroke
Copy link
Contributor Author

Got it its clear enough. Well I'll try to pack this in this PR then.

@PaddleStroke
Copy link
Contributor Author

Ok it was not too hard to change to point actually.
I also fixed an issue with the US units that were wrong (see above screenshow showing 145 in² instead of 5)
image

@maxwxyz
Copy link
Collaborator

maxwxyz commented Apr 27, 2024

@PaddleStroke I think default to single tool as it is also in sketcher is good, if all tools in the group are possible with the new tool. Saves a lot of toolbar space and you still have all tools in the dropdown.

@PaddleStroke
Copy link
Contributor Author

The default is very easy to change, it's like 2 lines of code to change. And for early stage of the tool I prefer to let as separate to avoid having to deal with haters. When its confirmed to be stable anyone can change the default by a tiny PR.

@yorikvanhavre
Copy link
Member

I'll let @WandererFan press the button ;)

@WandererFan WandererFan merged commit c12e1f8 into FreeCAD:main May 13, 2024
9 checks passed
@FEA-eng
Copy link
Contributor

FEA-eng commented May 14, 2024

It's a great tool. Will it support thickness in the future (when I press M when adding area annotation, nothing happens)?

@PaddleStroke
Copy link
Contributor Author

What is thickness? Is there already a thickness dimension tool?
Is it extent? Should a face selection offer the extent as a mode?

@FEA-eng
Copy link
Contributor

FEA-eng commented May 14, 2024

What is thickness? Is there already a thickness dimension tool? Is it extent? Should a face selection offer the extent as a mode?

I mean what pierreporte showed here: #13525 (comment)

Currently, it has to be added manually as an annotation. It would be nice to have it as a mode for face selection (in addition to area) but the problem is that the value may vary even for a single face and it might be difficult to compute it. But the mode could only consider faces with constant thickness (3rd dimension).

Perhaps I should create a feature request for this.

@pierreporte
Copy link

@FEA-eng I think it should show the distance at the selected point. This kind of dimension shouldn’t be used for variable thicknesses anyway. It could change value when the point is modified. The point could be attached to a cosmetic point if a precise location is needed.

@FEA-eng
Copy link
Contributor

FEA-eng commented May 14, 2024

@FEA-eng I think it should show the distance at the selected point. This kind of dimension shouldn’t be used for variable thicknesses anyway. It could change value when the point is modified. The point could be attached to a cosmetic point if a precise location is needed.

Right, the thickness where the user clicks will be sufficient. I made a new request to document this: #14012

@belinski-roman
Copy link

belinski-roman commented May 15, 2024

Теперь я также интегрировал длину дуги. Также можно добавить:

  • ориентир?
  • Площадь?
  • Длина кромки ?

elevation marks for architectural drawings
image021
msg-1002030191950-8464

@belinski-roman
Copy link

belinski-roman commented May 15, 2024

The default hotkey for this tool should be assigned the same as in Sketcher for the dimensional tool - "D"

@maxwxyz
Copy link
Collaborator

maxwxyz commented May 15, 2024

please create new issues and describe the problem there. TIA

@Reqrefusion
Copy link
Contributor

Reqrefusion commented May 19, 2024

@PaddleStroke TechDraw_Dimension_Pointer.svg and TechDraw_Dimension.svg What is the difference? Same icons but sized differently. Is this nuance difference meaningful?

@maxwxyz
Copy link
Collaborator

maxwxyz commented May 19, 2024

@Reqrefusion pointer is the cursor icon which is shown when the tool is active.

@PaddleStroke
Copy link
Contributor Author

The cursor has a small white cross that acts as the mouse pointer.

@Cobras62
Copy link

The cursor has a small white cross that acts as the mouse pointer.

Hello,
Why a small white cross? Can we change the colour of this cross? On a white background, it's not very visible...
Thank you!

@maxwxyz
Copy link
Collaborator

maxwxyz commented May 21, 2024

@Cobras62
Copy link

@maxwxyz : yes it is this version that I tested. But the white cross on the white background is difficult to see. Just look at the animation that presents this tool in the 1.0 releases notes and we realize that the white cross is not obvious :)

@PaddleStroke
Copy link
Contributor Author

PaddleStroke commented May 21, 2024 via email

@maxwxyz
Copy link
Collaborator

maxwxyz commented May 22, 2024

Ob my system it looks dark gray.

@Cobras62
Copy link

@PaddleStroke : ok thanks for this info :)
In any case, this future v1.0 and these new tools are a great piece of work !
Well done to everyone !

@Cobras62
Copy link

@maxwxyz : A friend who works in a design office has tested it and it's white for him too. We're both running Windows 11.

@PaddleStroke
Copy link
Contributor Author

I just checked and the color is indeed a preference.

    ParameterGrp::handle hGrp =
        App::GetApplication().GetParameterGroupByPath("User parameter:BaseApp/Preferences/View");
    color = hGrp->GetUnsigned("CursorCrosshairColor", color);

It is currently defined in the shared class ToolHandler, which is shared between sketcher and techdraw. So we need to change this to have separate colors. Please open an issue I'll do this later.

@Cobras62
Copy link

@PaddleStroke : I've just opened an issue. It's the first time I've done it, I hope I haven't done anything wrong ^^.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Issue or PR touches core sections (App, Gui, Base) of FreeCAD WB Sketcher Related to the Sketcher Workbench WB TechDraw Related to the TechDraw Workbench
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Problem] TechDraw doesn't have a 'smart dimension' tool
10 participants