EngineeringPlan: Vertex Painting in Sculpt Mode #72866

New Issue

Jeroen Bakker · 2020-01-03T07:48:56+01:00

Jeroen Bakker commented

2020-01-03 07:48:56 +01:00

This task will describe the engineering parts of vertex painting in sculpt mode.
My first impression was that the current patch is bloating and IMO that was not necessary. After discussing this, my second thought was that the scope that I had in mind for this patch was different.
The goal of this patch is to come to a clear design/scope/phasing, so it is clear what the changes entails. This is useful for code review, bug fixing, bug triaging and future developments.

The state of this design is still in progress.

Issues

The current patch has several issues in how the original design was translated into code. To name a few:

Architectural Decisions

To be validated
The project makes a distinction between painting for display/rendering (vertex painting) and painting for modifiers and other parts (color attribute painting). I am not familiar with any designs for attribute painting so it could be that the patch is making assumptions that might be changed in the near future. The project wants to rename the current vertex painting to something else. This is might eventually be the case, but is this the right moment to do this as the future design is still unclear.

Technical Decisions

To be validated
** [RENDER]: Vertex Color Node will only allow the selection for Sculpt Vertex Groups. To use the attribute vertex colors use the attribute node. Migration code could convert old files using the Vertex Color Node. Other alternative would be to support both Vertex colors in the Vertex color node, but that introduces UX complexity.
** [DRAWING]: Should we support per vertex transparency in the workbench engine? The workbench engine is optimized for animation playback speed, adding actual transparency in more detail than materials would in the right way would need more preprocessing to determine if alpha values are used in the vertex buffer.

Process Issues

During the code review the patch received new features on a regular 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.

It is unclear what the scope of the patch is. In the original discussion it was mentioned that after this project a attribute painting project should start. This patch somehow already tried to take this future project into account. Currently we need to reverse engineer the patch and try to understand what might be the scope of the project. This seems like lacking documentation.

The current patch scope is perhaps too limited as it can crash when you want to do for example attribute vertex painting.

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. Reason for this is to have some visual similarity with masking. IMO we should not do this as it tells the users something different.
Currently in the patch it is not possible to do attribute vertex painting. On technical level it will just crash and the old vertex paint mode forces workbench in the vertex color mode for the specific object, what than doesn't draw the colors you want to see, but the sculpt vertex colors. It has been mentioned that there is a attribute overlay needed to fix this, but that depends again on the workflow of attribute painting, what is not designed.

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

The above gives an description of the current sculpt patch, after reverse engineering and discussions.

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

It is unclear what the scope is of Sculpt Vertex Painting. What makes it hard to describe a roadmap at this time. Below mentioned the initial idea I had when starting on reviewing the patch.

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 paintingWhat should be done in the current patch**

This part of the design isn't finished and will change when more and more gets clear in the current design.

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...
Change the default name of sculpt vertex colors (Col) to something that does not clash with regular vertex colors.
Add migration code for the Attribute/Vertex Color Shader node
Fix existing the crashes
** Activate vertex painting mode (No vcol layer available in vertpaint, but batches requested anyway!)
Vertex color drawing in the workbench should perhaps be mode specific. By default use Sculpt Vertex Colors, and when in attribute vertex color mode use the Vertex Colors
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. Note that doing this correctly (minor performance regressions) needs more work.
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.

If this is the case, than we should also find better names for Sculpt Vertex Color...

This task will describe the engineering parts of vertex painting in sculpt mode. My first impression was that the current patch is bloating and IMO that was not necessary. After discussing this, my second thought was that the scope that I had in mind for this patch was different. The goal of this patch is to come to a clear design/scope/phasing, so it is clear what the changes entails. This is useful for code review, bug fixing, bug triaging and future developments. The state of this design is still in progress. # Issues The current patch has several issues in how the original design was translated into code. To name a few: **Architectural Decisions** * To be validated **The project makes a distinction between painting for display/rendering (vertex painting) and painting for modifiers and other parts (color attribute painting). I am not familiar with any designs for attribute painting so it could be that the patch is making assumptions that might be changed in the near future.** The project wants to rename the current vertex painting to something else. This is might eventually be the case, but is this the right moment to do this as the future design is still unclear. **Technical Decisions** * To be validated ** [RENDER]: Vertex Color Node will only allow the selection for Sculpt Vertex Groups. To use the attribute vertex colors use the attribute node. Migration code could convert old files using the Vertex Color Node. Other alternative would be to support both Vertex colors in the Vertex color node, but that introduces UX complexity. ** [DRAWING]: Should we support per vertex transparency in the workbench engine? The workbench engine is optimized for animation playback speed, adding actual transparency in more detail than materials would in the right way would need more preprocessing to determine if alpha values are used in the vertex buffer. **Process Issues** During the code review the patch received new features on a regular 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. It is unclear what the scope of the patch is. In the original discussion it was mentioned that after this project a attribute painting project should start. This patch somehow already tried to take this future project into account. Currently we need to reverse engineer the patch and try to understand what might be the scope of the project. This seems like lacking documentation. The current patch scope is perhaps too limited as it can crash when you want to do for example attribute vertex painting. **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. Reason for this is to have some visual similarity with masking. IMO we should not do this as it tells the users something different. * Currently in the patch it is not possible to do attribute vertex painting. On technical level it will just crash and the old vertex paint mode forces workbench in the vertex color mode for the specific object, what than doesn't draw the colors you want to see, but the sculpt vertex colors. It has been mentioned that there is a attribute overlay needed to fix this, but that depends again on the workflow of attribute painting, what is not designed. # 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** The above gives an description of the current sculpt patch, after reverse engineering and discussions. **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 It is unclear what the scope is of Sculpt Vertex Painting. What makes it hard to describe a roadmap at this time. Below mentioned the initial idea I had when starting on reviewing the patch. **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** This part of the design isn't finished and will change when more and more gets clear in the current design. 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... * Change the default name of sculpt vertex colors (Col) to something that does not clash with regular vertex colors. * Add migration code for the Attribute/Vertex Color Shader node * Fix existing the crashes ** Activate vertex painting mode (No vcol layer available in vertpaint, but batches requested anyway!) * Vertex color drawing in the workbench should perhaps be mode specific. By default use Sculpt Vertex Colors, and when in attribute vertex color mode use the Vertex Colors * 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. Note that doing this correctly (minor performance regressions) needs more work. * 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. If this is the case, than we should also find better names for Sculpt Vertex Color...

Jeroen Bakker self-assigned this 2020-01-03 07:48:56 +01:00

Jeroen Bakker commented

2020-01-03 07:48:56 +01:00

Changed status from 'Needs Triage' to: 'Confirmed'

Jeroen Bakker commented

2020-01-03 07:48:56 +01:00

Added subscribers: @Jeroen-Bakker, @PabloDobarro

Jeroen Bakker changed title from ~~(WIP) EngineeringPlan: Vertex Painting in Sculpt Mode~~ to EngineeringPlan: Vertex Painting in Sculpt Mode

2020-01-03 09:40:17 +01:00

Pablo Dobarro commented

2020-01-03 23:37:57 +01:00

Some initial notes

Whatever design or naming convention we choose, the most important thing is that we need to make sure the users know they should never use MLoopCol (the current vertex paint mode) for painting after merging the patch. The new implementation with the new tools is superior in every single way when it comes to painting and the current vertex paint mode will be obsolete. There won't be any use cases for painting in the old vertex paint mode.

The only benefit the current vertex paint has (MLoopCol) is per face colors, which are not important for painting and that mode has a big number of issues and design limitations for supporting proper painting. They are only important when users need to store custom data into the mesh (as we saw in the Ubisoft talk in the Blender Conference). We still not have any tools for it (that is what the attribute paint mode should solve). Until final attribute paint is done, we will have a half-finished attribute paint mode by renaming the old vertex paint and MLoopCols, which should be fine because that is what people were using for that use case.

So, the situation is:

MLoopCol can handle more precise data, but we don't have any tools designed for it. Until we have a design and tools for this, supporting a more complex data structure won't have any benefits to the user.
MLoopCol is a waste of memory when painting because the extra data will not affect the final result. When using it in sculpt mode, where all the operations are based in vertex iterators, we are going to store the same colors 4 times per vertex. My idea is that we should be able to paint the resolution of a 4K texture in multires (that is 17M vertices + duplicates), so we should not start making compromises related to performance and memory.

For these reasons, I think that the new implementation based on MVertCol should take precedence over the old vertex paint mode. The old Vertex Paint, Vertex Colors and MLoopCols should be renamed to something that does not suggest painting to avoid user confusion (color data? custom color data?...) and leave that mode there for compatibility and to support the minimum requirements of attribute painting until we have the full mode.

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.

That is still the idea but I'm not changing the name of the modes/attributes in that patch to avoid making it bigger. In the release after merging the patch, the idea would be to have the modes/data renamed:

Sculpt Vertex colors in the patch -> Default Vertex Colors and Vertex Painting tools
Vertex colors in the patch -> Attribute Paint/Custom color data/Color attribute/other name that does not suggest 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.

Yes, that is the idea. After merging the patch MVertCol should be the main datalayer to deal with mesh colors inside Blender, so it should have direct connection to the renderers. The current MLoopCol would be another mesh attribute that can store colors/ID/vectors/masks... so it does not make sense to render that by default. That is why I made the vertex colors node to output MVertCol, but the attribute node can output both (this way we can keep compatibility with old files without converting the datalayer)

The workflow for the user for the current patch is not well communicated to the user.

Agree, but I think we can solve this by changing the name of the current vertex painting tools to something that does not tell the users that it is a "painting" mode.

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 aaged.

I made it like that because the sculpt mask is also rendered that way and it is another vertex attribute. I don't think that there are cases where this is going to be a problem. In this case, we need to choose whatever is faster.

Cycles and EEVEE rendering in current patch are broken.

I did not test cycles, but EEVEE works for me.

It is currently required to have a MVertCol and MLoopCol for rendering in sculpt-mode.

It should not, last time I checked everything was working fine when using only MVertCol layers.

Color management: user is painting in display space.

This is why I'm adding the full paint brush to the patch. We need to test that the main brush actually works for painting and the color math of all its properties is correct. If we don't get the behavior of the brush exactly right from the beggining, it does not matter how many time we spend working on this because nobody will want to use this mode.

After changing the way MVertCols render, my idea was to render MLoopCols using the overlay in the "attribute paint" mode, but that was removed in a recent commit. Can we bring that overlay back?
When attribute paint is fully implemented, my idea would be to make that overlay customizable to display any attribute data (render the data as colors, as a gradient, as an ID map...)

Some initial notes Whatever design or naming convention we choose, the most important thing is that we need to make sure the users know they should *never* use MLoopCol (the current vertex paint mode) for painting after merging the patch. The new implementation with the new tools is superior in every single way when it comes to painting and the current vertex paint mode will be obsolete. There won't be any use cases for painting in the old vertex paint mode. The only benefit the current vertex paint has (MLoopCol) is per face colors, which are not important for painting and that mode has a big number of issues and design limitations for supporting proper painting. They are only important when users need to store custom data into the mesh (as we saw in the Ubisoft talk in the Blender Conference). We still not have any tools for it (that is what the attribute paint mode should solve). Until final attribute paint is done, we will have a half-finished attribute paint mode by renaming the old vertex paint and MLoopCols, which should be fine because that is what people were using for that use case. So, the situation is: - MLoopCol can handle more precise data, but we don't have any tools designed for it. Until we have a design and tools for this, supporting a more complex data structure won't have any benefits to the user. - MLoopCol is a waste of memory when painting because the extra data will not affect the final result. When using it in sculpt mode, where all the operations are based in vertex iterators, we are going to store the same colors 4 times per vertex. My idea is that we should be able to paint the resolution of a 4K texture in multires (that is 17M vertices + duplicates), so we should not start making compromises related to performance and memory. For these reasons, I think that the new implementation based on MVertCol should take precedence over the old vertex paint mode. The old Vertex Paint, Vertex Colors and MLoopCols should be renamed to something that does not suggest painting to avoid user confusion (color data? custom color data?...) and leave that mode there for compatibility and to support the minimum requirements of attribute painting until we have the full mode. > > 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. That is still the idea but I'm not changing the name of the modes/attributes in that patch to avoid making it bigger. In the release after merging the patch, the idea would be to have the modes/data renamed: - Sculpt Vertex colors in the patch -> Default Vertex Colors and Vertex Painting tools - Vertex colors in the patch -> Attribute Paint/Custom color data/Color attribute/other name that does not suggest 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. Yes, that is the idea. After merging the patch MVertCol should be the main datalayer to deal with mesh colors inside Blender, so it should have direct connection to the renderers. The current MLoopCol would be another mesh attribute that can store colors/ID/vectors/masks... so it does not make sense to render that by default. That is why I made the vertex colors node to output MVertCol, but the attribute node can output both (this way we can keep compatibility with old files without converting the datalayer) > > The workflow for the user for the current patch is not well communicated to the user. Agree, but I think we can solve this by changing the name of the current vertex painting tools to something that does not tell the users that it is a "painting" mode. > 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 aaged. I made it like that because the sculpt mask is also rendered that way and it is another vertex attribute. I don't think that there are cases where this is going to be a problem. In this case, we need to choose whatever is faster. > Cycles and EEVEE rendering in current patch are broken. I did not test cycles, but EEVEE works for me. > It is currently required to have a MVertCol and MLoopCol for rendering in sculpt-mode. It should not, last time I checked everything was working fine when using only MVertCol layers. > Color management: user is painting in display space. This is why I'm adding the full paint brush to the patch. We need to test that the main brush actually works for painting and the color math of all its properties is correct. If we don't get the behavior of the brush exactly right from the beggining, it does not matter how many time we spend working on this because nobody will want to use this mode. After changing the way MVertCols render, my idea was to render MLoopCols using the overlay in the "attribute paint" mode, but that was removed in a recent commit. Can we bring that overlay back? When attribute paint is fully implemented, my idea would be to make that overlay customizable to display any attribute data (render the data as colors, as a gradient, as an ID map...)

Jeroen Bakker commented

2020-01-04 00:26:58 +01:00

Totally not sure about what you say as there are many usecases that just cannot be done with MVertCol implementation. Just look at requirements from gaming modellers where you can have a fully red and a fully blue face against each other. This can be done with the current vertex painting.

Environment artist that use vertex colors for material blending. Low poly artists. Just to name a few of usecases that will be limited if this patch will be added in the current state.

Your patch breaks rendering, current files and can only be accepted when they will not break. Or when it has been discussed to be a acceptable consequence. Many file formats also uses MLoopCols to store colors.

I never heard of a decision to remove vertex paint , before adding a replacement system. If I read it correctly you want to remove or at least limit users to use them with this patch.

Looking at the original design it is mentioned totally differently so I would like to see when and who made that decision.

Ps don’t see this as criticism. I just think that in the end most high dense modellers would use a sculpt based vertex painting. I just think that this patch tries to do things in the wrong timing.

Totally not sure about what you say as there are many usecases that just cannot be done with MVertCol implementation. Just look at requirements from gaming modellers where you can have a fully red and a fully blue face against each other. This can be done with the current vertex painting. Environment artist that use vertex colors for material blending. Low poly artists. Just to name a few of usecases that will be limited if this patch will be added in the current state. Your patch breaks rendering, current files and can only be accepted when they will not break. Or when it has been discussed to be a acceptable consequence. Many file formats also uses MLoopCols to store colors. I never heard of a decision to remove vertex paint , before adding a replacement system. If I read it correctly you want to remove or at least limit users to use them with this patch. Looking at the original design it is mentioned totally differently so I would like to see when and who made that decision. Ps don’t see this as criticism. I just think that in the end most high dense modellers would use a sculpt based vertex painting. I just think that this patch tries to do things in the wrong timing.

Pablo Dobarro commented

2020-01-04 01:28:33 +01:00

Those use cases are related to using MLoopCol as data (material masks, for example). All those "painting" options are still going to be supported as the current vertex paint mode is only going to be renamed. Also, using brushes for painting low poly models or material mask is not the ideal solution. For that tasks it would be better to have proper precision tools (similar to edit mode selections) than a brush. With this change we should not loose any functionality, we are just redirecting users to a more advanced painting mode if they want to paint.

When I'm talking about painting I'm referring to the functionality of dedicated painting/texturing/illustration software, not to the fact that a mode uses a brush to modify the data. Modes can still have a brush tool for some task, but that does not mean they are painting modes. Something like this https://twitter.com/pablodp606/status/1213250893199613952 is just impossible to do in the current vertex paint mode, but it is doable with the new sculpt vertex colors and tools because it was designed for that. When users try to use vertex paint (or texture paint) for actual painting, they often get frustrated because the current modes lack a lot of feature and have so many design limitation related to painting. We just need to add a way to differentiate the two things:

Painting modes features: Wet paint, dynamic brush tip shape, advanced color mix modes, noise generators, particle brushes, image/color filters... this is what the sculpt vertex colors patch tries to introduce and what should be added to texture painting in the future.
Modes that "can use a brush" features: Mesh element selections, custom overlays, select and assign operators, value gradients based on topology, smoothing element selections... This is what modes like weight paint, the future attribute paint or the renamed MLoopCol vertex colors should have. And, on top of that, they can have a basic brush. This is were the use cases you mentioned should be supported, but it does not make sense to add wet paint or particle brushes to paint a low poly model, a material mask or the weights of a rig.

I think the only change required in previous files with the patch is to change the vertex colors node for the attribute node with the layer name (is this possible to do in versioning?). After that, I think it should render exactly the same.

Those use cases are related to using MLoopCol as data (material masks, for example). All those "painting" options are still going to be supported as the current vertex paint mode is only going to be renamed. Also, using brushes for painting low poly models or material mask is not the ideal solution. For that tasks it would be better to have proper precision tools (similar to edit mode selections) than a brush. With this change we should not loose any functionality, we are just redirecting users to a more advanced painting mode if they want to paint. When I'm talking about painting I'm referring to the functionality of dedicated painting/texturing/illustration software, not to the fact that a mode uses a brush to modify the data. Modes can still have a brush tool for some task, but that does not mean they are painting modes. Something like this https://twitter.com/pablodp606/status/1213250893199613952 is just impossible to do in the current vertex paint mode, but it is doable with the new sculpt vertex colors and tools because it was designed for that. When users try to use vertex paint (or texture paint) for actual painting, they often get frustrated because the current modes lack a lot of feature and have so many design limitation related to painting. We just need to add a way to differentiate the two things: - Painting modes features: Wet paint, dynamic brush tip shape, advanced color mix modes, noise generators, particle brushes, image/color filters... this is what the sculpt vertex colors patch tries to introduce and what should be added to texture painting in the future. - Modes that "can use a brush" features: Mesh element selections, custom overlays, select and assign operators, value gradients based on topology, smoothing element selections... This is what modes like weight paint, the future attribute paint or the renamed MLoopCol vertex colors should have. And, on top of that, they can have a basic brush. This is were the use cases you mentioned should be supported, but it does not make sense to add wet paint or particle brushes to paint a low poly model, a material mask or the weights of a rig. I think the only change required in previous files with the patch is to change the vertex colors node for the attribute node with the layer name (is this possible to do in versioning?). After that, I think it should render exactly the same.

Jeroen Bakker commented

2020-01-04 10:11:29 +01:00

The ideas you described are nowhere mentioned and discussed. When misinterpreted/understood this leads to a huge amount of regression failures (by design or understanding) around Blender. It also introduces a different workflow then described in the original design task. We just need to be clear that what will be delivered is well designed, discussed agreed upon and won't leave unsolvable regressions. This is my biggest concern as this project lacks written down vision/design.

Have you tested the current implementation already with modifiers, particles/hair, physics, python api etc, here the vertex colors are related to actual colors, what do users and developers expect here. Also professional used file formats needs to be addressed. This is what I mean that the patch might get bloated. My proposal is to take smaller steps and start with a design and feasible short term goals as 1) attribute painting is still unclear, 2) there are workflows that we have missed and might introduce regressions.

What do you think why Sculpt mode vertex painting is mentioned to be an edge case in the original design? The implementation seems like to imply something differently. hence my proposal was to limit the implementation to be more in line with an edge case. Can you start with the UI/UX design?

The ideas you described are nowhere mentioned and discussed. When misinterpreted/understood this leads to a huge amount of regression failures (by design or understanding) around Blender. It also introduces a different workflow then described in the original design task. We just need to be clear that what will be delivered is well designed, discussed agreed upon and won't leave unsolvable regressions. This is my biggest concern as this project lacks written down vision/design. Have you tested the current implementation already with modifiers, particles/hair, physics, python api etc, here the vertex colors are related to actual colors, what do users and developers expect here. Also professional used file formats needs to be addressed. This is what I mean that the patch might get bloated. My proposal is to take smaller steps and start with a design and feasible short term goals as 1) attribute painting is still unclear, 2) there are workflows that we have missed and might introduce regressions. What do you think why Sculpt mode vertex painting is mentioned to be an edge case in the original design? The implementation seems like to imply something differently. hence my proposal was to limit the implementation to be more in line with an edge case. Can you start with the UI/UX design?

Frozen_Death_Knight commented

2020-01-04 17:09:29 +01:00

Added subscriber: @Frozen_Death_Knight

Frozen_Death_Knight commented

2020-01-04 17:09:29 +01:00

@PabloDobarro @Jeroen-Bakker I just wanted to come in quickly and give my 2 cents. After having followed the development and dicussion surrounding this new painting workflow for some time, I believe it is about time that the patch got its own experimental Blender branch just like the old sculpting branch. I feel like this feature has ballooned up to the point that it needs more testers before being allowed on any Alpha master branch. The regular branches don't seem suited to deal with design issues like these and the sculpt branch ended up being a great success considering the amount of features that ended up in the official releases.

I think it would be a lot easier to figure out a solid plan that all parties would agree on if more 3D artists got to test and leave more feedback on the new features.

@PabloDobarro @Jeroen-Bakker I just wanted to come in quickly and give my 2 cents. After having followed the development and dicussion surrounding this new painting workflow for some time, I believe it is about time that the patch got its own experimental Blender branch just like the old sculpting branch. I feel like this feature has ballooned up to the point that it needs more testers before being allowed on any Alpha master branch. The regular branches don't seem suited to deal with design issues like these and the sculpt branch ended up being a great success considering the amount of features that ended up in the official releases. I think it would be a lot easier to figure out a solid plan that all parties would agree on if more 3D artists got to test and leave more feedback on the new features.

Paul commented

2020-01-04 17:40:21 +01:00

Added subscriber: @riceart

Paul commented

2020-01-04 17:40:21 +01:00

If possible, please bring back Size and Strenght Widget from 2.80 for all modes Sculpt/Paint/Vertex Paint. It can be somwhere as optional behavior i dont mind if its not default anymore. The new one is unusable on screen tablets.

And, second please bring back cursor (round circle) behavior from 2.80. RIght now when im making stroke it disapears. Its happening in sculpt mode so please do not add this to paint mode.

best regards, Paul

If possible, please bring back Size and Strenght Widget from 2.80 for all modes Sculpt/Paint/Vertex Paint. It can be somwhere as optional behavior i dont mind if its not default anymore. The new one is unusable on screen tablets. And, second please bring back cursor (round circle) behavior from 2.80. RIght now when im making stroke it disapears. Its happening in sculpt mode so please do not add this to paint mode. best regards, Paul

Konstantin commented

2020-01-04 20:44:12 +01:00

Added subscriber: @Ko

Konstantin commented

2020-01-04 20:44:12 +01:00

What about weight painting? Its integration will be more straightforward than with colors as weights assigned per vertex and cant be rendered.

Pablo Dobarro commented

2020-01-04 22:53:57 +01:00

@Jeroen-Bakker The ideas were discussed the first time I went to the studio with Brecht and Sergey and they mentioned in #67520. That task is about splitting painting functionality into a "artistic painting" mode and a "technical painting mode", which is what I'm trying to do in D5975. This is probably a misunderstanding because of the weird names that are used in the UI and the patch description, but the intention is not to include any compatibility breaking change and it is not trying to implement anything that wasn't already discussed. I think that if we keep the "vertex color" node to keep outputing MLoopCol, everything should work the same as before. The whole "sculpt vertex colors" workflow was introduced in the sculpt branch, where it is more complete (the UI is more polished and most features are complete and well integrated), so it already was tested for a while by a lot of users. We can use the sculpt branch to discuss UI/UX design while testing it on something that "works" instead of discussing it on paper.

I can split the patches into smaller steps if that makes the review process easier. The problem with that is that we can easily break things or make design mistakes during the merge process because we never tested if the thing works as a whole before starting adding code to master. I already have most the code ready for all the initial features of this new paint mode to support a properly designed painting workflow, but they are split into multiple patches with a lot of shared code and dependencies between them. I personally would prefer adding everything to a single patch to make sure we have a fully functional feature from the first commit. It may also help to make the process less confusing as we would avoid intermediate stages with weird unfinished features and mode naming that does not make sense for what it is trying to do.

@Jeroen-Bakker The ideas were discussed the first time I went to the studio with Brecht and Sergey and they mentioned in #67520. That task is about splitting painting functionality into a "artistic painting" mode and a "technical painting mode", which is what I'm trying to do in [D5975](https://archive.blender.org/developer/D5975). This is probably a misunderstanding because of the weird names that are used in the UI and the patch description, but the intention is not to include any compatibility breaking change and it is not trying to implement anything that wasn't already discussed. I think that if we keep the "vertex color" node to keep outputing MLoopCol, everything should work the same as before. The whole "sculpt vertex colors" workflow was introduced in the sculpt branch, where it is more complete (the UI is more polished and most features are complete and well integrated), so it already was tested for a while by a lot of users. We can use the sculpt branch to discuss UI/UX design while testing it on something that "works" instead of discussing it on paper. I can split the patches into smaller steps if that makes the review process easier. The problem with that is that we can easily break things or make design mistakes during the merge process because we never tested if the thing works as a whole before starting adding code to master. I already have most the code ready for all the initial features of this new paint mode to support a properly designed painting workflow, but they are split into multiple patches with a lot of shared code and dependencies between them. I personally would prefer adding everything to a single patch to make sure we have a fully functional feature from the first commit. It may also help to make the process less confusing as we would avoid intermediate stages with weird unfinished features and mode naming that does not make sense for what it is trying to do.

Jeroen Bakker commented

2020-01-05 13:56:15 +01:00

It is not about the patch, this is about the lack of documentation and transparency of the project. I want to come to an understanding what will be part of the first release. Currently during review the patch too small as it didn't address the scope of the first release. Also there is nowhere described what the scope is and if multiple releases are needed.

The only written information I have is a limited patch, a list of one liners that could be well thought of, or not and a very global idea of the project. During testing I find limitations and the patch is increased. This tells me that the way how this project is handled and and we should fix that.

I also miss the decisions and alternatives that have been evaluated. As you see all documentation.
Doing reverse engineering of a patch to come to an understanding of the decisions that have been made is bad, as this means that after release trianging, bug fixing, feature development will be lacking.

It is not about the patch, this is about the lack of documentation and transparency of the project. I want to come to an understanding what will be part of the first release. Currently during review the patch too small as it didn't address the scope of the first release. Also there is nowhere described what the scope is and if multiple releases are needed. The only written information I have is a limited patch, a list of one liners that could be well thought of, or not and a very global idea of the project. During testing I find limitations and the patch is increased. This tells me that the way how this project is handled and and we should fix that. I also miss the decisions and alternatives that have been evaluated. As you see all documentation. Doing reverse engineering of a patch to come to an understanding of the decisions that have been made is bad, as this means that after release trianging, bug fixing, feature development will be lacking.

Jeroen Bakker commented

2020-01-06 09:07:29 +01:00

@PabloDobarro I updated the description with your feedback. I think that the project documentation is lacking and that that needs to be addressed. Current lack of documentation leads to misunderstanding the scope of the patch vs the scope of this project vs the scope of several projects.

A patch should ideally be a minimum change, but should always be complete on user level. If this project needs more patches, these patches should be planned/described so during review we know what the scope of a current in review patch is, and that some parts will be addressed in a next patch. Currently I am spending hours per week just to understand the scoping, this is not the way how projects should be executed. Any professional developer knows that. I am only asking you as you are executing this project to come up with this documentation. I can help out (in guidance), but the actual writing should be done by the project.

Check how other projects do this. For example UDIM.

@PabloDobarro I updated the description with your feedback. I think that the project documentation is lacking and that that needs to be addressed. Current lack of documentation leads to misunderstanding the scope of the patch vs the scope of this project vs the scope of several projects. A patch should ideally be a minimum change, but should always be complete on user level. If this project needs more patches, these patches should be planned/described so during review we know what the scope of a current in review patch is, and that some parts will be addressed in a next patch. Currently I am spending hours per week just to understand the scoping, this is not the way how projects should be executed. Any professional developer knows that. I am only asking you as you are executing this project to come up with this documentation. I can help out (in guidance), but the actual writing should be done by the project. Check how other projects do this. For example UDIM.

WMROssi commented

2020-04-17 18:02:53 +02:00

Added subscriber: @Wesley-Rossi

Vyacheslav Kobozev commented

2020-05-08 08:56:51 +02:00

Added subscriber: @Vyach

blender-admin commented

2020-06-22 20:05:28 +02:00

This issue was referenced by f7bbc7cdbb

This issue was referenced by f7bbc7cdbb6cb0d28504c6a8dd51bee5330d1f17

Jeroen Bakker removed their assignment 2020-10-20 14:03:44 +02:00

Philipp Oeser commented

2021-03-10 13:57:31 +01:00

Added subscriber: @lichtwerk

Philipp Oeser commented

2021-03-10 13:57:31 +01:00

Just want to add here that since we can bake to vertex colors, it can now happen that if you have the experimental feature turned OFF but still have a sculpt vertex color layer on your mesh, baking will take place to that sculpt vertex color layer with no way to look at it, see #86455 (Baking to Vertex Colors bakes without result to look at (if sculpt vertex color layer is present but experimental option is disabled))

Brecht Van Lommel commented

2022-04-06 18:53:31 +02:00

Added subscriber: @brecht

Brecht Van Lommel commented

2022-04-06 18:53:31 +02:00

Changed status from 'Confirmed' to: 'Archived'

Brecht Van Lommel closed this issue

2022-04-06 18:53:31 +02:00

Brecht Van Lommel commented

2022-04-06 18:53:31 +02:00

This task is outdated, specific tasks regarding vertex colors are now in #97101 (Color attribute improvements).

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

EngineeringPlan: Vertex Painting in Sculpt Mode #72866

Issues

Alternatives

Roadmap