Sculpt Mode: Add exisiting Tools as Menu Operators #102427

Open
opened 2022-11-11 11:57:01 +01:00 by Julien Kaspar · 23 comments
Member

(NOTE) This task is part of the community & sub tasks of the workboard. Anyone is free to pick up this task and contribute. For any questions or needed reviewers, please tag @Sergey @HooglyBoogly and @JulienKaspar.

Previous limitations on using the Adjust Last Operation panel are being lifted, and fixing #99607 is being prioritized.
This will make it possible to add many active tools in the toolbar to the header menus as operators.
This has the following advantages:

  • Support the existing "Operate → Settings" paradigm
  • Many operators could be combined for simplicity
  • Easier to use various tools as one time operations instead of making them active
  • Easier to use filters via shortcuts and quick favorites.
  • Redo panel interactions for very high res meshes (Better with slow performance)
  • Simpler than implementing #97959. That can come after then.

Needed Operators

This is based on operators that currently only exist as active tools, or are not properly exposed at all yet.
Many operators are depending on the cursor position. This feature should be used heavily: da2ba40268

Initially or later on some operators can be combined into a single operator and accordingly renamed.
The first step is to add the existing operations as they are to the menus.
There are also still some open questions for some operators but these can be resolved during testing and review.

Sculpt

Transform Tools
  • Move
  • Rotate
  • Scale
  • Sphere (From Mesh Filter)
Trim Tools
  • Box Trim (Trim Mode ="Difference")
  • Lasso Trim (Trim Mode ="Difference")
  • Box Add (Trim Mode ="Join")
  • Lasso Add (Trim Mode ="Join")
Project Tools
  • Line Project
Fairing
  • Fair Positions
  • Fair Tangency
Mesh Filter Tool
  • Smooth
  • Surface Smooth
  • Scale
  • Inflate
  • Random
  • Relax Topology
  • Relax Face Set Boundaries
  • Sharpen
  • Enhance Details
  • Erase Displacement (Grayed out if if no Multires)
Remove
  • Delete Face Set

Mask

  • Mask by Color

Face Sets

  • Box Face Set
  • Lasso Face Set

Paint

Color Filter Tool
  • Fill Color (Part of #99769)
  • Blur (Smooth color filter type)
  • Hue
  • Saturation
  • Value
  • Brightness
  • Contrast
  • Red
  • Green
  • Blue
Other
  • Expand Color by Topology (sculpt.expand with target data set to Color)

Layout

This is how the operators could be arranged.

image.png

image.png image.png image.png image.png
(NOTE) This task is part of the community & sub tasks of the workboard. Anyone is free to pick up this task and contribute. For any questions or needed reviewers, please tag @Sergey @HooglyBoogly and @JulienKaspar. Previous limitations on using the Adjust Last Operation panel are being lifted, and fixing #99607 is being prioritized. This will make it possible to add many active tools in the toolbar to the header menus as operators. This has the following advantages: - Support the existing "Operate → Settings" paradigm - Many operators could be combined for simplicity - Easier to use various tools as one time operations instead of making them active - Easier to use filters via shortcuts and quick favorites. - Redo panel interactions for very high res meshes (Better with slow performance) - Simpler than implementing #97959. That can come after then. ## Needed Operators This is based on operators that currently only exist as active tools, or are not properly exposed at all yet. Many operators are depending on the cursor position. This feature should be used heavily: da2ba40268 Initially or later on some operators can be combined into a single operator and accordingly renamed. The first step is to add the existing operations as they are to the menus. There are also still some open questions for some operators but these can be resolved during testing and review. ### Sculpt ##### Transform Tools - [x] Move - [x] Rotate - [x] Scale - [x] Sphere (From Mesh Filter) ##### Trim Tools - [x] Box Trim *(Trim Mode ="Difference")* - [x] Lasso Trim *(Trim Mode ="Difference")* - [x] Box Add (*Trim Mode ="Join")* - [x] Lasso Add *(Trim Mode ="Join")* ##### Project Tools - [x] Line Project ##### Fairing - [x] Fair Positions - [x] Fair Tangency ##### Mesh Filter Tool - [x] Smooth - [x] Surface Smooth - ~~Scale~~ - [x] Inflate - [x] Random - [x] Relax Topology - [x] Relax Face Set Boundaries - [x] Sharpen - [x] Enhance Details - [x] Erase Displacement *(Grayed out if if no Multires)* ##### Remove - [ ] Delete Face Set ### Mask - [ ] Mask by Color ### Face Sets - [ ] Box Face Set - [ ] Lasso Face Set ### Paint ##### Color Filter Tool - ~~Fill Color~~ (Part of #99769) - [ ] Blur (Smooth color filter type) - [ ] Hue - [ ] Saturation - [ ] Value - [ ] Brightness - [ ] Contrast - [ ] Red - [ ] Green - [ ] Blue ##### Other - [ ] Expand Color by Topology (`sculpt.expand` with target data set to `Color`) ## Layout This is how the operators could be arranged. ![image.png](https://archive.blender.org/developer/F13891538/image.png) | ![image.png](https://archive.blender.org/developer/F13891549/image.png) | ![image.png](https://archive.blender.org/developer/F13891552/image.png) | ![image.png](https://archive.blender.org/developer/F13891555/image.png) | ![image.png](https://archive.blender.org/developer/F13891557/image.png) | -- | -- | -- | -- |
Author
Member

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

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

Added subscribers: @JulienKaspar, @JosephEagar

Added subscribers: @JulienKaspar, @JosephEagar
Author
Member

From recent module meeting:

A vital problem to solve here, is to add a general operator property to use an eyedropper when executed from a menu, but use the cursor position when executed via a shortcut.
This needs a general solution and discussion with the UI module.

EDIT: This seems to be already implemented with D12255.

From recent module meeting: > A vital problem to solve here, is to add a general operator property to use an eyedropper when executed from a menu, but use the cursor position when executed via a shortcut. > This needs a general solution and discussion with the UI module. EDIT: This seems to be already implemented with [D12255](https://archive.blender.org/developer/D12255).
Julien Kaspar added this to the Sculpt, Paint & Texture project 2023-02-08 10:48:52 +01:00
Contributor

Hello, I'd like to work on this please.
I just want to make sure that I understand the requirements of this issue correctly. The main requirement is to add the long list of operators mentioned in the issue description to the sculpting workspace UI, correct?

For example, this is the current state of the Sculpt menu on my end.
image

And the issue requires that its updated to be as so:

And the same applies to the other menus: mask, face sets, and paint. Correct?

Also, I'd like to ask for any hints on where to start on the code/documentation side. I'll be searching on my own, but any hints are greatly appreciated.

Hello, I'd like to work on this please. I just want to make sure that I understand the requirements of this issue correctly. The main requirement is to add the long list of operators mentioned in the issue description to the sculpting workspace UI, correct? For example, this is the current state of the `Sculpt` menu on my end. ![image](/attachments/19bb2b19-cf4d-4703-8c69-41c67fac02ae) And the issue requires that its updated to be as so: <image src="https://archive.blender.org/developer/F13891549/image.png" height=400></image> And the same applies to the other menus: `mask`, `face sets`, and `paint`. Correct? Also, I'd like to ask for any hints on where to start on the code/documentation side. I'll be searching on my own, but any hints are greatly appreciated.
Author
Member

@Tarek-Yasser Thanks! Feel free to assign the issue to yourself once you're working on it.
You also understood the description right.
Some of these operators are very straight forward to add and barely need any adjustments in the code. Probably just a OPTYPE_DEPENDS_ON_CURSOR flag.
Others are more complicated (like the Filters) because of their specific implementation.

The patch doesn't need to include all of the listed operators at once. First working on the low hanging fruits is also fine.

General documentation and help can be found here: https://developer.blender.org/
@JosephEagar can give some technical pointers for this patch specifically.

@Tarek-Yasser Thanks! Feel free to assign the issue to yourself once you're working on it. You also understood the description right. Some of these operators are very straight forward to add and barely need any adjustments in the code. Probably just a `OPTYPE_DEPENDS_ON_CURSOR` flag. Others are more complicated (like the Filters) because of their specific implementation. The patch doesn't need to include all of the listed operators at once. First working on the low hanging fruits is also fine. General documentation and help can be found here: https://developer.blender.org/ @JosephEagar can give some technical pointers for this patch specifically.
Contributor

Thank you for the quick reply. I'll be sure to consult the documentation in case any operator proves to be challenging. I'll also try to submit patches incrementally so things are easier to review.

Quick question: Am I supposed to assign myself or not? The old website (I think) allowed this, but this one isn't allowing me to assign myself.

Thank you for the quick reply. I'll be sure to consult the documentation in case any operator proves to be challenging. I'll also try to submit patches incrementally so things are easier to review. Quick question: Am I supposed to assign myself or not? The old website (I think) allowed this, but this one isn't allowing me to assign myself.
Author
Member

@Tarek-Yasser You should be able to do assign yourself.
If there are still any issues with the site you can get support at #blender-developers-migration-support on chat.blender ;)

@Tarek-Yasser You should be able to do assign yourself. If there are still any issues with the site you can get support at #blender-developers-migration-support on chat.blender ;)
Contributor

Quick update regarding assigning myself: After asking at #blender-developers-migration-support. Brecht informed me that only members of the Blender organization can be assigned to issues (message can be found here).

So I guess I'm "unofficialy assigned" for the time being.

Regarding my progress so far. I've been scouring the blender docs and grepping whatever I'm seeing in the UI for any hint where the code for the menus might be to no avail (yet). The documentation for this module seems a bit scarce so I'll try digging deeper.

EDIT: I think I found the UI code. It seems to be at release/scripts/startup/bl_ui/space_view3d.py

Quick update regarding assigning myself: After asking at `#blender-developers-migration-support`. Brecht informed me that only members of the Blender organization can be assigned to issues [(message can be found here)](https://blender.chat/channel/blender-developers-migration-support?msg=nwF4S844SphMn4K2f). So I guess I'm "unofficialy assigned" for the time being. Regarding my progress so far. I've been scouring the blender docs and grepping whatever I'm seeing in the UI for any hint where the code for the menus might be to no avail (yet). The documentation for this module seems a bit scarce so I'll try digging deeper. EDIT: I think I found the UI code. It seems to be at `release/scripts/startup/bl_ui/space_view3d.py`

Taking advantage of this revisions you can include lasso hide, is an useful tool

Taking advantage of this revisions you can include lasso hide, is an useful tool
Author
Member

@Dethknight I didn't include any non existing operators in this task.
But missing gesture operators are very welcome too, as part of #80390.

@Dethknight I didn't include any non existing operators in this task. But missing gesture operators are very welcome too, as part of [#80390](https://projects.blender.org/blender/blender/issues/80390).
Philipp Oeser removed the
Interest
Sculpt, Paint & Texture
label 2023-02-10 09:10:58 +01:00
Contributor

A few updates here:

  1. For Move, Rotate, and Scale, I grepped through the code and inspected the UI for a while and found this section:
    efabe81c91/release/scripts/startup/bl_ui/space_view3d.py (L1034-L1036)

    It seems to be transforming the whole object without doing anything special, is this the intended behaviour?

  2. Toggle Visibility, Fair Positions, and Fair Tangency troubles.

    Old long question

    Regarding Toggle Visibility, Fair Positions, and Fair Tangency. I tried copying layout.operator(...) like the previous point but it didn't work. Each time the sculpt menu is opened, I get the following error:

    rna_uiItemO: operator missing srna '<operator>'
    <path_to_builds>/build_linux_lite/bin/3.5/scripts/startup/bl_ui/space_view3d.py:3218
    

    Here are the sections where I think the operators are defined:

    I tried the following values operator:
    - sculpt.face_sets_change_visibility
    - sculpt.face_sets_edit (I think this has a property mode to use with FAIR_POSITIONS and FAIR_TANGENCY?)

    From what I could scrape together (this part seems to not have much documentation): SCULPT_OT_face_sets_change_visibility defines an operator (OT?), which is called face_sets_change_visibility and falls sculpt. So it should be called like so:

    layout.operator("<operator>")
    

    Any guidance/documentation would be much appreciated.

    Update: After watching this part. I think these functions are not yet exposed in the Python API?. @JulienKaspar is this what you meant when you said that "some operators are straightforward and others are more complicated"?

    So, trying out:

    • sculpt.face_set_change_visibility
    • sculpt.face_set_edit
      With "set" instead of "sets" seems to work? I'm so confused...
  3. I can't seem to find "Box/Lasso Add". I could only find "Box/Lasso Trim" and "Line Project" in space_toolsystem_toolbar.py. Do they have another name?

Some notes for new contributors or blender newbies:

  1. If builds take too long for you (building on an HDD or slow computer) try make lite. I'm not sure if this would be suitable for working on every part of the code, but it works for me.
  2. New developer info/Overview in the docs can be quite helpful. Especially for Navigating the code. I accidently skipped the code layout diagram and it could've saved me a bit of time.
  3. This video did a pretty good job of explaining some stuff I was wondering about, and glued things together pretty well (in my mind at least). Please give it a watch if you can.
A few updates here: 1. For Move, Rotate, and Scale, I grepped through the code and inspected the UI for a while and found this section: https://projects.blender.org/blender/blender/src/commit/efabe81c916cfa4268326ff5595f27a7d7df714f/release/scripts/startup/bl_ui/space_view3d.py#L1034-L1036 It seems to be transforming the whole object without doing anything special, is this the intended behaviour? 2. `Toggle Visibility`, `Fair Positions`, and `Fair Tangency` troubles. <details> <summary> Old long question </summary> Regarding `Toggle Visibility`, `Fair Positions`, and `Fair Tangency`. I tried copying `layout.operator(...)` like the previous point but it didn't work. Each time the sculpt menu is opened, I get the following error: ``` rna_uiItemO: operator missing srna '<operator>' <path_to_builds>/build_linux_lite/bin/3.5/scripts/startup/bl_ui/space_view3d.py:3218 ``` Here are the sections where I think the operators are defined: - Toggle Visibility: - https://projects.blender.org/blender/blender/src/commit/efabe81c916cfa4268326ff5595f27a7d7df714f/source/blender/editors/sculpt_paint/sculpt_face_set.cc#L787-L794 - https://projects.blender.org/blender/blender/src/commit/efabe81c916cfa4268326ff5595f27a7d7df714f/source/blender/editors/sculpt_paint/sculpt_face_set.cc#L972-L992 - Fair Positions/Tangency: - https://projects.blender.org/blender/blender/src/commit/efabe81c916cfa4268326ff5595f27a7d7df714f/source/blender/editors/sculpt_paint/sculpt_face_set.cc#L1058-L1097 - https://projects.blender.org/blender/blender/src/commit/efabe81c916cfa4268326ff5595f27a7d7df714f/source/blender/editors/sculpt_paint/sculpt_face_set.cc#L1465-L1485 I tried the following values `operator`: - `sculpt.face_sets_change_visibility` - `sculpt.face_sets_edit` (I think this has a property `mode` to use with `FAIR_POSITIONS` and `FAIR_TANGENCY`?) From what I could scrape together (this part seems to not have much documentation): `SCULPT_OT_face_sets_change_visibility` defines an operator (OT?), which is called `face_sets_change_visibility` and falls `sculpt`. So it should be called like so: ``` layout.operator("<operator>") ``` Any guidance/documentation would be much appreciated. Update: After watching [this part](https://www.youtube.com/live/tCdx7gzp0Ac?feature=share&t=2439). I think these functions are not yet exposed in the Python API?. @JulienKaspar is this what you meant when you said that "some operators are straightforward and others are more complicated"? </details> So, trying out: - `sculpt.face_set_change_visibility` - `sculpt.face_set_edit` With ***"set"*** instead of "sets" seems to work? I'm so confused... 3. I can't seem to find "Box/Lasso Add". I could only find "Box/Lasso Trim" and "Line Project" in `space_toolsystem_toolbar.py`. Do they have another name? Some notes for new contributors or blender newbies: 1. If builds take too long for you (building on an HDD or slow computer) try `make lite`. I'm not sure if this would be suitable for working on every part of the code, but it works for me. 2. [New developer info/Overview](https://wiki.blender.org/wiki/Developer_Intro/Overview) in the docs can be quite helpful. Especially for [Navigating the code](https://wiki.blender.org/wiki/Developer_Intro/Overview#Navigating_the_Code). I accidently skipped the code layout diagram and it could've saved me a bit of time. 3. [This video](https://www.youtube.com/live/tCdx7gzp0Ac) did a pretty good job of explaining some stuff I was wondering about, and glued things together pretty well (in my mind at least). Please give it a watch if you can.
Contributor

@JulienKaspar
I checked out the rest of the operations and I have some notes/questions. I know you're an artist so you might be not quite familiar with the nitty gritty details of the code, feel free to refer me to someone else.

  • Which operators should get the flag OPTYPE_DEPENDS_ON_CURSOR, or how can I determine that myself? I'm quite inexperienced with sculpting so things aren't quite obvious yet.

  • For menu entries that can be disabled depending on the existence of something, is there a consistent way to query the information I need? For example: Relax Face Set Boundaries (Grayed out if no Face Sets?).

    I did check out the Python API documentation, but it didn't contain the information I needed. I also tried compiling the bpy module and importing it, but it appears there's only data at runtime (inside blender)

  • Currently, the only way for me to inspect Python code at runtime is to add print statements, compile, run, and repeat. Is there a better way to do this? (attaching a debugger, etc...)

  • For the Paint menu, I can't seem to find the types Invert, Levels, and Expand Color by Topology.

  • Also for the Paint menu, I think the Blur type is instead called Smooth, is this correct?

@JulienKaspar I checked out the rest of the operations and I have some notes/questions. I know you're an artist so you might be not quite familiar with the nitty gritty details of the code, feel free to refer me to someone else. - Which operators should get the flag `OPTYPE_DEPENDS_ON_CURSOR`, or how can I determine that myself? I'm quite inexperienced with sculpting so things aren't quite obvious yet. - For menu entries that can be disabled depending on the existence of something, is there a consistent way to query the information I need? For example: `Relax Face Set Boundaries (Grayed out if no Face Sets?)`. I did check out the Python API documentation, but it didn't contain the information I needed. I also tried compiling the `bpy` module and importing it, but it appears there's only data at runtime (inside blender) - Currently, the only way for me to inspect Python code at runtime is to add print statements, compile, run, and repeat. Is there a better way to do this? (attaching a debugger, etc...) - For the `Paint` menu, I can't seem to find the types `Invert`, `Levels`, and `Expand Color by Topology`. - Also for the `Paint` menu, I think the `Blur` type is instead called `Smooth`, is this correct?
Author
Member

@Tarek-Yasser True. Technical information is so far still missing from these tasks.
Refer to @JosephEagar here or on blender.chat for more info.

  1. OPTYPE_DEPENDS_ON_CURSOR should generally be used for operators where the initial cursor position is vital for the operator (Expand) or when the active face set is used, ie. the face set under the cursor (Filters).

  2. I'll edit the main text to be more clear. The Invert and Levels operatorsd do not exist in sculpt mode and are only availbile in the vertex/texture painting modes.
    Instead there are color filters for Brightness and Value.

Expand Color by Topology is available via the sculpt.expand operator and setting the data target to Color.

  1. That is right. I'll fix the description to be more accurate.
@Tarek-Yasser True. Technical information is so far still missing from these tasks. Refer to @JosephEagar here or on blender.chat for more info. 1. `OPTYPE_DEPENDS_ON_CURSOR` should generally be used for operators where the initial cursor position is vital for the operator (Expand) or when the active face set is used, ie. the face set under the cursor (Filters). 4. I'll edit the main text to be more clear. The `Invert` and `Levels` operatorsd do not exist in sculpt mode and are only availbile in the vertex/texture painting modes. Instead there are color filters for `Brightness` and `Value`. `Expand Color by Topology` is available via the `sculpt.expand` operator and setting the data target to `Color`. 5. That is right. I'll fix the description to be more accurate.
Contributor

Quick update:

  • The Sculpt menu PR is still ongoing (hopefully close to done?) on the original branch of the issue (sculpt_mode_add_menu_operators).
  • The rest of the issue for the time being is continuing on 102427_continued.
    • There, I've currently re-arranged the Mask menu to look similar to the posted image, I've also added Mask by Colors in the menu.

I'll try not to dive in too deep as to not make things grow out of hand again.

Quick update: - The `Sculpt` menu PR is still ongoing (hopefully close to done?) on the original branch of the issue (`sculpt_mode_add_menu_operators`). - The rest of the issue for the time being is continuing on `102427_continued`. - There, I've currently re-arranged the `Mask` menu to look similar to the posted image, I've also added `Mask by Colors` in the menu. I'll try not to dive in too deep as to not make things grow out of hand again.
Author
Member

@Tarek-Yasser Thanks! Any issues that are resolved will be marked as done or removed from this task description 👍

@Tarek-Yasser Thanks! Any issues that are resolved will be marked as done or removed from this task description 👍
Contributor

Another update:

  • Finished up the Face Sets menu. Also did a few basic tests to make sure things work.
    Note that the sculpt_face_set_edit operator seems to need the redo panel treatment.

  • Started working on Paint, added all of the menu in the screenshot except Invert and Levels, I can't really find them.
    For "combo" operators (Hue/Saturation/Value), The code seems to treat each as a different operator.
    Was the intention in the original issue to somehow have one entry in the menu for all of them or should I implement them as one entry each?

    Current tool implementation also has each as a different operator/option.
    image
    The main difference between the tool and the menu entry is that the tool allows you to choose a color for Fill, the menu entry does not. It just uses black. I think the most user friendly solution would to be pop up a color picker once the user clicks the menu entry. I'll look into that for a bit (no rabbit holes this time!)

    The rest of the operator variants: Hue, Saturation, ..., Blue just require a strength so they don't need anything to be "functional"

    This operator also lacks a modal keymap, a proper status bar, and the mouse cursor. I'll probably add them like I did with mesh filters.

Another update: - Finished up the `Face Sets` menu. Also did a few basic tests to make sure things work. Note that the `sculpt_face_set_edit` operator seems to need the redo panel treatment. - Started working on `Paint`, added all of the menu in the screenshot except `Invert` and `Levels`, I can't really find them. For "combo" operators (`Hue/Saturation/Value`), The code seems to treat each as a different operator. Was the intention in the original issue to somehow have one entry in the menu for all of them or should I implement them as one entry each? Current tool implementation also has each as a different operator/option. ![image](/attachments/698ab85a-8137-4b49-96b8-19992e31148d) The main difference between the tool and the menu entry is that the tool allows you to choose a color for `Fill`, the menu entry does not. It just uses black. I think the most user friendly solution would to be pop up a color picker once the user clicks the menu entry. I'll look into that for a bit (no rabbit holes this time!) The rest of the operator variants: `Hue`, `Saturation`, ..., `Blue` just require a strength so they don't need anything to be "functional" This operator also lacks a modal keymap, a proper status bar, and the mouse cursor. I'll probably add them like I did with mesh filters.
Member

edit: I don't actually know if popover or operator_menu_hold is correct. Popover interestly integrates as a submenu; I did not try operator_menu_hold

The HSV color filter ops are implemented separately, yes. We could combine them if that would be better.

For Fill, I think you can use UILayout.popover for it. UILayout.operator_menu_hold is another possibility. The former requires subclassing Panel and the latter a Menu. In both cases you'll want to make a color button to the appropriate last operator property, like so:

layout.prop(WindowManager.operator_properties_last("sculpt.color_filter"), "fill_color")
And then make a button to the actual operator with UILayout.operator as normal:

layout.operator("sculpt.color_filter")

I imagine the panel class would look like this:

class VIEW3D_PT_sculpt_fill_popover(Panel):
    bl_space_type = 'VIEW_3D'
    bl_region_type = 'UI'
    bl_category = "Tool"
    bl_label = "Fill"
    
    #This may not be necassary
    bl_context = "popover"
    
    @classmethod
    def poll(cls, context):
        if not self.is_popover:
            return False # Popover only
        if not context.sculpt_object:
            return False
        return True
    
    def draw(self, context):
        layout = self.layout
    
        #Prevent modal version of color filter
        layout.operator_context = "EXEC_DEFAULT"
        
        layout.prop(WindowManager.operator_properties_last("sculpt.color_filter"), "fill_color")
        props = layout.operator("sculpt.color_filter", text="Fill")
        props.type = "FILL"
        props.strength = 1.0

        
*edit: I don't actually know if `popover` or `operator_menu_hold` is correct. `Popover` interestly integrates as a submenu; I did not try `operator_menu_hold`* The HSV color filter ops are implemented separately, yes. We could combine them if that would be better. For Fill, I think you can use `UILayout.popover` for it. `UILayout.operator_menu_hold` is another possibility. The former requires subclassing Panel and the latter a Menu. In both cases you'll want to make a color button to the appropriate last operator property, like so: ` layout.prop(WindowManager.operator_properties_last("sculpt.color_filter"), "fill_color") ` And then make a button to the actual operator with `UILayout.operator` as normal: ` layout.operator("sculpt.color_filter") ` I imagine the panel class would look like this: ``` class VIEW3D_PT_sculpt_fill_popover(Panel): bl_space_type = 'VIEW_3D' bl_region_type = 'UI' bl_category = "Tool" bl_label = "Fill" #This may not be necassary bl_context = "popover" @classmethod def poll(cls, context): if not self.is_popover: return False # Popover only if not context.sculpt_object: return False return True def draw(self, context): layout = self.layout #Prevent modal version of color filter layout.operator_context = "EXEC_DEFAULT" layout.prop(WindowManager.operator_properties_last("sculpt.color_filter"), "fill_color") props = layout.operator("sculpt.color_filter", text="Fill") props.type = "FILL" props.strength = 1.0 ```
Member

Btw I refactored color filter to support the last operator panel and redo last operator, like with mesh filter.

Btw I refactored color filter to support the last operator panel and redo last operator, like with mesh filter.
Contributor

So I spent yesterday playing with the code you posted and looking at the python examples and API documentation.

The code you posted needed some small modifications to work:

class VIEW3D_PT_sculpt_fill_popover(Panel):
    bl_space_type = 'VIEW_3D'
    bl_region_type = 'UI'
    bl_category = "Tool"
    bl_label = "Fill"

    @classmethod
    def poll(cls, context):
        # is_popover doesn't exist on self
        if not context.sculpt_object:
            return False
        return True

    def draw(self, context):
        layout = self.layout

        layout.prop(WindowManager.operator_properties_last("sculpt.color_filter"), "fill_color")
        props = layout.operator("sculpt.color_filter", text="Fill")
        props.type = "FILL"
        props.strength = 1.0 # Doesn't seem to matter?


# ... 
# Don't forget to register!

I don't think the current fill color UI "popover" is correct? Here's what I imagined:

  • You click Paint/Fill Color,
  • You get some color picker to choose the color,
  • You are then asked to choose the object to paint
  • You click and hold, the object is painted with the color depnending on how much you moved the mouse.

Here's how the above snippet renders in the UI:
image


One more thing, layout.operator_context = "EXEC_DEFAULT" doesn't seem to work as intended. I assume the intended behaviour is that you choose the color, then click Fill and click the object, applying the color fill immediately without dragging.
But currently, the following error is printed in the terminal:

ERROR (wm.operator): source/blender/windowmanager/intern/wm_event_system.cc:1540 wm_operator_invoke: invalid operator call 'SCULPT_OT_color_filter'

Removing the line restores the modal behaviour (i.e. clicking and dragging) which works correctly, but I'm not sure if this is what you wanted.


Removing bl_context draws the submenu in "Tools" as shown in the previous image. Adding bl_context = ".sculpt_mode" draws it in the header besides Dyntopo too. Note that "popover" is not a valid option for bl_context.
image

If I misunderstood what's required, just point me to something similar and I'll figure things out.


The redo panel seems to have a bug with Fill Color. If I use the menu entry, then try to change the color or strength through the redo panel, it doesn't work. If I try applying the menu entry again then use the redo panel, the color/strength are then updated properly. Using the tools panel seems to work fine the first time.


Next up: cancelling / modal keymap + cursor.

Another unrelated thing I've noticed is that the viewport shading setting is not saved. For example, when I set it to Render Preview, save the blend file, then re-open, it's reset to Solid.

So I spent yesterday playing with the code you posted and looking at the python examples and API documentation. The code you posted needed some small modifications to work: ```python class VIEW3D_PT_sculpt_fill_popover(Panel): bl_space_type = 'VIEW_3D' bl_region_type = 'UI' bl_category = "Tool" bl_label = "Fill" @classmethod def poll(cls, context): # is_popover doesn't exist on self if not context.sculpt_object: return False return True def draw(self, context): layout = self.layout layout.prop(WindowManager.operator_properties_last("sculpt.color_filter"), "fill_color") props = layout.operator("sculpt.color_filter", text="Fill") props.type = "FILL" props.strength = 1.0 # Doesn't seem to matter? # ... # Don't forget to register! ``` I don't think the current fill color UI "popover" is correct? Here's what I imagined: - You click Paint/Fill Color, - You get some color picker to choose the color, - You are then asked to choose the object to paint - You click and hold, the object is painted with the color depnending on how much you moved the mouse. Here's how the above snippet renders in the UI: ![image](/attachments/87ea8639-8650-49c3-9505-36aa1a9ecd4c) --- One more thing, `layout.operator_context = "EXEC_DEFAULT"` doesn't seem to work as intended. I assume the intended behaviour is that you choose the color, then click `Fill` and click the object, applying the color fill immediately without dragging. But currently, the following error is printed in the terminal: ``` ERROR (wm.operator): source/blender/windowmanager/intern/wm_event_system.cc:1540 wm_operator_invoke: invalid operator call 'SCULPT_OT_color_filter' ``` Removing the line restores the modal behaviour (i.e. clicking and dragging) which works correctly, but I'm not sure if this is what you wanted. --- Removing bl_context draws the submenu in "Tools" as shown in the previous image. Adding `bl_context = ".sculpt_mode"` draws it in the header besides `Dyntopo` too. Note that "popover" is not a valid option for bl_context. ![image](/attachments/05cd444a-e24b-4685-86cc-0a2159e6352a) If I misunderstood what's required, just point me to something similar and I'll figure things out. --- The redo panel seems to have a bug with Fill Color. If I use the menu entry, then try to change the color or strength through the redo panel, it doesn't work. If I try applying the menu entry again then use the redo panel, the color/strength are then updated properly. Using the tools panel seems to work fine the first time. --- Next up: cancelling / modal keymap + cursor. Another unrelated thing I've noticed is that the viewport shading setting is not saved. For example, when I set it to `Render Preview`, save the blend file, then re-open, it's reset to `Solid`.
Author
Member

@Tarek-Yasser Thanks for looking deeper into the Color Fill operator.
It's fair to say that this one is more complex than the other ones and should get more design discussions before going further.

IMO the best way to implement this is to follow conventions of most image editing applications, by adding a color setting to the scene itself.

Any fill operator, tool or brush should then be able to either use the Unified color setting of the scene (default) or define their own color in their tool setting or adjust last operation settings.

This Scene color then needs to be always visible in the header of any mode capable of painting.

@Tarek-Yasser Thanks for looking deeper into the Color Fill operator. It's fair to say that this one is more complex than the other ones and should get more design discussions before going further. IMO the best way to implement this is to follow conventions of most image editing applications, by adding a color setting to the scene itself. Any fill operator, tool or brush should then be able to either use the Unified color setting of the scene (default) or define their own color in their tool setting or adjust last operation settings. This Scene color then needs to be always visible in the header of any mode capable of painting.
Contributor

Ok, let's say we were to implement this. There are a few things that I think should be considered:

  • Which other operators would need to be changed?
  • How would they access the scene color? Would it be stored in the context (the closest thing to a "global" I've met)?
  • Would there be a situation were we have some sort of conflict? As far as I know there should be at most one tool/operator active at one time but there might be a situation I don't know. Perhaps animations?
  • I assume the scene color would need to be saved and loaded along with the blend file too?
  • Signaling to the user that a specific tool's/operator's color is separate from this scene color for operators that have their own color.

If we are to look things we already have, I think it would look something like Vertex/Texture Paint:
image

I could be wrong, but this feels like it could potentially be a separate issue? Depending on the amount of change, this could be something that takes time since it might span a few parts of Blender.

Ok, let's say we were to implement this. There are a few things that I think should be considered: - Which other operators would need to be changed? - How would they access the scene color? Would it be stored in the context (the closest thing to a "global" I've met)? - Would there be a situation were we have some sort of conflict? As far as I know there should be at most one tool/operator active at one time but there might be a situation I don't know. Perhaps animations? - I assume the scene color would need to be saved and loaded along with the blend file too? - Signaling to the user that a specific tool's/operator's color is separate from this scene color for operators that have their own color. If we are to look things we already have, I think it would look something like Vertex/Texture Paint: ![image](/attachments/7bf801d2-6ecb-45ee-8bf7-6553fb672ac8) I could be wrong, but this feels like it could potentially be a separate issue? Depending on the amount of change, this could be something that takes time since it might span a few parts of Blender.
140 KiB
Author
Member

@Tarek-Yasser Yes the fill oeprator can be kept as a separate topic.
Ideally the discussion can continue over on #99769.

I'll scratch it from the description in this task.

But about your questions:
I'm pretty sure a scene color already exists, because of bpy.types.UnifiedPaintSettings.use_unified_color.
This color could be more generally exposed in the header at all times.
This one is also saved with the .blend file.

I would propose that:
If a tool/brush is already defining their own color, then this one takes precedent and will be shown in the header (like in Vertex/Texture Paint Mode) instead and will be used for any color operator instead of the scene color.

@Tarek-Yasser Yes the fill oeprator can be kept as a separate topic. Ideally the discussion can continue over on #99769. I'll scratch it from the description in this task. But about your questions: I'm pretty sure a scene color already exists, because of `bpy.types.UnifiedPaintSettings.use_unified_color`. This color could be more generally exposed in the header at all times. This one is also saved with the .blend file. I would propose that: If a tool/brush is already defining their own color, then this one takes precedent and will be shown in the header (like in Vertex/Texture Paint Mode) instead and will be used for any color operator instead of the scene color.
Contributor

@JulienKaspar @JosephEagar Ok, I think this issue and the #104718 are really done now.

The last push should (hopefully) finalize Color Filter.

Stuff implemented:

  • The mouse cursor now switches to WM_CUROSOR_EW_SCROLL.
    image
  • A modal keymap is implemented. It's currently identical to Filter Mesh.
  • the status bar shows the keys for cancelling and confirming. No icons sadly, haven't figured that out but I'll gladly add them if someone tells me how or where to look. I've already spent a bit of time trying to figure it out previously and don't want to block this issue any longer if possible.
    image
  • Color Filter cancelling. Just Copy-pasted it from Mesh Filter and modified the undo type in SCULPT_orig_vert_data_init to SCULPT_UNDO_COLOR.

The following are some notes and potential docs I wrote down while working.

Notes

  • Expand color by topology crashes?
    Inside sculpt_expand_colors_update_task_cb, vd.mask which is a pointer is dereferenced without any checks.
    Drawing a mask solves the crashing issue but I'm not sure if a mask is even required for this to work?
    So I've got two solutions depending on the required behaviour:

    1. If a mask is required, grey out the button completely.
      The only way I currently know of checking if a mask exists is to loop over the PBVH and check vertices.
      Something like this:
      for vertex in pbvh:
        if vertex.mask:
          return true
      
      I'm not quite sure if there's a better way (cached somewhere perhaps?)
    2. If a mask is not required, we can just check if the mask is nullptr before dereferencing it.
  • Filter Mesh/Color share some code.
    If the code is not going to change much, it might be a good issue for new developers to pull out this common code.

    Here's a list of the code I know is mostly duplicated:

    • Modal keymap setup
    • Operator cancel function
    • Modal event handling inside the modal function.
    • Status bar update code

Using keymap actions in operator code.

This follows from Filter Mesh, you can find steps to how the modal keymap was implemented in a comment in #104718.

if (event->type == EVT_MODAL_MAP) {
  int ret = MY_OP_CONFIRM;

  switch (event->val) {
    case MY_OP_CANCEL:
      /* Operator cancelling code (clean up, reverting changes, etc...) */
      return OPERATOR_CANCELLED;

    case MY_OP_CONFIRM: 
      /* Confirmation code (adding undo nodes, clean up, etc...) */
      return OPERATOR_FINISHED;

    /* Other cases if needed */
  }
}
@JulienKaspar @JosephEagar Ok, I think this issue and the #104718 are really done now. The last push should (hopefully) finalize Color Filter. Stuff implemented: - The mouse cursor now switches to `WM_CUROSOR_EW_SCROLL`. ![image](/attachments/d57c4482-4e00-44c1-b4de-9a3daaed032e) - A modal keymap is implemented. It's currently identical to Filter Mesh. - the status bar shows the keys for cancelling and confirming. No icons sadly, haven't figured that out but I'll gladly add them if someone tells me how or where to look. I've already spent a bit of time trying to figure it out previously and don't want to block this issue any longer if possible. ![image](/attachments/54f41927-c92a-4b34-b83f-f9c0b8a0eff5) - Color Filter cancelling. Just Copy-pasted it from Mesh Filter and modified the undo type in `SCULPT_orig_vert_data_init` to `SCULPT_UNDO_COLOR`. The following are some notes and potential docs I wrote down while working. # Notes - Expand color by topology crashes? Inside `sculpt_expand_colors_update_task_cb`, `vd.mask` which is a pointer is dereferenced without any checks. Drawing a mask solves the crashing issue but I'm not sure if a mask is even required for this to work? So I've got two solutions depending on the required behaviour: 1. If a mask is required, grey out the button completely. The only way I currently know of checking if a mask exists is to loop over the PBVH and check vertices. Something like this: ``` for vertex in pbvh: if vertex.mask: return true ``` I'm not quite sure if there's a better way (cached somewhere perhaps?) 2. If a mask is not required, we can just check if the mask is `nullptr` before dereferencing it. - Filter Mesh/Color share some code. If the code is not going to change much, it might be a good issue for new developers to pull out this common code. Here's a list of the code I know is mostly duplicated: - Modal keymap setup - Operator cancel function - Modal event handling inside the modal function. - Status bar update code # Using keymap actions in operator code. This follows from Filter Mesh, you can find steps to how the modal keymap was implemented in a comment in #104718. ```C if (event->type == EVT_MODAL_MAP) { int ret = MY_OP_CONFIRM; switch (event->val) { case MY_OP_CANCEL: /* Operator cancelling code (clean up, reverting changes, etc...) */ return OPERATOR_CANCELLED; case MY_OP_CONFIRM: /* Confirmation code (adding undo nodes, clean up, etc...) */ return OPERATOR_FINISHED; /* Other cases if needed */ } } ````
1.7 KiB
3.6 KiB
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: blender/blender#102427
No description provided.