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

Introduce an "Edit Shape" Tool to the RigidBody Editor #447

Closed
ilexp opened this Issue Dec 8, 2016 · 9 comments

Comments

2 participants
@ilexp
Member

ilexp commented Dec 8, 2016

Summary

The RigidBody Editor currently provides no way of editing already-created shapes. This would be a really useful addition with a big usability increase.

Analysis

Quote from the original issue #54:

As soon as non-convex polygons with N vertices are supported, it would make a lot more sense to have a nice vertex editor. Maybe there could be a shared "edit shape" tool that would allow to:

  • Resize (using radius "vertex") and move (using center "vertex") circles.
  • Move vertices of a poly-like shape.
  • Dynamically add vertices to a poly-like shape by dragging a point on the outline where currently no vertex exists.
  • Deleting vertices from a poly-like shape.
  • Make sure that it's possible to see the generated individual convex shapes, so it becomes apparent to the user that arbitrarily complex shapes are not "free". Maybe somehow visualize a "less is better" relation for vertex and fixture count?
  • Multi-selection might not be required, or at least should be carefully evaluated for its cost / benefit ratio. My guess would be it's not actually that useful in this case.
  • Implementation note: Vertex editing should not be implemented using the SelObj / ObjectEditor API, since that would (a) bring more overhead than usefulness and (b) merge vertex editing with shape editing.
    • Instead, do a lightweight implementation in a separate RigidBodyEditorTool.
    • This may require extending and modifying how RigidBodyEditorTools work, specifically allowing multiple begin / end cycles without deselecting.

As a soft prerequisite, there probably should be a base ShapeInfo API / class for the various ShapeInfos (Poly, Loop, Chain) to reduce the amount of type checking code and keep it more extensible. This would also allow to simplify some RigidBodyShapeCamViewLayer rendering, RigidBodyRenderer and RigidBodyEditor code a lot.

  • This issue still requires some concept work.
  • Requirement: Issue #446
  • Could benefit from issue #331, but that would mean moving this to v3.0
  • Could also benefit from a previous refactoring / cleanup of the RigidBody editor where needed.

Attachments

  • Related: Issue #54

@ilexp ilexp added this to the General milestone Dec 8, 2016

@YMRYMR

This comment has been minimized.

Show comment
Hide comment
@YMRYMR

YMRYMR Apr 23, 2017

Contributor

Some thoughts:

  • A solution could be to edit/replace the NoRigidBodyEditorTool, using it as a continuous action while no other action is selected (the shared "edit shape" tool mentioned). This new action would need to:
    • Understand what kind of shape is being rendered/modified.
    • Understand if the hovered selector is a real vertex (only in poly shapes) or an imaginary vertex rendered to allow rotation and scale. That may be redundant with existing functionality?
  • To avoid conflicts between the center vertex mentioned and other vertices, maybe shapes should be selectables/moveables by clicking inside them, instead of using a selector.
  • Incorrect shapes should be selectables/editables as well.
  • Adding something like public virtual void OnCollectStateWorldOverlayDrawcalls(Canvas canvas) { } to RigidBodyEditorTool would allow to render the selectors (including the "add real vertex" dummy). Not sure about the edit functionality yet.
  • Multiselection could be useful to add vertex options like refine or simplify (maybe using a context menu), but that could be done in the future as an incremental update.
  • Unifying the ShapeInfos in one base class could impact this issue, but the shared "edit shape" tool would still need to know what kind of shape is being rendered/modified and act accordingly.
Contributor

YMRYMR commented Apr 23, 2017

Some thoughts:

  • A solution could be to edit/replace the NoRigidBodyEditorTool, using it as a continuous action while no other action is selected (the shared "edit shape" tool mentioned). This new action would need to:
    • Understand what kind of shape is being rendered/modified.
    • Understand if the hovered selector is a real vertex (only in poly shapes) or an imaginary vertex rendered to allow rotation and scale. That may be redundant with existing functionality?
  • To avoid conflicts between the center vertex mentioned and other vertices, maybe shapes should be selectables/moveables by clicking inside them, instead of using a selector.
  • Incorrect shapes should be selectables/editables as well.
  • Adding something like public virtual void OnCollectStateWorldOverlayDrawcalls(Canvas canvas) { } to RigidBodyEditorTool would allow to render the selectors (including the "add real vertex" dummy). Not sure about the edit functionality yet.
  • Multiselection could be useful to add vertex options like refine or simplify (maybe using a context menu), but that could be done in the future as an incremental update.
  • Unifying the ShapeInfos in one base class could impact this issue, but the shared "edit shape" tool would still need to know what kind of shape is being rendered/modified and act accordingly.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Apr 24, 2017

Member
  • A solution could be to edit/replace the NoRigidBodyEditorTool, using it as a continuous action while no other action is selected (the shared "edit shape" tool mentioned). This new action would need to:
  • Understand what kind of shape is being rendered/modified.
  • Understand if the hovered selector is a real vertex (only in poly shapes) or an imaginary vertex rendered to allow rotation and scale. That may be redundant with existing functionality?
  • To avoid conflicts between the center vertex mentioned and other vertices, maybe shapes should be selectables/moveables by clicking inside them, instead of using a selector.

We can avoid the ambiguity issues by not unifying the two tools and I would in fact like to keep them separate. The default "no tool" tool can continue to transform one or multiple shapes as a whole while the new vertex editing tool can focus on vertices and imaginary vertices only.

The user flow I have in mind is:

  • By default, the "no tool" is selected for general transform.
  • Users can switch to vertex editing mode using the tool button.
  • When creating any new shape, vertex editing will be selected afterwards by default, so users can edit the shape they just created.

Shortcut keys can be used to quickly switch between all tools.

  • Incorrect shapes should be selectables/editables as well.

Yep, definitely. With the new vertex editing, we will either have to account for the fact that users can create invalid shapes or introduce some mechanism that prevents them from doing so.

  • Adding something like public virtual void OnCollectStateWorldOverlayDrawcalls(Canvas canvas) { } to RigidBodyEditorTool would allow to render the selectors (including the "add real vertex" dummy). Not sure about the edit functionality yet.

👍

Maybe OnCollectWorldOverlayDrawcalls though, because the camview stuff is kind of abstracted away behind the tools / environment API.

  • Multiselection could be useful to add vertex options like refine or simplify (maybe using a context menu), but that could be done in the future as an incremental update.

I agree, that would be an option for later. Let's keep this out for now, so we have a simpler first iteration.

  • Unifying the ShapeInfos in one base class could impact this issue, but the shared "edit shape" tool would still need to know what kind of shape is being rendered/modified and act accordingly.

Yep, true. No reason to defer this issue, especially since the unification might get pushed to v3.0.

Member

ilexp commented Apr 24, 2017

  • A solution could be to edit/replace the NoRigidBodyEditorTool, using it as a continuous action while no other action is selected (the shared "edit shape" tool mentioned). This new action would need to:
  • Understand what kind of shape is being rendered/modified.
  • Understand if the hovered selector is a real vertex (only in poly shapes) or an imaginary vertex rendered to allow rotation and scale. That may be redundant with existing functionality?
  • To avoid conflicts between the center vertex mentioned and other vertices, maybe shapes should be selectables/moveables by clicking inside them, instead of using a selector.

We can avoid the ambiguity issues by not unifying the two tools and I would in fact like to keep them separate. The default "no tool" tool can continue to transform one or multiple shapes as a whole while the new vertex editing tool can focus on vertices and imaginary vertices only.

The user flow I have in mind is:

  • By default, the "no tool" is selected for general transform.
  • Users can switch to vertex editing mode using the tool button.
  • When creating any new shape, vertex editing will be selected afterwards by default, so users can edit the shape they just created.

Shortcut keys can be used to quickly switch between all tools.

  • Incorrect shapes should be selectables/editables as well.

Yep, definitely. With the new vertex editing, we will either have to account for the fact that users can create invalid shapes or introduce some mechanism that prevents them from doing so.

  • Adding something like public virtual void OnCollectStateWorldOverlayDrawcalls(Canvas canvas) { } to RigidBodyEditorTool would allow to render the selectors (including the "add real vertex" dummy). Not sure about the edit functionality yet.

👍

Maybe OnCollectWorldOverlayDrawcalls though, because the camview stuff is kind of abstracted away behind the tools / environment API.

  • Multiselection could be useful to add vertex options like refine or simplify (maybe using a context menu), but that could be done in the future as an incremental update.

I agree, that would be an option for later. Let's keep this out for now, so we have a simpler first iteration.

  • Unifying the ShapeInfos in one base class could impact this issue, but the shared "edit shape" tool would still need to know what kind of shape is being rendered/modified and act accordingly.

Yep, true. No reason to defer this issue, especially since the unification might get pushed to v3.0.

@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 4, 2017

Member

Progress

Immediate ToDo

  • Check out the PR, build the project and test vertex editing from the user side. Gather a list of issues and / or potential improvements.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Squash-merge the PR back to master.
Member

ilexp commented Jun 4, 2017

Progress

Immediate ToDo

  • Check out the PR, build the project and test vertex editing from the user side. Gather a list of issues and / or potential improvements.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Squash-merge the PR back to master.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 5, 2017

Member

Progress

  • Various minor tweaks and changes to the pull request.
  • Investigated the new vertex editor from a users perspective.

Immediate ToDo

  • The (default) shape transform tool should show up in the toolbar, as the first item.
  • The edit vertices tool should be next in the toolbar to the shape transform tool.
  • There should be a toolbar separator between editing and creation tools.
    • Might achieve this by checking for large priority gaps or similar.
  • Switching between vertex edit and shape transform should be more seamless.
    • Shape transform by default (as-is)
    • Edit vertices can be activated via tool button, hotkey (both as-is) or double-clicking a shape.
    • Clicking on a shape area should switch back to transform mode.
      • Investigate how this is implemented right now.
      • Hovered shapes still show an indicator, so detection works - it's just blocked from executing.
      • Find a general way to handle this for all potential tools.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • Fix the partial update for shapes while vertices are being dragged around.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Squash-merge the PR back to master.
Member

ilexp commented Jun 5, 2017

Progress

  • Various minor tweaks and changes to the pull request.
  • Investigated the new vertex editor from a users perspective.

Immediate ToDo

  • The (default) shape transform tool should show up in the toolbar, as the first item.
  • The edit vertices tool should be next in the toolbar to the shape transform tool.
  • There should be a toolbar separator between editing and creation tools.
    • Might achieve this by checking for large priority gaps or similar.
  • Switching between vertex edit and shape transform should be more seamless.
    • Shape transform by default (as-is)
    • Edit vertices can be activated via tool button, hotkey (both as-is) or double-clicking a shape.
    • Clicking on a shape area should switch back to transform mode.
      • Investigate how this is implemented right now.
      • Hovered shapes still show an indicator, so detection works - it's just blocked from executing.
      • Find a general way to handle this for all potential tools.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • Fix the partial update for shapes while vertices are being dragged around.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Squash-merge the PR back to master.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 10, 2017

Member

Progress

  • The default / non-tool is now represented as an icon in the toolbar.
  • There are now auto-created toolbar separators for toolbar items that are >= 100 apart in their SortOrder.
  • The edit vertices tool is now next to the default / non-tool, and they're separated from the creation tools.
  • Fixed polygon shape updates while dragging around vertices.
  • Improved snap-to-grid in the master branch.
  • Fixed convex polygons not being updated when invoking UpdateShape, even though they already were when re-assigning vertices.
  • Vertex edit interaction markers are now handled in object space, so they still work when the object has been rotated, scaled or moved.

Immediate ToDo

  • Switching between vertex edit and shape transform should be more seamless.
    • Shape transform by default (as-is)
    • Edit vertices can be activated via tool button, hotkey (both as-is) or double-clicking a shape.
    • Clicking on a shape area should switch back to transform mode.
      • Extend the tool API to indicate whether the selected tools action is available or not, given the current user input / mouse pos / etc.
      • As long as the action tool is not available, allow non-tool actions. Deselect the tool when performing one.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Cleanup the PolygonRigidBodyEditorOverlay class and integrate its contents directly into the tool.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Squash-merge the PR back to master.
Member

ilexp commented Jun 10, 2017

Progress

  • The default / non-tool is now represented as an icon in the toolbar.
  • There are now auto-created toolbar separators for toolbar items that are >= 100 apart in their SortOrder.
  • The edit vertices tool is now next to the default / non-tool, and they're separated from the creation tools.
  • Fixed polygon shape updates while dragging around vertices.
  • Improved snap-to-grid in the master branch.
  • Fixed convex polygons not being updated when invoking UpdateShape, even though they already were when re-assigning vertices.
  • Vertex edit interaction markers are now handled in object space, so they still work when the object has been rotated, scaled or moved.

Immediate ToDo

  • Switching between vertex edit and shape transform should be more seamless.
    • Shape transform by default (as-is)
    • Edit vertices can be activated via tool button, hotkey (both as-is) or double-clicking a shape.
    • Clicking on a shape area should switch back to transform mode.
      • Extend the tool API to indicate whether the selected tools action is available or not, given the current user input / mouse pos / etc.
      • As long as the action tool is not available, allow non-tool actions. Deselect the tool when performing one.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Cleanup the PolygonRigidBodyEditorOverlay class and integrate its contents directly into the tool.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Squash-merge the PR back to master.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 10, 2017

Member

Progress

  • Refactored RigidBody Editor tool handling to allow greater variety in how mouse input is handled.
  • Tools can now specify whether they're hovering action hotspots and whether they can begin a new continuous action given current state and user input.
  • The RigidBody Editor uses that information to handle rightclick deselect and how to react to any given input.
  • Tools are no longer mutually exclusive with shape transform actions, leading to a more seamless integration between edit vertex and edit shape mode because unused user input is allowed to be forwarded.

Immediate ToDo

  • Some smaller fixes / cleanup
    • Add OnActionKeyReleased mouse buttons.
    • Investigate focus lost tool action end.
    • Investigate EndToolAction visibility level.
  • Double-click on a shape should select the vertex editor.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Move hover / user input state checks into OnMouseMove.
  • Cleanup the PolygonRigidBodyEditorOverlay class and integrate its contents directly into the tool.
  • Check Undo / Redo support.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Review the Pull Request changelist in full.
  • Squash-merge the PR back to master.
Member

ilexp commented Jun 10, 2017

Progress

  • Refactored RigidBody Editor tool handling to allow greater variety in how mouse input is handled.
  • Tools can now specify whether they're hovering action hotspots and whether they can begin a new continuous action given current state and user input.
  • The RigidBody Editor uses that information to handle rightclick deselect and how to react to any given input.
  • Tools are no longer mutually exclusive with shape transform actions, leading to a more seamless integration between edit vertex and edit shape mode because unused user input is allowed to be forwarded.

Immediate ToDo

  • Some smaller fixes / cleanup
    • Add OnActionKeyReleased mouse buttons.
    • Investigate focus lost tool action end.
    • Investigate EndToolAction visibility level.
  • Double-click on a shape should select the vertex editor.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Move hover / user input state checks into OnMouseMove.
  • Cleanup the PolygonRigidBodyEditorOverlay class and integrate its contents directly into the tool.
  • Check Undo / Redo support.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Review the Pull Request changelist in full.
  • Squash-merge the PR back to master.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 10, 2017

Member

Progress

  • Some smaller fixes / cleanup
  • Integrated PolygonRigidBodyEditorOverlay class directly into the tool.
  • Separated action state update from drawing code.
  • Simplified and cleaned up UndoRedo support.

Immediate ToDo

  • Double-click on a shape should select the vertex editor.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • Consider making interaction indicators rects for performance reasons on very large polygons.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Review the Pull Request changelist in full.
  • Squash-merge the PR back to master.
Member

ilexp commented Jun 10, 2017

Progress

  • Some smaller fixes / cleanup
  • Integrated PolygonRigidBodyEditorOverlay class directly into the tool.
  • Separated action state update from drawing code.
  • Simplified and cleaned up UndoRedo support.

Immediate ToDo

  • Double-click on a shape should select the vertex editor.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • Interaction indicators in Edit vertices mode should be rendered on top of RigidBody shapes, not below.
  • Consider making interaction indicators rects for performance reasons on very large polygons.
  • The default cursor in Edit vertices mode should be white, only switching to orange when hovering an interactive area.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Review the Pull Request changelist in full.
  • Squash-merge the PR back to master.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 11, 2017

Member

Progress

  • Interaction indicators are now rects, and rendered with a Z offset that places them on top of other overlays.
  • Various API extensions for RigidBody editor tools.
  • Various tweaks and bugfixes related to snap-to-grid.

Immediate ToDo

  • Double-click on a shape should select the vertex editor.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Review the Pull Request changelist in full.
  • Squash-merge the PR back to master.
Member

ilexp commented Jun 11, 2017

Progress

  • Interaction indicators are now rects, and rendered with a Z offset that places them on top of other overlays.
  • Various API extensions for RigidBody editor tools.
  • Various tweaks and bugfixes related to snap-to-grid.

Immediate ToDo

  • Double-click on a shape should select the vertex editor.
  • Edit vertices should be able to edit circle shapes using dummy vertices in center and on the radius.
  • Edit vertices should work for all poly-like shapes, including chain and loop shapes.
  • The Edit vertices tool icon should be a (non-default) cursor.
  • Examine the source code and gradually fix issues that may turn up. Push this back to the PR branch.
  • Review the Pull Request changelist in full.
  • Squash-merge the PR back to master.
@ilexp

This comment has been minimized.

Show comment
Hide comment
@ilexp

ilexp Jun 17, 2017

Member

Progress

  • Edit vertices now support all shapes, including poly-like shapes and circle shapes.
  • Postponed double-click and cursor changes, as they're both more of a nice2have thing and it seems more important to go forward with this issue soon.
  • Reviewed the latest source code iteration, squash-merged it back to master.
  • Release in progress.
Member

ilexp commented Jun 17, 2017

Progress

  • Edit vertices now support all shapes, including poly-like shapes and circle shapes.
  • Postponed double-click and cursor changes, as they're both more of a nice2have thing and it seems more important to go forward with this issue soon.
  • Reviewed the latest source code iteration, squash-merged it back to master.
  • Release in progress.

@ilexp ilexp closed this Jun 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment