Armature drawing code refactor #106232

Sybren A. Stüvel · 2023-03-28T17:13:45+02:00

Sybren A. Stüvel commented

2023-03-28 17:13:45 +02:00

Refactor of the armature drawing code. This PR introduces a strategy pattern for the implementation of the various armature drawing modes. Instead of having to switch (arm->drawtype) everywhere, one function is used to select the appropriate ArmatureBoneDrawStrategy subclass.

In the future this class could be expanded even further. Currently there are still places where there are explict checks for the draw type, making it cumbersome to add new types. For now I feel it's already a good enough move forward; future extensions can be done in later PRs.

#106230 introduces a new armature draw type, using this new strategy pattern. To be able to build it, it also contains the code from (an older version of) this PR.

Refactor of the armature drawing code. This PR introduces a [strategy pattern](https://en.wikipedia.org/wiki/Strategy_pattern) for the implementation of the various armature drawing modes. Instead of having to `switch (arm->drawtype)` everywhere, one function is used to select the appropriate `ArmatureBoneDrawStrategy` subclass. In the future this class could be expanded even further. Currently there are still places where there are explict checks for the draw type, making it cumbersome to add new types. For now I feel it's already a good enough move forward; future extensions can be done in later PRs. ------------- #106230 introduces a new armature draw type, using this new strategy pattern. To be able to build it, it also contains the code from (an older version of) this PR.

Sybren A. Stüvel added 2 commits 2023-03-28 17:13:56 +02:00

6d21d2a1fd Refactor armature drawing code

9c91f236fa Add Spheres armature mode

Sybren A. Stüvel added the

Module

Animation & Rigging

label 2023-03-28 17:14:08 +02:00

Sybren A. Stüvel added this to the Animation & Rigging project 2023-03-28 17:14:13 +02:00

Sybren A. Stüvel referenced this pull request

2023-03-28 17:15:09 +02:00

WIP: anim/armature-mode-spheres #106230

Sybren A. Stüvel requested review from Christoph Lendenfeld 2023-03-28 17:16:53 +02:00

Sybren A. Stüvel requested review from Nate Rupsis 2023-03-28 17:17:03 +02:00

Christoph Lendenfeld commented

2023-03-30 12:01:54 +02:00

Really liking the changes to the code. A lot clearer what's going on

I assume the Sphere drawing mode should also work in edit mode? Currently it draws the default bone

Really liking the changes to the code. A lot clearer what's going on I assume the Sphere drawing mode should also work in edit mode? Currently it draws the default bone

Sybren A. Stüvel commented

2023-03-30 12:18:13 +02:00

Really liking the changes to the code. A lot clearer what's going on

👍

I assume the Sphere drawing mode should also work in edit mode? Currently it draws the default bone

That's for #106230, but no, for now sphere drawing mode won't work in edit mode. The problem is that for edit mode to work well, you need access to the bone tail, roll, etc.

A solution that was discussed is to draw the octo's in the spheres edit mode, as additional 'ghosts' next to the spheres. For this I feel more of the drawing code should be moved into the ArmatureBoneDrawStrategy classes, or ArmatureDrawContext could use some refactoring. Currently the drawing code is IMO still too linked to the drawing mode.

> Really liking the changes to the code. A lot clearer what's going on :+1: > I assume the Sphere drawing mode should also work in edit mode? Currently it draws the default bone That's for #106230, but no, for now sphere drawing mode won't work in edit mode. The problem is that for edit mode to work well, you *need* access to the bone tail, roll, etc. A solution that was discussed is to draw the octo's in the spheres edit mode, as additional 'ghosts' next to the spheres. For this I feel more of the drawing code should be moved into the `ArmatureBoneDrawStrategy` classes, or `ArmatureDrawContext` could use some refactoring. Currently the drawing code is IMO still too linked to the drawing mode.

Sybren A. Stüvel force-pushed anim/armature-drawing-refactor from 9c91f236fa to d93b3f7b78

2023-07-21 12:33:57 +02:00

Compare

Sybren A. Stüvel force-pushed anim/armature-drawing-refactor from d93b3f7b78 to 3524e97955

2023-07-21 12:37:14 +02:00

Compare

Sybren A. Stüvel commented

2023-07-21 12:38:36 +02:00

I just force-pushed just the code refactor; the addition of sphere drawing mode will be left to another PR. That way this is just a non-functional refactor patch.

Sybren A. Stüvel changed title from ~~WIP: Armature drawing code refactor~~ to Armature drawing code refactor

2023-07-21 12:41:41 +02:00

Christoph Lendenfeld approved these changes 2023-07-21 14:53:44 +02:00

Christoph Lendenfeld left a comment

lgtm. Just a function name that I think could be more descriptive

source/blender/draw/engines/overlay/overlay_armature.cc Outdated

						
				@ -96,0 +128,4 @@

				                         const int select_id) const = 0;

				  /** Should the relationship line between this bone and its parent be drawn? */

				  virtual bool use_bone_relation_to_parent(const EditBone *ebone,

Christoph Lendenfeld commented

2023-07-21 14:47:44 +02:00

not sure about the function name. "use" is raising the question what it is used for.
The comment explains it but I think calling it e.g. should_draw_relation_to_parent would make it clear within the function name itself

not sure about the function name. "use" is raising the question what it is used for. The comment explains it but I think calling it e.g. `should_draw_relation_to_parent` would make it clear within the function name itself

dr.sybren marked this conversation as resolved

Sybren A. Stüvel added 1 commit 2023-07-21 14:58:24 +02:00

1617d38b1c Reviewer feedback

Sybren A. Stüvel added 1 commit 2023-07-21 15:00:58 +02:00

4684b94201 Merge remote-tracking branch 'origin/main' into anim/armature-drawing-refactor

Sybren A. Stüvel referenced this issue from a commit

2023-07-21 15:03:59 +02:00

Anim: Refactor the armature drawing code

Sybren A. Stüvel manually merged commit f645130465 into main

2023-07-21 15:04:10 +02:00

Sybren A. Stüvel removed this from the Animation & Rigging project 2023-09-14 15:05:25 +02:00

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Armature drawing code refactor #106232