Change Details

This task will describe the engineering parts of vertex painting in sculpt mode. The reason for this is that the current patch is bloating and IMO that was not necessary. The goal of this patch is to come to a clear design that then can be implemented.

This task will describe the engineering parts of vertex painting in sculpt mode. The reason for this is that the current patch is bloating and IMO that was not necessary. The goal of this patch is to come to a clear design that then can be implemented. The state of this design is still in progress. Only start implementing this when everyone agrees upon these changes. = Issues = The current patch has several issues in how the original design was translated into code. To name a few: **Architectural Issues** * Current patch assumes that vertex painting will be merged into sculpt vertex painting. But in the original design it is mentioned that vertex painting will be merged into attribute painting. * Current patch takes steps into supporting `MVertCol` to render engines. It thereby takes precedence control over the rendering of vertex painting and thereby limits current functionality. * The workflow for the user for the current patch is not well communicated to the user. **Process Issues** * On top of the undecided implementation new features are still added on a weekly basis, making the code review process totally not efficient. People are spending a lot of time in reviewing and all needs to reviewed the week after, where new features are added, where it is unclear if the issues that were addressed in the previous review are handled correctly. **Implementation Issues** * Color management: user is painting in display space. * Vertex colors are actually averaged into a face color. But in the UI we say Vertex Colors. It is unclear why the colors needs to be averaged. IMO in sculpting you have dense meshes and you don't see the difference. But perhaps there are cases why this is needed. If so we should describe these use cases. * Cycles and EEVEE rendering in current patch are broken. * It is currently required to have a `MVertCol` and `MLoopCol` for rendering in sculpt-mode. I would expected that only one would be needed. As `MVertCol` has more precedence this makes it impossible to do any Vertex Painting. = Alternatives = Personally I miss the information why the Sculpt Vertex Painting cannot use the `MLoopCol`. If this is possible the workflow for the user is more clear and the implementation does not have so many issues/limitations. **Alternative 1: current sculpt patch** This should describe the current implementation as of end of 2019. As more and more features are added it is not easy to extract an design from the current implementation. The issues described above outlines the current workings/limitations. Perhaps Pablo can put in the ideas here. **Alternative 2: expectation how I expected the original design to be implemented** Current design is a goal implementation where attribute painting has been added. As we currently don't have a clear understanding how attribute painting will look like this we should not make any assumptions yet on the implementation. Therefore my expectation was a hard separation between `MVertCol` and `MLoopCol`. Both could be used during rendering and an operation to sync between the active `MVertCol` and `MLoopCol`. Current patch has parts of it, but as the current development is limiting functionality it is unclear if this alternative is actually what is the goal for = Roadmap = **Long term road map** * Get the basics correct with the next use cases in mind ** A sculptor wants to paint during sculpting. ** A modeller/lighter wants to use vertex painting as it can hold more precise data. * Design attribute painting * Implement attribute painting ** Will merge current vertex painting into attribute painting **What should be done in the current patch** Focus on the bare minimum and the stuff that has been agreed upon. Don't add fancy stuff on top of it, before we settle on the basics. As in the initial design is mentioned that sculpt vertex painting is an implementation for an edge case, so should not take control of the other implementations as it currently does. * It must be possible use sculpt vertex colors without vertex colors and vise versa. * Remove the face averaging. It feels like a hack where users has no control over and communicates the wrong information to users. Or we should come up with an actual user benefit. If we were able to use `MLoopCol` this isn't a problem, but as we are not storing the data here this limitation introduces some edge cases with incorrect feedback to users. The later could also be solved by user documentation, but as there isn't any design or user documentation at all... * As `MVertCol` is for artistic drawing only, I would not expect that it has precedence over vertex painting. The user should be able to control which layer is displayed (by the name). `MLoopCol` should have more precedence then `MVertCol` as it holds more data and use cases and is also the expected format for modellers/lighters. * Change the default name of sculpt vertex colors (Col) to something that does not clash with regular vertex colors. * Change the Attribute/Vertex Color Shader node to support both vertex colors (or any shading stuff) * Fix existing the crashes ** Activate vertex painting mode (No vcol layer available in vertpaint, but batches requested anyway!) * If you want to support alpha rendering in the workbench we should use the transparency_accum buffers. The current implementation a hack to get it working in the viewport. This should be done in a separate patch. * Usability: When starting sculpting and there is no `MVertCol` layer, a default needs to be added. * Usability: When Sculpt painting brush is active the context menu should have a color picker.

This task will describe the engineering parts of vertex painting in sculpt mode. The reason for this is that the current patch is bloating and IMO that was not necessary. The goal of this patch is to come to a clear design that then can be implemented. The state of this design is still in progress. Only start implementing this when everyone agrees upon these changes. = Issues = The current patch has several issues in how the original design was translated into code. To name a few: **Architectural Issues** * Current patch assumes that vertex painting will be merged into sculpt vertex painting. But in the original design it is mentioned that vertex painting will be merged into attribute painting. * Current patch takes steps into supporting `MVertCol` to render engines. It thereby takes precedence control over the rendering of vertex painting and thereby limits current functionality. * The workflow for the user for the current patch is not well communicated to the user. **Process Issues** * On top of the undecided implementation new features are still added on a weekly basis, making the code review process totally not efficient. People are spending a lot of time in reviewing and all needs to reviewed the week after, where new features are added, where it is unclear if the issues that were addressed in the previous review are handled correctly. **Implementation Issues** * Color management: user is painting in display space. * Vertex colors are actually averaged into a face color. But in the UI we say Vertex Colors. It is unclear why the colors needs to be averaged. IMO in sculpting you have dense meshes and you don't see the difference. But perhaps there are cases why this is needed. If so we should describe these use cases. * Cycles and EEVEE rendering in current patch are broken. * It is currently required to have a `MVertCol` and `MLoopCol` for rendering in sculpt-mode. I would expected that only one would be needed. As `MVertCol` has more precedence this makes it impossible to do any Vertex Painting. = Alternatives = Personally I miss the information why the Sculpt Vertex Painting cannot use the `MLoopCol`. If this is possible the workflow for the user is more clear and the implementation does not have so many issues/limitations. **Alternative 1: current sculpt patch** This should describe the current implementation as of end of 2019. As more and more features are added it is not easy to extract an design from the current implementation. The issues described above outlines the current workings/limitations. Perhaps Pablo can put in the ideas here. **Alternative 2: expectation how I expected the original design to be implemented** Current design is a goal implementation where attribute painting has been added. As we currently don't have a clear understanding how attribute painting will look like this we should not make any assumptions yet on the implementation. Therefore my expectation was a hard separation between `MVertCol` and `MLoopCol`. Both could be used during rendering and an operation to sync between the active `MVertCol` and `MLoopCol`. Current patch has parts of it, but as the current development is limiting functionality it is unclear if this alternative is actually what is the goal for = Roadmap = **Long term road map** * Get the basics correct with the next use cases in mind ** A sculptor wants to paint during sculpting. ** A modeller/lighter wants to use vertex painting as it can hold more precise data. * Design attribute painting * Implement attribute painting ** Will merge current vertex painting into attribute painting **What should be done in the current patch** Focus on the bare minimum and the stuff that has been agreed upon. Don't add fancy stuff on top of it, before we settle on the basics. As in the initial design is mentioned that sculpt vertex painting is an implementation for an edge case, so should not take control of the other implementations as it currently does. * It must be possible use sculpt vertex colors without vertex colors and vise versa. * Remove the face averaging. It feels like a hack where users has no control over and communicates the wrong information to users. Or we should come up with an actual user benefit. If we were able to use `MLoopCol` this isn't a problem, but as we are not storing the data here this limitation introduces some edge cases with incorrect feedback to users. The later could also be solved by user documentation, but as there isn't any design or user documentation at all... * As `MVertCol` is for artistic drawing only, I would not expect that it has precedence over vertex painting. The user should be able to control which layer is displayed (by the name). `MLoopCol` should have more precedence then `MVertCol` as it holds more data and use cases and is also the expected format for modellers/lighters. * Change the default name of sculpt vertex colors (Col) to something that does not clash with regular vertex colors. * Change the Attribute/Vertex Color Shader node to support both vertex colors (or any shading stuff) * Fix existing the crashes ** Activate vertex painting mode (No vcol layer available in vertpaint, but batches requested anyway!) * If you want to support alpha rendering in the workbench we should use the transparency_accum buffers. The current implementation a hack to get it working in the viewport. This should be done in a separate patch. * Usability: When starting sculpting and there is no `MVertCol` layer, a default needs to be added. * Usability: When Sculpt painting brush is active the context menu should have a color picker.