Page MenuHome

Add quadriflow remesher
ClosedPublic

Authored by Sebastian Parborg (zeddb) on Mon, Aug 26, 6:39 PM.
Tags
None
Tokens
"Love" token, awarded by belich."Love" token, awarded by amonpaike."Love" token, awarded by tobkum."Mountain of Wealth" token, awarded by franMarz."Love" token, awarded by bnzs.

Details

Summary

I've integrated the QuadriFlow remesher into blender:
https://github.com/hjwdzh/QuadriFlow

Perhaps we should rename the voxel_remesher files to simply remesher or something?

Diff Detail

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
extern/quadriflow/CMakeLists.txt
2–3

Please make this file similar to e.g. extern/glog/CMakeLists.txt.

  • Don't check minimum cmake version or define a new project here.
  • Use blender_add_lib with name extern_quadriflow.
  • Use SRC, INC and INC_SYS variables.
  • Etc.
45–73

Remove the build flag checks and just enable what we use.

113

Remove this, it's not needed.

source/blender/editors/object/object_remesh.c
218
/home/brecht/dev/worktree/source/blender/editors/object/object_remesh.c: In function ‘quadriflow_remesh_exec’:
/home/brecht/dev/worktree/source/blender/editors/object/object_remesh.c:218:30: warning: passing argument 1 of ‘BKE_mesh_smooth_flag_set’ from incompatible pointer type [-Wincompatible-pointer-types]
     BKE_mesh_smooth_flag_set(ob, true);
                              ^~
In file included from /home/brecht/dev/worktree/source/blender/editors/object/object_remesh.c:44:0:
/home/brecht/dev/worktree/source/blender/blenkernel/BKE_mesh.h:186:6: note: expected ‘struct Mesh *’ but argument is of type ‘Object * {aka struct Object *}’
 void BKE_mesh_smooth_flag_set(struct Mesh *me, const bool use_smooth);
      ^~~~~~~~~~~~~~~~~~~~~~~~
Brecht Van Lommel (brecht) requested changes to this revision.Thu, Aug 29, 7:22 PM
Brecht Van Lommel (brecht) added inline comments.
extern/quadriflow/CMakeLists.txt
113

This is actually causing a crash on startup here, I guess due to TBB being linked twice.

If quadriflow depends on TBB being enabled, it should handle that in setup_liblinks in macros.cmake.

This revision now requires changes to proceed.Thu, Aug 29, 7:22 PM
LazyDodo (LazyDodo) requested changes to this revision.Thu, Aug 29, 8:20 PM

Mostly minor stuff for windows.

extern/quadriflow/CMakeLists.txt
10

I don't think we have moved to c++14 just yet, is 14 absolutely required or will our current baseline of c++11 work?

19

MSVC needs to following to get M_PI and friends

if (WIN32)
    add_definitions(-D_USE_MATH_DEFINES)
endif()
111

${LEMON_SOURCES}

extern/quadriflow/blender_config.cmake
13 ↗(On Diff #17587)

BOOST_INCLUDE_DIR all uppercase, no S at the end.

intern/quadriflow/CMakeLists.txt
41

MSVC needs to following to get M_PI and friends

if (WIN32)
    add_definitions(-D_USE_MATH_DEFINES)
endif()
source/blender/editors/object/object_remesh.c
218

This is a straight up build error on windows.

extern/quadriflow/CMakeLists.txt
10

Yes, not having this results in a build error on my end.

113

I doesn't depend on it with our build options, so I think I can simply skip linking it.

source/blender/editors/object/object_remesh.c
282–283

Specifying an absolute number of faces is not very easy, it's hard to estimate how many you need.

I think we can change this to a percentage of the number of faces in the input mesh. Seems more intuitive to say you want 10% or 400% of the number of faces?

source/blender/editors/object/object_remesh.c
282–283

I don't think having it as a percentage would be very useful.
Sculpt meshes are usually very messy and the face count can vary quite a lot.

I think it is best to just have an approximate number like it is now. The face count is very loosely specified anyways.

Perhaps it would be better to expose the target edge length of the quads?

source/blender/editors/object/object_remesh.c
282–283

The current solution requires the user to find the number of faces the mesh has and do the math manually, or to use trial and error to find an appropriate number of faces. Being able to say "more faces" or "fewer faces" is easier.

Being able to specify the target edge length is fine as an alternate option, but I still think a percentage is the most convenient option to use by default. For target edge length you again have to look at the dimensions of the object and do some math or use trial and error.

source/blender/editors/object/object_remesh.c
282–283

The current method does not require the user to do the math manually.

If we do a percentage as you suggest then people will have to look up the face count and do math in their head.

I don't see how it would be easier than simply a target number of faces.

Sebastian Parborg (zeddb) marked 12 inline comments as done.Wed, Sep 4, 5:37 PM
Sebastian Parborg (zeddb) updated this revision to Diff 17819.

I've updated the patch with your feedback.

@LazyDodo (LazyDodo) can you see if you get any build errors now on Windows?

@Brecht Van Lommel (brecht) I left out the new face count modes. I just wanted to tackle the build related issues/feedback first.
I will start working on those changes when you think that the build side of things is up to snuff.

I forgot to mention that I think I have managed to make the "preserve sharp" feature not crash anymore. At least with my test cases I haven't been able to get it to crash. (But that doesn't mean that it produces a good result in all of the cases)

LazyDodo (LazyDodo) accepted this revision.

Can confirm there's no more build issues on windows, however we may want to ping @Sergey Sharybin (sergey) on the cpp14 requirement.

Only looked at the build system stuff, seems fine with some minor comments.

CMakeLists.txt
266

This option also needs to be added as info_cfg_option at the end of this file.

extern/quadriflow/CMakeLists.txt
16

lowercase configure_file( for consistency

34–35

Boost and Eigen headers should go to INC_SYS

Sebastian Parborg (zeddb) marked 3 inline comments as done.Thu, Sep 5, 2:24 PM
Sebastian Parborg (zeddb) updated this revision to Diff 17860.

Updated with requested cmake changes.

Sebastian Parborg (zeddb) marked 2 inline comments as done.Mon, Sep 9, 7:50 PM
Sebastian Parborg (zeddb) updated this revision to Diff 18009.

Sorry for the delay, now everything should be fine I hope.

Brecht Van Lommel (brecht) requested changes to this revision.Tue, Sep 10, 2:58 PM

Blender freezes or even relatively simple, it should remain responsive. Otherwise it looks like Blender might be hanging to users trying this operator for the first time.

We might not be able to provide useful progress with quadriflow currently, but it should be able to do something basic to keep the UI responsive with the jobs system. You can look at e.g. the ocean modifier bake operator for an example.

extern/quadriflow/CMakeLists.txt
2

Add license header and suppress warnings:

diff --git a/extern/quadriflow/CMakeLists.txt b/extern/quadriflow/CMakeLists.txt
index bb794e3..488d95a 100644
--- a/extern/quadriflow/CMakeLists.txt
+++ b/extern/quadriflow/CMakeLists.txt
@@ -1,3 +1,30 @@
+# ***** BEGIN GPL LICENSE BLOCK *****
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software Foundation,
+# Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+#
+# The Original Code is Copyright (C) 2019, Blender Foundation
+# All rights reserved.
+# ***** END GPL LICENSE BLOCK *****
+
+# avoid noisy warnings
+if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_C_COMPILER_ID MATCHES "Clang")
+  add_c_flag(
+    "-Wno-unused-result"
+  )
+endif()
+
 set(CMAKE_CXX_STANDARD 14)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
intern/quadriflow/CMakeLists.txt
18

2015 -> 2019

intern/quadriflow/quadriflow_capi.cpp
33

obj_vertex -> ObjectVertex

38

inline keyword is unnecessary for these class methods.

53

obj_vertexHash -> ObjectVertexHash

65

Add comments in this function to explain what the various steps do, so it's easy to get an overview.

/* Get remeshing parameters. */
...
/* Copy mesh to quadriflow data structures. */
...
...
intern/quadriflow/quadriflow_capi.hpp
27

typedef struct{ -> typedef struct QuadriflowRemeshData {

Also make sure to use clang-format for commit.

47

quadriflow_remesh -> QFLOW_quadriflow_remesh

All lowercase is usually reserved for static functions, and we use these uppercase prefixes for separate modules.

release/scripts/startup/bl_ui/properties_data_mesh.py
479

Quadriflow Remesh -> QuadriFlow Remesh, or just remove the text because it's the same.

The Remesh panel contents is confusing now. It's not clear that the properties only apply to voxel remesh. One improvement would be to rename "Voxel Size" to "Edge Length", and then use the mesh properties as default values for the QuadriFlow operator properties.

source/blender/blenkernel/intern/mesh_remesh_voxel.c
154

Add comments to this function as well.

161

Free looptris when they are done being used in this function.

195–196

Are these comments still relevant?

I guess the quality isn't as good as we want, but probably not unstable?

source/blender/editors/object/object_remesh.c
161–163

QUA_ -> QUADRIFOW_REMESH_

300–306

Input target number of faces in the new mesh
Specify target number of faces relative to the current mesh
Input target edge length in the new mesh

In general, try to use consistent terminology and avoid implementation details. Right now there is a mix of current/original, amount/number/count.

315

Calculates -> Create

334

Preserve sharp -> Preserve Sharp, and similar capitalization for other properties.

338

Does "border" mean edges with only a single adjacent face? If so, call this "boundary" for consistency with edit mode tools.

347

Can you explain in more detail what adaptive scale means?

364

Number of faces should not be the default value, should be either edge length or ratio.

365

Face Count Mode -> Mode

366

How to calculate the number of output faces -> How to specify the amount of detail for the new mesh

374

The face count ratio in respect to the original mesh -> Relative number of faces compared to the current mesh

379

target_edge_len -> target_edge_length

384

The desired edge length on the output quads -> Target edge length in the new mesh

393

Num of quads -> Number of Faces

394

The approximate amount of faces (quads) on the new mesh -> Approximate number of faces (quads) in the new mesh.

395–396

Remove this part of the comment, set the minimum value to 1 and let users use Ratio for that instead.

398

255 -> INT_MAX

404–409

10.0f -> FLT_MAX

422–427

This property only really makes sense in the "adjust last operator" panel, and even then it's a hack.

I suggest to just remove this property as the operator displays a popup and multiple values can be tweaked without triggering a recompute.

This revision now requires changes to proceed.Tue, Sep 10, 2:58 PM
Sebastian Parborg (zeddb) marked 29 inline comments as done.Wed, Sep 11, 4:19 PM
Sebastian Parborg (zeddb) updated this revision to Diff 18104.
Brecht Van Lommel (brecht) requested changes to this revision.Wed, Sep 11, 5:54 PM

This currently fails:

  • Add monkey mesh.
  • Use Quadriflow Remesh
  • Number of Faces shows 1 (should be actual estimate)
  • Execute operator
  • Empty mesh, but message claims remesh successful
release/scripts/startup/bl_ui/properties_data_mesh.py
473–481

Add , expand=True so it's easy to see the available options at a glance.

source/blender/makesdna/DNA_mesh_types.h
267–268

Use = 0, and = 1,. No need for bitflags, plus it does not get initialized correctly now since this is 1 and 2.

source/blender/makesrna/intern/rna_mesh.c
59–60

Voxel Remesher -> Voxel
Quad Remesher -> Quad or (Quadriflow?)

This revision now requires changes to proceed.Wed, Sep 11, 5:54 PM
Sebastian Parborg (zeddb) marked 4 inline comments as done.Wed, Sep 11, 6:40 PM
Sebastian Parborg (zeddb) updated this revision to Diff 18116.

Hopefully everything is fixed now. *fingers crossed*

Be sure to add release notes along with the commit.

This revision is now accepted and ready to land.Thu, Sep 12, 3:47 PM