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

[For v0.19] New draft edit tool for multiple object editing and with all modifiers #1975

Closed
wants to merge 18 commits into from

Conversation

@Moult
Copy link
Contributor

Moult commented Feb 17, 2019

This is the in-progress PR of creating a new, refactored, edit tool that:

  1. Allows editing of one or more objects at once
  2. Allows you to use all modifiers on individual vertices or edges to modify an existing wire

When complete I am hoping for the current draft edit tool to be deprecated.

Discussion thread: https://forum.freecadweb.org/viewtopic.php?f=23&t=34114

  • It allows you to select multiple objects to edit instead of just one.
  • It highlights the object lines and the points in red.
  • It stays in the mode and allows you to run other modifiers.
  • A very hackish hook into the move modifier is added as a proof of concept.

Thank you for creating a pull request to contribute to FreeCAD! To ease integration, please confirm the following:

  • Branch rebased on latest master git pull --rebase upstream master
  • Unit tests confirmed to pass by running ./bin/FreeCAD --run-test 0
  • Commit message is well-written
  • Commit message includes issue #<id> or fixes #<id> where <id> is the associated MantisBT issue id if one exists

And please remember to update the Wiki with the features added or changed once this PR is merged.


 * It allows you to select multiple objects to edit instead of just one.
 * It highlights the object lines and the points in red.
 * It stays in the mode and allows you to run other modifiers.
 * A very hackish hook into the move modifier is added as a proof of concept.
@Moult Moult changed the title Add new edit tool icon, shortcut, and basic modes. [For v0.19] Add new edit tool icon, shortcut, and basic modes. Feb 17, 2019
@Moult Moult changed the title [For v0.19] Add new edit tool icon, shortcut, and basic modes. [For v0.19] New draft edit tool for multiple object editing and with all modifiers Feb 17, 2019
@Moult

This comment has been minimized.

Copy link
Contributor Author

Moult commented Feb 20, 2019

PR updated for "move" tool support.

Please watch the video demo below, it shows:

  1. Selecting multiple objects and turning them into an "Edit mode" (the idea is for this mode to highlight more editing handles, allow selection through objects, and show the draft bases of other components in case they are invisible), it also makes it easier to select handles by making them larger and red.
  2. Shows moving a single vertex from one of the selected objects. This is similar to the existing draft edit mode, but more intuitive as it uses the move command so it allows you to pick any arbitrary start and end point.
  3. Shows moving both vertices and edges at the same time from multiple shapes, using the move command.
  4. Shows leaving the "draft edit" mode.

This will make the "stretch" tool and the node move tool for wires and lines obsolete (though not for beziers, arcs, and other special shapes).

https://peertube.social/videos/watch/d39d4f3c-b3c8-4a18-b8c4-6719d0f70f48

Moult added 2 commits Mar 2, 2019
I suspect the scale command itself is a little broken, so I need to fix that before adding subelement support
@Moult

This comment has been minimized.

Copy link
Contributor Author

Moult commented Mar 3, 2019

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?

@Moult

This comment has been minimized.

Copy link
Contributor Author

Moult commented Mar 3, 2019

Sorry just a heads up I notice that the "modules" label was added, but I don't think this is correct, due to 0828a4a and 58df634 - it's pretty benign code I think, but I think it does touch core.

objects = '[' + ','.join(['FreeCAD.ActiveDocument.' + object.Name for object in self.selected_objects]) + ']'
FreeCADGui.addModule("Draft")
self.commit(translate("draft","Copy"),
['Draft.scale('+objects+',delta='+DraftVecUtils.toString(delta)+',center='+DraftVecUtils.toString(self.node[0])+',copy='+str(copy)+',legacy='+str(legacy)+')',
self.commit(translate("draft","Copy" if self.task.isCopy.isChecked() else "Scale"),

This comment has been minimized.

Copy link
@luzpaz

luzpaz Mar 3, 2019

Contributor

@Moult is there a missing " specifically before the if ?

@yorikvanhavre yorikvanhavre added core and removed modules labels Mar 28, 2019
@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented Apr 24, 2019

Keeping this on hold because of the first line in the PR description above, be sure to notify if there is anything required here..

@Moult

This comment has been minimized.

Copy link
Contributor Author

Moult commented Apr 24, 2019

Feel free to merge :) don't think I missed a quote, but I'm kinda on holiday so I don't have a computer to look in detail :)

@yorikvanhavre yorikvanhavre removed the on hold label Apr 25, 2019
@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented Apr 25, 2019

Ok will give it a test spin & merge

@Moult

This comment has been minimized.

Copy link
Contributor Author

Moult commented May 3, 2019

Bump :)

@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented May 8, 2019

Okay just gave it a test run now. Nice!! It needs some more work as it fails in a number of cases without warning, for ex. if you select an obj that is not a wire or line. But it doesn't modify the existing behaviour if you don't use the new option, so I think there is no problem in having it merged already...

@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented May 8, 2019

Merged with 64d7ed3

@Moult

This comment has been minimized.

Copy link
Contributor Author

Moult commented May 13, 2019

@yorikvanhavre

This comment has been minimized.

Copy link
Member

yorikvanhavre commented May 14, 2019

Sounds like a plan :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.