Details

Reviewers

Matias Mendiola (mendio)
Antonio Vazquez (antoniov)
Daniel Martinez Lara (pepeland)

Commits

rB7d712dcd0bbd: Gpencil: Add offset(Location, Rotation, Scale) by Layer, Stroke and Material to…

Summary

This patch moves general setting to a panel, renames randomize panel to advanced, where 3 new offset modes are introduced:

offset by material
offset by layer
offset by strokes

Video instructions: https://www.youtube.com/watch?v=_RJ1Q0gxbAs

Use cases:

easily fix Z fighting when switching to 3d layer mode by offsetting by materials
one stop shop for creating parallax effect by controlling layer offset
create interesting geometrical animations with stroke mode

It works by multiplying the effect on each selected entity.
New modes also have additional controls for step and offset of first point. This way you can group together layer (for example if you have lines and fill separated)

Diff Detail

Repository: rB Blender

Event Timeline

Aleš Jelovčan (frogstomp) requested review of this revision.Jun 2 2022, 8:16 PM

Aleš Jelovčan (frogstomp) created this revision.

Added offset by material

Aleš Jelovčan (frogstomp) retitled this revision from Gpencil: Add offset by Layer & offset by stroke number to Offset modifier to Gpencil: Add offset(Location, Rotation, Scale) by Layer, Stroke and Material to Offset modifier.Jun 3 2022, 1:36 PM

Not sure if to have 9 x 2 (18) new variables in the UI is the best solution.... why not a mode selector to offset as today, layers or strokes.... if you need to do several offsets you could add more modifiers.

@Matias Mendiola (mendio) what do you think?

and now adding material we will have 9 x 3 new UI variables.... IMHO the idea of adding a mode and use the modifier for one type of offset at time could be cleaner.

I have talked with other members of the GP Team about the great number of attributes and we agree that it's better to add a mode selector and keep only 9 values. If you need more transformations, you can just add more modifiers to the stack.

@Aleš Jelovčan (frogstomp) could you make the change? The function that you added is very cool, it's just to change how to use it, but we keep your ideas. Great Job!

@Matias Mendiola (mendio) what name we can use for the default mode (actual)...for Layers, Strokes and Material the name is obvious.

This revision now requires changes to proceed.Jun 3 2022, 7:57 PM

Sure, I can make changes over this weekend. So how should elements be ordered? Do we keep general offset and random offset as is, then add mode dropdown and have those 3 options?
In my tests it was the case with Suzzane that I needed to combine stroke offset and layer offset. Now those things are in panels one above the other, which is convenient to have in one view.
If we split this in more modifiers it will be more confusing to use, because we'll have general and random offset of the second modifier always in between. with panels you can just collapse what is not needed and have this way clearer understanding of the offset combination. I would also assume that having more modifiers to achieve the same thing adds some burden to performance, as general offset matrix is always calculated.. I think.

So before proceeding I'd like to hear it directly from the other members. Just to make sure we're covering all the cases.
Thanks for the feedback!

Removed new panels and made Mode dropdown instead
Added step & start offset parameter for Stroke mode
Renamed Randomize panel -> Advanced

To do:

hide control those parameters where not needed

Currently the code is just commented out in case of further reasoning on this topic.

It would be best if we could get also general offset in Modes, that way the UI will be even more clear. But this has backwards compatibility implications.
Same goes for the variable names. Currently it is reusing all parameters form randomize. If we make it more general then we loose backwards compatibility.

Further work will include scaling down stroke offset mode, currently the sliders act too aggressively compared to other modes (multiplied by stroke count)

moved general offset settings to mode:

normalized stroke offset so the last stroke is offset by position parameter
added offset towards custom object, it is reacting to it, but the calculation is not exactly right.
fixed steps parameter, so the offset stays inside bounds

I need to fix this patch for latest master, but running into some merge conflicts.

@Aleš Jelovčan (frogstomp) Yes, the patch is broken and includes many unrelated changes.

correct clean patch for latest master

updated to master

hold... changed sometihing with lineart camera. will fix

Awesome promissing modifier changes, A question, is randomize only affecting strokes ?

is the selected Mode will be the only offset, or this modes idea is only a way to select the expanded panel "due to visibility issue"?
IMHO i think randomize mode utility may be useful more if is not an indepandant mode but is integrated panel in every mode(Layer , stroke ,Material ... maybe in future created vertex groups ) as panel like we have already in offset modifier.
Any news on this patch ?

@hamza.SMA (hamza.elbarmaki) hi,

No news on this patch, have not done any work due to vacation. Aiming for 3.4.
Currently we need to decide on some usability issues, namely backwards compatibility. Because internally "General mode" uses one set of variables and others use different set. This is because originally it was designed like this.
This means that now if you switch type, general mode will not use the same numbers as the others.. feels weird.

The idea here is with modes, you can add multiple modifiers of this kind and chain them like this. Visually it's better because you can see all variables in one place.
Open for other suggestions :)

@Aleš Jelovčan (frogstomp) , hi
enjoy your vacation,

Open for other suggestions :)

i have a suggestion,but because i have math background speciality so probably is not useful but can be for cleaning drawing or maybe a motion graphic usage cases.

in stroke mode : in order to offset only small /big strokes( they will have less/more vertex) we will need to set a custom range influence about number vertex that form the stroke ,
we can have a custom curve for this range and the values in loc/rot/scale act as factors for this curve "i can see this option will be useful for creating animation as using proportional editing in viewport but animated, i don't know if my imagination is reached but i hope".
sorry for my bad english.

updated to master
all is controlled with one set of variables

to do:

unused rna controls cleanup
figure out what to do with object control
apply step and offset to layers and materials

ideally we would be able to set origin towards which the transforms go with object selector. now it always goes towards the object center

@Aleš Jelovčan (frogstomp) I'm testing your patch and IMHO we have a missunderstanding here. The Offset has two types... the general Offset and the randomize values.

The general Offset is used a lot with drivers and to move things, and the random to create variations. IMHO we must keep the general section (including Loc/Roc/Scale) and add the new randomize type to only the Random section... is in this section were we can random by stroke, by material, etc....

@Matias Mendiola (mendio) @Daniel Martinez Lara (pepeland) what do you think?

I think you're on the right track, but the design just needs a bit of polish.

Separated general and randomized offset in panels
Cleanup of modes

TODO:

figure out math behind object selector

@Antonio Vazquez (antoniov) hi, is the base of those updates now correct? Keeping two pairs of variables, I have only put the general ones inside a panel, so user can fit all the relevant data in viewport when stacking multiple modifiers of that kind.
We should also think of the naming of the now kept "Randomize" panel.. maybe Advanced?

I am clueless about the camera/object transformation, so we can also remove it from that patch.

@Aleš Jelovčan (frogstomp) I have not tested the patch but your UI now looks right. Agree about camera/object... better do it in a separated patch.

For UI names, @Matias Mendiola (mendio) can help us here.

removed offset towards object ui
added step and entity offset for material and layer offset
math correction so that the first stroke stays stationary
bug and crash fixes by handling exceptions

Great work! The code in general is OK, just review my comments above. The functions to find index or count elements is very slow, so we must try to reduce the use (if you can).

Waiting for @Daniel Martinez Lara (pepeland) and @Matias Mendiola (mendio) about the usability and UI.

source/blender/gpencil_modifiers/intern/MOD_gpenciloffset.c
136	Not sure of the logic...but do you need find the index for each loop? maybe you could do that outside the loop and reuse the value in the loop.
149	Do you need count materials for each loop? The listbase_count is a very expensive function.
160	Save as above about listbase_counf and findindex

Antonio Vazquez (antoniov) added inline comments.Dec 31 2022, 10:47 AM

source/blender/gpencil_modifiers/intern/MOD_gpenciloffset.c
160	I mean "Same" not "Save"...the autotranslator does these things

put index fetching code and check for sizes to correct loop
unified clamping method with max_ii
set proper use of variable types
cleanup and readability improvements

Some notes:

The Offset General setting should be always visible and not inside a panel. This settings are the simplest ones and always have to be at hand

Stroke Step and Stroke start offset name and tooltip are the same, please update the tooltip to clarify the use of this controls.

It seems to be an inconstancy in the X, Y, Z axis offset between the different Modes. For example in Layer Mode a 1.5 value bring strokes in a positive direction and in Stroke Mode the same value move the layer backward in a negative direction

This revision now requires changes to proceed.Jan 4 2023, 11:10 PM

unified offset behavior for the 3 new modes
set general panel to be opened by default
separated step and start parameters in multiple labels and added more descriptive tooltip
refactored shared variable names
if -> case

@Matias Mendiola (mendio) thanks for review. I managed to organize and simplify the code a lot, the calculation is now done the same way for all three new modes.
I am open for better tooltips, just note that it is possible to change the label for each, but tooltip needs to be general for all of them.
Otherwise we really need to separate lots of parts into multiple variables and elements, which is really not ideal from coding perspective.

I have set General panel to be opened by default. If you really insist on having this move out of panel to be always on, this of course is possible, but I think we are forcing one workflow onto user, who might have different workflow.

When adding those 3 modes I wanted to solve overlap issues when switching from 2d stroke mode to 3d. Sometimes you need this is you want to create crowd with array modifier.
In some cases you need to use more than just one mode to achieve this. Combination of layer and stroke offset.
In this case you would have 2 Offset modifiers, and If General is always opened, then it gets in the way, you will have trouble fitting it in view.

The same logic you can also see in Array modifier, where Relative offset is opened by default.

Code looks better now, just few style changes.

What we need are better tooltips.

source/blender/gpencil_modifiers/intern/MOD_gpenciloffset.c
200	1.0f
286	Material Offset. Usually, we capitalize first letter for all words.
290	Layer Offset
source/blender/makesrna/intern/rna_gpencil_modifier.c
1522	@Matias Mendiola (mendio) Maybe you can find better tooltips.
1649	Start Offset

Nice work! I'll definitely play with it when landed in master.
Tiny, tiny remark about the UI text labels: the Blender style is to capitalize all words, so Stroke Step, Stroke Offset and so on.
And in line 282 of MOD_gpenciloffset.c a typo: Stroke instead of Stoke.