Page MenuHome

Sculpt: Clay Thumb Brush
ClosedPublic

Authored by Pablo Dobarro (pablodp606) on Nov 12 2019, 11:24 PM.
Tokens
"Love" token, awarded by matic."Dat Boi" token, awarded by shader."Love" token, awarded by ilia_kole."Love" token, awarded by amonpaike."Love" token, awarded by lopoIsaac."Like" token, awarded by hitrpr."Love" token, awarded by TheCharacterhero."Love" token, awarded by ebarranco."Love" token, awarded by Maged_afra."Love" token, awarded by christianclavet."Yellow Medal" token, awarded by threedslider."Love" token, awarded by ThomasJohann."Love" token, awarded by ofuscado."Love" token, awarded by brilliant_ape."Love" token, awarded by andruxa696."Love" token, awarded by RodDavis."Love" token, awarded by TheFlow."Burninate" token, awarded by Alrob."Love" token, awarded by franMarz."100" token, awarded by Frozen_Death_Knight."Love" token, awarded by xrg."Love" token, awarded by tiagoffcruz."Love" token, awarded by Brandon777.

Details

Summary

This brush simulates deforming clay with your fingers, accumulating
material during the stroke. It has a plane that tilts during the stroke
in the front part of the brush to achieve this effect.

Diff Detail

Repository
rB Blender

Event Timeline

Dalai Felinto (dfelinto) added inline comments.
source/blender/editors/sculpt_paint/sculpt.c
5759

Could we have any comment explaining what this function is doing, and the whys?
The comments can be populated throughout the code after an initial overall big picture.

5804

Suggestion: const float tilt_mix = local_co[1] > 0.0f ? 0.0f : 1.0f;

5833

Same here. This function is the core function of the brush and is severely under commented.

5842

Please follow code style for comments (Starts with a capital letter, ends with a period)

Pablo Dobarro (pablodp606) marked 4 inline comments as done.
  • Add comments, review update
Jeroen Bakker (jbakker) requested changes to this revision.Dec 20 2019, 8:20 AM

Code wise Just some small stuff. I do have to agree that the working of (sculpt) brushes isn't well documented and we should spend time adding documentation.
Note that commenting code is not adding subtitles in comment blocks like in this patch. That is useful but still very limited.

Things to be aware of when writing comments:

  • Code is typically written once, but read many times. So the time someone needs to spend on looking what exactly happens should be optimized
  • Start how the brush works from a functional (user) way and refer to important variables/functions in the code.
  • Document variables/attributes and parameters
    • like the clay_add variable, is unclear to which brush it belongs to as it has a very generic name and is not categorized like other settings. You could add info to which brush it belongs, why the value exists (eg. The rotation that is added to the sculpt plane to simulate clay addition) It is better to document where it is defined.

I can imagine that describing the attributes would give a good starting point how a brush works.

source/blender/editors/sculpt_paint/sculpt.c
5791

Is there a reason why we don't store the rad in clay_add field?

5821

1.0f

5896

don't use int when they are float

source/blender/editors/sculpt_paint/sculpt_intern.h
398

think this is a bad name.

This revision now requires changes to proceed.Dec 20 2019, 8:20 AM
Pablo Dobarro (pablodp606) marked 2 inline comments as done.
  • Clay Thumb: Pressure Stabilizer
source/blender/editors/sculpt_paint/sculpt.c
5791

All other brushes use degrees to store the angles. It is easier if in the future I need to expose some of these variables as a property in the UI.

All artifacts are now fixed and the brush input samples number does not longer affect the brush behavior. It now has a second input sample buffer to stabilize the pressure and the radius variation, as well as configured input curves.

This revision is now accepted and ready to land.Jan 22 2020, 11:40 AM

(remember to mention this in https://wiki.blender.org/wiki/Reference/Release_Notes/2.83 ideally with image or video, and enough information so @Aaron Carlisle (Blendify) can help adding this to the 2.83 manual).

at this point blender must necessarily be brought to android and ipad ... hehehe

Mr (Kestrel) removed a subscriber: Mr (Kestrel).
This revision was automatically updated to reflect the committed changes.