GPv3: Opacity modifier #116946
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
7 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#116946
Loading…
Reference in New Issue
No description provided.
Delete Branch "LukasTonne/blender:gp3-opacity-modifier"
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?
Opacity modifier implementation based on GP2. Functionality is largely unchanged:
Color Mode is either
Stroke
orFill
for modifying color opacity orHardness
.Uniform Opacity does two things at once (!):
Weight as Factor (button to the right of Opacity Factor): Use the vertex group as opacity factor rather than an overall influence. This is confusing and hard to convey, but copies behavior from GP2.
The Influence panel contains the same filter settings as the GP2 modifier, with some small changes:
Note 1: Layers don't have UI for setting a pass yet, this will be a generic layer attribute. This can be set through the API for testing.
Note 2: In GP2 a pass value of zero was used to disable pass filters. Since zero is a valid pass ID an explicit flag has been added for the purpose of turning pass filters on and off.
Note: Vertex groups are not supported in GP3 yet. The modifier uses a stub function that returns a constant weight of
0.0f
when a vertex group is used.The Influence settings have been moved into a separate DNA struct, which should help with reusability in various modifiers. Various utility functions can be found int
MOD_grease_pencil_util.hh
for handling influence settings and generatingIndexMasks
for modernized C++ code.48f61cfe8b
toca665e7ad8
WIP: Grease Pencil Opacity modifierto Grease Pencil Opacity modifier@ -0,0 +109,4 @@
/* TODO Would be nice to have the checkbox in the same line as the pass button. */
row = uiLayoutRow(col, true);
uiItemR(row, ptr, "use_layer_pass_filter", UI_ITEM_NONE, "Filter by layer pass", ICON_NONE);
Would be nice to have the checkbox in the same line as the pass button. I haven't found a good way to combine these buttons yet, could use help from UI experts.
It's ugly but possible, here's an example from the limit location constraint:
I Agree to have the checkbox on the same line. Hans proposal seems a right choice
I didn't know about the
uiLayoutRowWithHeading
feature, thanks.The code here might make more sense in the
blender::modifiers
namespace, since that matches the module. Potentiallyblender::modifiers::grease_pencil
if that was helpful.I don't see the "fill_opacity" attribute used in any existing code. I had thought that was replaced with the "opacity" attribute per point.
@ -0,0 +80,4 @@
}
/* XXX Placeholder for vertex groupn weights. */
static VArray<float> get_grease_pencil_modifier_vertex_weights(
You should just be able to retrieve the vertex group as an attribute
Took some explaining from @filedescriptor how this works (maybe a good topic for technical docs?). As far as i understand the
VArray
wrapper for thecurves->deform_verts()
is ok here, since we're only using a single vertex group. For something like the armature modifier i'd guess we still want to use theMDeformVert
directly.Right, exactly
@ -0,0 +162,4 @@
if (use_vgroup_opacity) {
/* Use the first stroke point as vertex weight. */
const IndexRange points_range = points_by_curve[curve_i];
points_range
->points
is the cononical variable name here. "range" is already described by the type@ -0,0 +163,4 @@
if (use_vgroup_opacity) {
/* Use the first stroke point as vertex weight. */
const IndexRange points_range = points_by_curve[curve_i];
const float stroke_weight = points_range.is_empty() ? 1.0f :
You can count on the fact that curves always have at least one point
At least one or at least two? I need two for this length factor calculation.
Falk says curves can have 1 point only, so i still need to check.
@ -0,0 +182,4 @@
const IndexMask &curves_mask)
{
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> hardnesses = attributes.lookup_or_add_for_write_span<float>(
Could this be skipped if there's no hardness attribute already?
I suppose it could. The old modifier is quite inconsistent in how the "uniform" setting is applied (confusingly called "normalized" internally). In the color modes (stroke, fill) it then applies the settings as an offset rather than a factor, with some unexplained bias and clamping. But this isn't used for the hardness mode, so 🤷
@ -0,0 +198,4 @@
const ModifierEvalContext *ctx,
bke::CurvesGeometry &curves)
{
GreasePencilOpacityModifierData *omd = (GreasePencilOpacityModifierData *)md;
C++ style casts (elsewhere in this file too)
Grease Pencil Opacity modifierto GPv3: Opacity modifier@HooglyBoogly "fill_opacity" is used for strokes with a fill material. Essentially this is the equivalent of the point opacity but for the entire n-gon shape.
Okay, I just don't see any mentions of
"fill_opacity"
in the source code. I guess it's planned?@ -0,0 +122,4 @@
else if (use_weight_as_factor) {
/* Use vertex group weights as opacity factors. */
const float vgroup_weight = vgroup_weights[point_i];
const float point_factor = vgroup_weight;
remove this and pass
vgroup_weight
to clamp fn? 😅Haha yeah, this is the result of me trying to make sense of the logic of the old modifier.
@ -0,0 +239,4 @@
*grease_pencil, omd->influence, mask_memory);
Vector<Drawing *> drawings = greasepencil::get_drawings_for_write(
*grease_pencil, layer_mask, frame);
for (Drawing *drawing : drawings) {
threading::parallel_for
?@ -0,0 +131,4 @@
const float vgroup_weight = vgroup_weights[point_i];
const float vgroup_influence = invert_vertex_group ? 1.0f - vgroup_weight : vgroup_weight;
opacities.span[point_i] = std::clamp(
opacities.span[point_i] + omd.color_factor * curve_factor * vgroup_influence - 1.0f,
Wondering if this could be made much clean by using
DefaultMixer
?That seems a bit unnecessary here.
1 - current
as some kind of cutoff point, and then allows ramping up the influence to 2.0, presumably so one can reach full opacity/hardness again. It all seems very ad-hoc.I'd prefer to not try and "fix" this too much, since the primary purpose is compatibility with GP2.
Yes, we don't support it yet.
@ -46,6 +46,7 @@ class DATA_PT_modifiers(ModifierButtonsPanel, Panel):
def draw(self, _context):
layout = self.layout
ob_type = _context.object.type
Was this added by mistake? I don't see where
ob_type
is used here.@ -154,2 +155,2 @@
* Only set on modifiers in evaluated objects. The flag indicates that the user modified inputs
* to the modifier which might invalidate simulation caches.
* Only set on modifiers in evaluated objects. The flag indicates that the user modified
* inputs to the modifier which might invalidate simulation caches.
Looks like a
clang-format
issue here.@ -7426,0 +7574,4 @@
RNA_def_property_update(prop, 0, "rna_Modifier_update");
}
static void rna_def_modifier_grease_pencil_vertex_group(StructRNA *srna,
Should this be
rna_def_modifier_grease_pencil_vertex_group_filter
to be consistent with the other filters?Not really, the other "filters" are purely selection masks based on layers or materials. The vertex group is always either a multiplier or, in the case of the opacity modifier, an offset/factor/something.
I appended the "filter" suffix to layer/material mostly so it would be clear that these are just selection criteria and their settings don't otherwise affect the modifier.
Looks like
blenkernel/intern/modifier.cc
doesn't have thoseBKE_
functions inblender::modifiers
orblender::bke
namespace, is this intentional? Or it's just pending migration? I could maybe update that part in a separate PR as well.Looks good to me!
@ -103,0 +104,4 @@
"GREASE_PENCIL_OPACITY",
ICON_MOD_OPACITY,
"Opacity",
"Opacity of the strokes"},
Opacity of the strokes
->Change the opacity of the strokes
I know this is just copying the old description, might as well fix it though
@ -0,0 +1,338 @@
/* SPDX-FileCopyrightText: 2005 Blender Authors
Copyright year can be updated
@ -0,0 +57,4 @@
static void copy_data(const ModifierData *md, ModifierData *target, const int flag)
{
const GreasePencilOpacityModifierData *omd =
You can write
const auto *
with the cast, it might keep it on one line@ -0,0 +94,4 @@
bke::MutableAttributeAccessor attributes = curves.attributes_for_write();
bke::SpanAttributeWriter<float> opacities = attributes.lookup_or_add_for_write_span<float>(
"opacity", bke::AttrDomain::Point);
const bke::AttributeReader<float> vgroup_weights = attributes.lookup_or_default<float>(
Use the * operator overload to just store the VArray directly here
Some notes regarding UI
The indent for passes is a little off. Is possible to fix somehow?
@mendio When you click "Comment and Close" it actually closes the PR. Confusing, i know :)
sorry for the close, my bad :)
Names simplified.
Thanks for catching, i had the property decorators disabled on the wrong sub-layout.