Note: If you're planning to join the meeting, feel free to add talking points to the agenda. ## Attendees - Dalai - Julien - Sergey - Daniel - Hans ## Announcements - [2023-07-13 Texture Painting Polishing Workshop](https://devtalk.blender.org/t/2023-07-13-texture-painting-polishing-workshop/30284) A small workshop to discuss possible targets for improving the texture painting experience. No definitive goals were set yet. The meeting notes will be used to align short term targets with core developers becoming availible to work on the module. - Joe Eagar's contract ended. Ton's statement: > For personal reasons, Joe Eagar, one of the Sculpt/Paint module team members, was asked to step aside. His development grant (which lasted 18 months) will be ended by the end of July as well. ## Agenda ### Needs Commit ### [High Priority Bugs](https://projects.blender.org/blender/blender/issues?q=&type=all&sort=&state=open&labels=281%2c296%2c285%2c290&milestone=0&project=0&assignee=0&poster=0) - Julien will go over repeat setting and view plane bug. - Hans can look into "Dyntopo Smooth Shading Bug" ### Current Topics - Discusssing the current state of the module and how to continue in the short term. - Do we continue with meetings for now? - Remaining development targets for 4.0 including high priority fixes. - Daniel: Layered sculpting and multires would be more worthwhile targets with more use cases (Sculpting corrective shapes for animation) - Also Roll brush patch is highly requested to be wrapped up - Julien: Those should be the next targets for sculpting. But Dyntopo is good to wrap up. - Sergey: Patch is mostly done. A lot of time was spend so we try to get as much into main as possible. Would also love multires improvements! - Dalai: Dyntopo and multires will take time away from other developers. We need to align projects vs module work. - Hans is willing to branch out to sculpting module (technical topics/features and performance and fixing technical debt) - Will fix bugs and open doors for new features. - Area of experience is more for mesh and sculpting than texture and udims. - For now no module meetings necessary. Module is in an orphaned state with focus only on minimal maintainance of high priority issues and documenting. - Sergey is availible for important issues - Julien will keep an eye on current issues and state of module - Dalai: Focus on defining strategic tragets to have poeple use Blender for sculpting (Not just for rendering and modeling) - Daniel & Julien will work out the most important needed improvements (Like layered scupting) - Sergey can work on Multires fixes once able. - Daniel & Julien will work on Brush Essentials Assets (Primary work in short term) - Julian and Bastien can help with porting tools to brushes in the code - Aligning resources and developer time to finish the Dyntopo PR. - Although the rest of the team is busy now, there may be time soon to tackle this as an internal project (if it is still waiting then) - Sergey will still take a few feeks to wrap up compositor work as well. - Hans is also taking time off in August. * Dynamic Topology Split Strategy * Dalai asked Sergey to analyze how he would suggest the patch to be split (see later in this document). * This is intended as to map out what the initial future step would be for the development team. * But it can also serve as a guideline in case Joe wants to keep iterating on the patch (given that he did a few updates on it since last week). ### Other Topics ------ ### Dynamic Topology Project #### General notes The presentation of the project needs to be updated. The project is not about refactoring, but about bringing new functionality: performance and attribute preservation. Refactoring by its definition does not change an external behavior. The description needs to be updated to become a single-read self-sufficient information. Start with the description of the problem the patch solves, description how it is done. Referring to state of other branches and their history should be avoided in the general part of the PR description. It is fine to have such information later on (after the "---" marker). It is important to give context about such referring, because reviewers are not familiar with the state and history of code outside of the PR. Inspirational read: - https://wiki.blender.org/wiki/Process/Contributing_Code#Ingredients_of_a_Pull_Request - https://dev.to/karaluton/a-guide-to-perfecting-pull-requests-2b66 #### Splittable parts Based on a quick pass over the patch and personal experience of structuring projects. ##### NotForPR Changes marked as `NotForPR` should be removed prior to asking for a review. ##### startup.blend It is hard to tell what exactly is changed in the file. A lot of versioning is to be done in the `versioning_default.cc`. There might be a good reason to update the file, but more information about it is needed. ##### MSVC + Clang changes Similar to other platforms and compiler configuration we are open for compilation error fixes. That being said, the compilation fix needs to be submitted and presented separately. ##### BKE_dyntopo_set.hh There are likely other places where iteration over unique set of elements is needed, and the implementation does not seem to be bound to Dynamic Topology. Should consider: * Make it a BLI primitive * Cover it with regression tests * Include (rudimentary) benchmarking, similar to the BLI_set_test.cc ##### BLI_heap_minmax.hh Move to a separate PR and consider: * Expand the description. I.e. it states it does priority queue, so how is it comparable/different from std::priority_queue. * Cover with regression tests ##### BMesh edge collapse The current PR description states that the current algorithm in the main branch has bugs. Having a more robust edge collapse is a general good improvement to have outside of dyntopo. It should be presented and reviewed from this pesrpective. The implementation probably needs a re-iteration, as it is a bit strange to generate specializations of such non-trivial algorithm for all types of callbacks. ##### Edit BMesh fair vertices It does not seem to be used in the patch, and the intent is not really clear. If this is a new operator which is intended to be exposed it should go into a separate PR. ##### CD_FLAG_TEMPORARY Such semantic change should be isolated to own incremental step. This is likely to touch some API changes in the BKE_attribute.h and BKE_customdata.h ##### ATTR_NO_OPT Either needs to be completely removed, or presented as a separate PR. It was mentioned the change was rejected, but I did not find a PR to get more context on this topic. From personal experience sometimes it is handy to be able to disable optimisation of a specific function during development/troubleshooting. Being able to do so without looking up for an exact syntax sounds good. So to me it does seem to be a reasonable thing to have. ##### BLI_mempool.h The patch introduces `BLI_mempool_get_size`. Is not very clear what the difference from `BLI_mempool_len` is. If it does something else it needs to go to a separate review. ##### BMesh tool flags The belief is that it is possible to extract this part of the patch, keeping it no-functional-changes for the current state, and review it from this perspective. Doing so will give ability to present the need of such change and understand it and validate more efficiently, as well as removing bulk of changes related on the API change. ##### geometry_attributes.cc / mesh_data.cc From reading the code it seems that it is actually a bug fix to something that is already in the main branch: make undo in sculpt mode do what one expect it to do after adding attribute. If it is indeed so, the change needs to be split and explained what the fix mean on user level. ##### join_mesh_single It seems that this is a change which Blender benefits from outside of the dynamic topology project. So seems reasonable to split the change out and review it by the modelling team. ##### space_image folder Seems to be a lot of changes, and not needed for the dynamic topology project. ##### Auto-save changes Needs to be split out, and seems to be already submitted as #110088 The issue is the performance and UX. #### Closing notes Surely the notes above do not cover the juicy parts of the actual dynamic topology changes. The goal so far was to bring the patch in an easier to review state. With the size of this patch it takes a lot of mental energy to even go over those easy-to-separate changes, and it will be an iterative process to tackle the entirety of the patch.