Anim: Per bone wire width for custom shapes #120176
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
6 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#120176
Loading…
Reference in New Issue
No description provided.
Delete Branch "ChrisLend/blender:bone_wire_width"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
The setting adds the "Custom Shape Wire Width" option to the "Viewport Display/Custom Shape" section of a pose bone.
As the setting says, this controls how thick the wire is drawn in the viewport.
This is done by adding a geometry shader that makes two triangles out of a line.
Location in menu.
The Anti-Aliasing is controlled by the setting Viewport->Quality->Smooth Wires->Overlay in the user preferences.
Example.
Artifacts
When increasing the line width, you can get the following issue.
This comes from extruding each edge along the normal of its direction. This could be solved by adding round caps. IMO that is out of scope for this PR though since the same issue also occurs with thick edges in edit mode
For the review
Test File Attached
no_geom
shader. It is there though, so Mac shouldn't crash.Future improvements
Just a note. Apple doesn't support geometry shaders and a work-around needs to be added for the metal backend. See the "no geom" shaders.
@Jeroen-Bakker thanks I was wondering about those. I will ask in the blender chat for mac people to review/test this once it's ready.
See #119146 for implementation for no geom shaders.
Sorry wrong button.
@Jeroen-Bakker I got the Anti Aliasing to work.
I had this weird artifact that with certain bone colors, the bones disappeared.
Using
RenderDoc
I was able to determine that this came from theantialiasing_ps
.No idea why, but what I needed to do was to add the
lineOutput
back to the fragment shader. Even though I just always set it tovec4(0)
this makes it work.Yes lineOutput is defined as an output and should always be set. Otherwise it can create undefined behavior.
I think the simplest solution for the visual artifacts on thick wires might be to introduce rounded tips:
@ZedDB I thought about that as well. But I think I'll leave that for a future improvement.
WIP: Anim: Per bone wire width for custom shapesto Anim: Per bone wire width for custom shapeshmmm in the Rain rig, I can't seem to find the option to set the thickness. Am I looking in the wrong place?
@dr.sybren it should be there
for good measure I just merged main in case that makes a difference
@ -0,0 +1,47 @@
/* SPDX-FileCopyrightText: 2019-2023 Blender Authors
if i may, i think
2023
should be replaced by2024
Good catch. I think the currently preferred approach is to just use the year at which the file was first created, so
2019-2023
→2024
.I didn't review the GLSL, my shader-fu is not strong enough.
@ -3237,0 +3240,4 @@
continue;
}
LISTBASE_FOREACH (bPoseChannel *, pchan, &ob->pose->chanbase) {
pchan->custom_shape_wire_width = 1.0;
It might be enough to add this to
DNA_action_defaults.h
. The file doesn't exist yet; check90ef46baa1
to see how I addedDNA_anim_defaults.h
.Just tried that and without the versioning code the wire_width is at 0 for old files.
Should I still add the defaults code?
@ -882,2 +890,3 @@
OVERLAY_bone_instance_data_set_color_hint(&inst_data, outline_color);
OVERLAY_bone_instance_data_set_color(&inst_data, outline_color);
inst_data.color_a = encode_2f_to_float(outline_color[0], outline_color[1]);
/* Due to the encoding clamping the passed in floats, the wire width needs to be scaled down.
eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeuw. Not your code, but just that this is necessary. Please do add what the floats are clamped to (I'm guessing 1.0, but better to have this explicit).
I am not sure what to do here. It is clamping to 2, but I am only using the 0-1 range.
Adding a comment here means it can go out of sync with the code easily.
@ -883,1 +891,3 @@
OVERLAY_bone_instance_data_set_color(&inst_data, outline_color);
inst_data.color_a = encode_2f_to_float(outline_color[0], outline_color[1]);
/* Due to the encoding clamping the passed in floats, the wire width needs to be scaled down.
* Keep in sync with the max value of RNA. */
Define this maximum as a
constexpr
in a header somewhere, then use it in the various places, instead of having16.0
that needs to be synced up.moved it to
DNA_action_types.h
just above wherebPoseChannel
is definedThe artifacts mentioned in the PR description are more pronounced with curved shapes. The image below shows this well, but is at thickness = 10, which I think will be used quite rarely in production files. With thickness = 4 it still looks good.
@ChrisLend do you know if round caps are something we'll get soon, as in for Blender 4.2?
@dr.sybren I don't know. There is still more than a month to go until beta, but I have never done such a thing so I can't give an estimate.
I think it should be doable by just emitting more vertices in the geometry shader....
@Jeroen-Bakker could you share some of your wisdom with us?
@ -0,0 +10,4 @@
geometry_out.finalColor = color;
geometry_noperspective_out.edgeCoord = coord;
gl_Position = pos;
/* Multiply offset by 2 because gl_Position range is [-1..1]. */
The comment seems a bit incomplete. It's nice that that range is
[-1…1]
, but without knowing what the range ofoffset
is, this doesn't help much.About this and the comment below. I copied that comment from the _geom.glsl file I used as a reference to construct this.
I see how that isn't clear from the PR but I am still uncertain how I should handle that.
Change the comment only in this file or make a refactor that changes it in all
@ -0,0 +12,4 @@
gl_Position = pos;
/* Multiply offset by 2 because gl_Position range is [-1..1]. */
gl_Position.xy += offset * 2.0 * pos.w;
/* Correct but fails due to an AMD compiler bug, see: #62792.
Move the comment underneath the
#if 0
so that it's clearer that it's scoped to that.Also issue #62792 has been closed as 'resolved' years ago, so not sure why this is referenced here.
@ -20,0 +17,4 @@
geometry_in.finalColor.a = 1.0;
/* Due to packing, the wire width is passed in compressed. If the RNA range is increased, this
* needs to change as well. */
geometry_in.wire_width = bone_color.a * 16.0;
AFAIK it should be possible to
#include
a file in both GLSL and C++, so that this constant can be shared from a single definition. If that's not possible, at least mentionWIRE_WIDTH_COMPRESSION
here so that it's clear where this value comes from.@ -0,0 +15,4 @@
/* Due to packing, the wire width is passed in compressed. If the RNA range is increased, this
* needs to change as well. */
const float wire_width = bone_color.a * 16.0;
Same as above, should be using the constant, or as a fallback reference it.
@ -246,1 +246,4 @@
/* Used in the armature drawing code. Due to the encoding clamping the passed in floats, the wire
* width needs to be scaled down. */
static constexpr float WIRE_WIDTH_COMPRESSION = 16.0f;
Also mention that this is used in some GLSL files. If those explicitly name the constant, it shouldn't be necessary to list the filenames here (as those can get outdated).
+This should be protected by
#ifdef __cplusplus
@mod_moder can you explain why?
This is C header, but
constexpr
is CPP thing.@ -1165,0 +1167,4 @@
RNA_def_property_ui_text(
prop, "Custom Shape Wire Width", "Adjust the line thickness of custom shapes");
/* When increasing the property range,
* also modify the geometry shader and overlay_armature.cc. */
This comment is now outdated.
LGTM!
Checkout
From your project repository, check out a new branch and test the changes.