Page MenuHome

Multires: Implement edge boundary automasking
AbandonedPublic

Authored by Pablo Dobarro (pablodp606) on Mar 17 2020, 6:38 PM.

Details

Summary

This implements the multires part of sculpt_vertex_is_boundary sculpt
API function. Edge boundary automasking should now work in multires the
same way it works on meshes and bmesh.

Diff Detail

Repository
rB Blender
Branch
multires-boundary-automasking (branched from master)
Build Status
Buildable 7198
Build 7198: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) requested changes to this revision.Apr 2 2020, 10:08 AM
Jeroen Bakker (jbakker) added inline comments.
source/blender/blenkernel/intern/subdiv_ccg.c
1784

No need for the many indents. Every branch ends with a return statement. So else doesn't need to be added. Saves some code and indents.

1786

BLI_assert(IN_RANGE_INCL(coord->grid_index, 0, subdic_ccg->num_grids-1))
but that adds an assumption that num_grids cannot be 0

BLI_assert(IN_RANGE(coord->grid_index, -1, subdic_ccg->num_grids))
is less clear.

IMO the IN_RANGE is a bit of a weird macro as it isn't doing what I expected it would be doing. It is more a IN_RANGE_EXCL.

1839

typo

This revision now requires changes to proceed.Apr 2 2020, 10:08 AM

Can only comment on BKE_subdiv_ccg_is_geometry_boundary.

General notes:

  • Explain what geometry boundary for CCG is. Add this in front of the function definition.
  • Don't use else after return. That only causes unnecessary indentation.
  • Consider moving logic from if-else chains to a funcitons. For example:
if (is_corner_grid_coord(...)) {
  return is_geometry_boundary_for_corner_grid_coord(...);
}

Some extra reading: https://clang.llvm.org/extra/clang-tidy/checks/readability-else-after-return.html

Sergey Sharybin (sergey) requested changes to this revision.Apr 2 2020, 10:23 AM

This is already implemented in 2.90 in a different way