Proposal for draft modifiers to work in draft edit mode

[carlopav]

Not sure about double clicking, as this prohibits the ability to select multiple objects for draft edit mode. Currently, I have draft edit mode set to the shortcut "DE" which is very fast to press on the left hand side of the keyboard. I like the idea of escape being the consistent way to end a command.

+1

I'd like to work with @carlopav to get his editing node handle code showing up when draft mode is enabled

+1

[Moult]

You can now copy individual subelements instead of the entire object.

PR for reference: https://github.com/FreeCAD/FreeCAD/pull/1975

[Moult]

The rotate command now works with subelements. You can now rotate individual vertices or edges within a draft wire. It is also possible to rotate and copy at the same time.

PR for reference: https://github.com/FreeCAD/FreeCAD/pull/1975

[yorik]

Not sure about double clicking, as this prohibits the ability to select multiple objects for draft edit mode.

Indeed with your new move tool this would be a problem.

Great work!!

[Moult]

Ability to scale subelements now implemented.

Quote from the PR:

The last commit basically rounds out this PR in terms of implementing features. It is now possible to:

- For move, rotate, or scale, choose to modify one or more subelements (vertices / edges) instead of an entire object
- Highlight a draft object so that it is easier to see / select, even if it is invisible or a base of another component
- Tab-preselect draft objects to edit them easier with preselection highlighting
- Fixes the regular scale function which seemed to be just broken (didn't bother splitting out into another PR as I'm refactoring that area anyway, so as to prevent a complex merge conflict)

There is more polish which can be done (in particular, the scale UI code, which is special), as well as more features to be added (array subelements? offset subelements? etc), but I think this is a good stopping point for this PR as it covers move/rotate/scale. Ideally I'd like to wait until v0.18 gets released, merge this PR, do some heavy-testing as well as with @carlopav 's code, continue cleaning, and then do more PRs to polish/extend this "subelement editing" concept.

What do you reckon?

[carlopav]

Just Great!

[vocx]

Quote from the PR:

- Tab-preselect draft objects to edit them easier with preselection highlighting

Just a question. Your pull request #1975 seems to introduce cycling selection using the Tab key. Have you tried this lately with 0.19? I don't seem to be able to make it work. I wonder, since realthunder's LinkMerge (pull request #2350) came after your code, maybe his code overwrote yours.

Pull request #20 in the BIM Workbench seems to also reference this type of pre-selection.

I wrote Draft Edit Improved (which given the recent pull request #2597 will have to be renamed) but I haven't updated it in a while, because I wasn't sure how the tool is supposed to be used. Maybe you can add a full explanation about it.

Also, as far as I can tell, your idea was to implement more things on this new "edit mode", and deprecate the original Draft Edit tool. However, it seems this was never completed, right? So far it only works with move, rotate, and scale, with Draft Wire. Is this correct?

[Moult]

Just a question. Your pull request #1975 seems to introduce cycling selection using the Tab key.

From memory, although I need to check, the shortcut was changed to the "`" key (above the tab key) prevent clashes with the default OS behaviour of tab to switch input field focus.

[vocx]

...
I've created a work-in-progress PR for developing this tool. It is possible to implement it fully quite quickly in a very hacky way, but I want to take this opportunity to refactor some of the draft code to make it more flexible:

https://github.com/FreeCAD/FreeCAD/pull/1975
...

This was merged in git commit 64d7ed3697.

I just want to mention that with the reorganization of the Draft code that we are doing, I took the liberty of cleaning various snippets that you wrote in Draft_Move, Draft_Rotate, and Draft_Scale. My analysis is this, your code is good but a bit unreadable, with too many list comprehensions and parentheses inside parentheses. My recommendation is to make the code a bit more verbose. I know it sucks to make things verbose, because they are less computationally efficient, but ultimately it's much better for future maintenance. There are many users of Python who aren't expert programmers so for them looking at clean source code is great.

If you want to see the changes we did, look at these files:

Code: Select all

draftguitools/gui_move.py
draftguitools/gui_rotate.py
draftguitools/gui_scale.py

Old. Everything is smushed into a single format().

Code: Select all

        for object in self.selected_subelements:
            for index, subelement in enumerate(object.SubObjects):
                if not isinstance(subelement, Part.Edge):
                    continue
                arguments.append('[FreeCAD.ActiveDocument.{}, {}, {}]'.format(
                    object.ObjectName,
                    int(object.SubElementNames[index][len("Edge"):])-1,
                    DraftVecUtils.toString(self.vector)))
        command.append('Draft.copyMovedEdges([{}])'.format(','.join(arguments)))
        command.append('FreeCAD.ActiveDocument.recompute()')
        return command

New. Each command in a separate line for more clarity, also naming more variables.

Code: Select all

        for obj in self.selected_subelements:
            for index, subelement in enumerate(obj.SubObjects):
                if not isinstance(subelement, Part.Edge):
                    continue
                _edge_index = int(obj.SubElementNames[index][E:]) - 1
                _cmd = '['
                _cmd += 'FreeCAD.ActiveDocument.'
                _cmd += obj.ObjectName + ', '
                _cmd += str(_edge_index) + ', '
                _cmd += DraftVecUtils.toString(self.vector)
                _cmd += ']'
                arguments.append(_cmd)

        all_args = ', '.join(arguments)
        command.append('Draft.copyMovedEdges([' + all_args + '])')
        command.append('FreeCAD.ActiveDocument.recompute()')
        return command

Obviously this is more verbose and maybe not as fast if we measure in microseconds, but I think it's more maintainable. Since this is graphical code, it doesn't need to be extremely fast, the graphical event loop anyway is slow. Human reaction times are in the order of 100 milliseconds. Anything faster than that we will not notice. We want maximum efficiency only with code that should run completely from the terminal.

In other cases there are list comprehensions for lists of maybe 10 elements (vertices in a polygon, for example). Sure the list comprehension is fast, but for this case we don't save significant time if we just do a good old loop. It would only make sense if we need to process thousands of elements, but in this case, most time is spent in the GUI processing.

I also noticed some issues with the functions that modify vertices, edges, and perform copy and move, inside Draft.py: moveVertex, moveEdge, copyMovedEdges, copyMovedEdge, etc. However, these issues I will address once most Draft objects have been moved by Carlo to individual modules. Then it will be much easier to handle, because right now, modifying a 7000 line module is a bit hard.

I know that since you made these changes to Draft, you moved your focus more to BlenderBIM. I think at some point you basically said "do whatever you want to my code, or remove it, whatever". That's fine. I don't think we need to get rid of your code, but I think it makes sense to keep it clean and maintainable.

[carlopav]

I just want to add a note to what you said @vocx:
- the subelement modifiers code will be moved to the respective function modules (.move, .rotate, .scale);
- the function is just partially complete, because it only works with Draft Polyline (Draft_Wire), on other objects it produces errors (at least this is my experience), so we should make the user aware of that befor the release of 0.19.