Instead of accumulating displacement for each vertex into the neighbors, accumulate the opposite displacement from each neighbor into the vertex. I think this is the same and it does not produce artifacts in Multires.
Details
Details
Diff Detail
Diff Detail
- Repository
- rB Blender
Event Timeline
Comment Actions
- Multithread the filter
After this change there is no need to calculate the displacement on a first sigle threaded iteration, so the entire filter can run per vertex in parallel as any other filter
Comment Actions
I think this is the same and it does not produce artifacts in Multires.
If it's the same then why the artifacts are fixed?
In general, what is the issue you're solving here, how to reproduce it?
| source/blender/editors/sculpt_paint/sculpt_filter_mesh.c | ||
|---|---|---|
| 360–361 | float disp_sharpen[3] = {0.0f, 0.0f, 0.0f}; | |
Comment Actions
@Sergey Sharybin (sergey) This is the artifact this fixes. As you can see, the grid pattern is clearly visible when using the filter in multires because on the previous implementation duplicate vertices in grid's boundaries were applying the displacement twice towards the same location.
Comment Actions
One thing which is hard to me to easily see from the code is whether you're reading and writing from/to the same vertices array.
Usually when you're modifying values in some data array based on neighborhood you need to either have a duplicate of original data or write to a separate array. Otherwise, after you've handled element index 0 you would use its modified value when handling element index 1. See what I mean here?
Comment Actions
Yes, but that is currently how all the filters, smooth and relax brushes work (except for HC smooth I think). Doing it correctly will require making a copy of the current coordinates (and normals in some brushes) of the whole mesh per stroke step and per smooth iteration. These brushes already have some important performance issues, so I'm not sure if we should consider doing this without having faster smoothing algorithms or some other optimizations in the sculpt code to compensate for the performance loss.
Comment Actions
Not sure what do you mean by "filters" here, but a lot of brush tools do require to have original vertices (Layer is one of them), and some stroke settings also requires original coordinates (anchored stroke). This is done as a combination of sculpt undo system and PBVH proxy system. You don't need to store original data for the entire mesh, you on;y need to do so for nodes which are actually changing.
I am sure you can implement a proper solution without measurable performance hit. I am unconvinced we should be accepting intrinsicly wrong algorithm, which is prone for giving unpredictable artifacts depending on how artists' system decided to schedule threads.
P.S. There are also no mention of anything related on the performance in your patch. Is it bad presentation of all aspects of the change? Or is there no measurable performance gain?
Comment Actions
I created this task D7911 to discuss the read/write problem. Proxies will affect performance a lot on any brush and filter that requires multiple iterations as the combine proxies task needs to be executed after each iteration to read the correct coordinates in the next one, which is basically the same issue as in the patch I proposed. The smooth brush was working like this (skipping the proxies and reading/writing at the same time) since 2.79.
Comment Actions
Accumulating into the neighbors does not work, as there are duplicated vertices coordinates and with the old code only one of those duplicated ones will be written to. Going in the other direction as in this patch solves that.
The problem of original coordinates remains, but this is a net improvement I think.