Design: Brush Assets Tool System Integration #126032

New Issue

Julian Eisel · 2024-08-07T13:05:20+02:00

Julian Eisel commented

2024-08-07 13:05:20 +02:00

Detailed design for allowing tools to use brushes with the new brush asset system.

Much of this is already implemented (partially as WIP) in blender/blender!125449.

Motivation

Since brush assets were merged, all sculpt and paint modes have a unified "Brush" tool that is used for all brushes.
Based on feedback, some tools that use brushes should be brought back for the grease pencil draw mode. Namely the eraser tool and fill tool, but the primitive tools that are still present should also still be able to use any draw brush, as before.

This means we need to bring back some of the old behavior, where tools and brushes have some level of connection.

I can also imagine that we'd want to bring back some brush based tools in other sculpt/paint modes. For example the pose or cloth brushes are quite distinct from normal sculpt brushes. Having dedicated pose and cloth tools might be useful (the "Brush" tool still allows selecting pose and cloth brushes directly). So best to not hardcode this to the grease pencil use case too much.

Design

UI

Brush selector popups only show compatible brushes, e.g. eraser brushes for the eraser tool
Asset shelf region at the bottom of the screen always shows all brushes
Brush tool shows all brushes (also in brush selector popups)
When activating a brush incompatible with the current tool (e.g. draw brush with the eraser active), the brush tool is activated
Never switch away from the brush tool automatically. E.g. activating an eraser brush while the brush tool is active doesn't activate the eraser
Tools remember the last used brush

This should give predictable behavior where the brush tool always gives access to all brushes, and users can optionally use tools for convenient access to special kinds of brushes, whereby the last used one is remembered.

Tool definition

The eraser tool would be defined like this:

@ToolDef.from_fn
def erase():
    return dict(
        idname="builtin_brush.Erase",
        label="Erase",
        icon="brush.gpencil_draw.erase",
        brush_type='ERASE',
        options={'USES_BRUSHES'},
    )

Changes:

New USES_BRUSH flag (so "Brush" tool can still support any brush) -- blender/blender!125911
Brush type is limited via new brush_type argument
Remove data_block tool definition member. Not needed with the above

Special Case: Grease Pencil Primitive Tools

The grease pencil primitive tools to draw shapes (lines, cirles, rectangles, ...) never set the data_block which usually indicated that a tool supported brushes. It was just forced to display a brush selector which happened to work. It doesn't anymore.

Instead:

Properly set brush info: Set USES_BRUSHES and brush_type='DRAW'
This breaks cursors and keymaps (get overridden by paint cursor & keymap), so:
- No longer set paint cursors when the tool sets a custom cursor
- Disable paint keymap for primtivie tools, hardcoded exception in keymap poll

Filtering Brushes in the UI

Brush assets store the brush types for all compatible modes in their asset metadata -- blender/blender!124618
BrushAssetShelf.asset_poll() filters out brushes when displayed in the popup (checked via context) based on the active tool (from context) and the asset metadata. -- Done in blender/blender!125449.

Open Question: How to Connect Tools With Brushes?

We need to somehow...:

... remember the brush used for a tool
... set a default brush for each tool (e.g. soft eraser)
... set the remembered brush on tool changes
... update the tool when active brush changes

See first comment for possible solutions.

Detailed design for allowing tools to use brushes with the new brush asset system. Much of this is already implemented (partially as WIP) in blender/blender!125449. ## Motivation Since brush assets were merged, all sculpt and paint modes have a unified "Brush" tool that is used for all brushes. Based on feedback, some tools that use brushes should be brought back for the grease pencil draw mode. Namely the eraser tool and fill tool, but the primitive tools that are still present should also still be able to use any draw brush, as before. This means we need to bring back some of the old behavior, where tools and brushes have some level of connection. I can also imagine that we'd want to bring back some brush based tools in other sculpt/paint modes. For example the pose or cloth brushes are quite distinct from normal sculpt brushes. Having dedicated pose and cloth tools might be useful (the "Brush" tool still allows selecting pose and cloth brushes directly). So best to not hardcode this to the grease pencil use case too much. ## Design ### UI - Brush selector popups only show compatible brushes, e.g. eraser brushes for the eraser tool - Asset shelf region at the bottom of the screen always shows all brushes - _Brush_ tool shows all brushes (also in brush selector popups) - When activating a brush incompatible with the current tool (e.g. draw brush with the eraser active), the brush tool is activated - Never switch away from the brush tool automatically. E.g. activating an eraser brush while the brush tool is active doesn't activate the eraser - Tools remember the last used brush This should give predictable behavior where the brush tool always gives access to all brushes, and users can optionally use tools for convenient access to special kinds of brushes, whereby the last used one is remembered. ### Tool definition The eraser tool would be defined like this: ```py @ToolDef.from_fn def erase(): return dict( idname="builtin_brush.Erase", label="Erase", icon="brush.gpencil_draw.erase", brush_type='ERASE', options={'USES_BRUSHES'}, ) ``` Changes: - New `USES_BRUSH` flag (so "Brush" tool can still support any brush) -- blender/blender!125911 - Brush type is limited via new `brush_type` argument - Remove `data_block` tool definition member. Not needed with the above ### Special Case: Grease Pencil Primitive Tools The grease pencil primitive tools to draw shapes (lines, cirles, rectangles, ...) never set the `data_block` which usually indicated that a tool supported brushes. It was just forced to display a brush selector which happened to work. It doesn't anymore. Instead: - Properly set brush info: Set `USES_BRUSHES` and `brush_type='DRAW'` - This breaks cursors and keymaps (get overridden by paint cursor & keymap), so: - No longer set paint cursors when the tool sets a custom cursor - Disable paint keymap for primtivie tools, hardcoded exception in keymap poll ### Filtering Brushes in the UI - Brush assets store the brush types for all compatible modes in their asset metadata -- blender/blender!124618 - `BrushAssetShelf.asset_poll()` filters out brushes when displayed in the popup (checked via context) based on the active tool (from context) and the asset metadata. -- Done in blender/blender!125449. ### Open Question: How to Connect Tools With Brushes? We need to somehow...: - ... remember the brush used for a tool - ... set a default brush for each tool (e.g. soft eraser) - ... set the remembered brush on tool changes - ... update the tool when active brush changes See first comment for possible solutions.

Julian Eisel added the

Type

Design

label 2024-08-07 13:05:20 +02:00

Julian Eisel added this to the Brush Assets & Asset Shelf project 2024-08-07 13:05:20 +02:00

Julian Eisel commented

2024-08-07 13:13:00 +02:00

Open Question: How to Connect Tools With Brushes?

We need to somehow...:

... remember the brush used for a tool

... set a default brush for each tool (e.g. soft eraser)

... set the remembered brush on tool changes

... update the tool when active brush changes

Solution 1: Bring back `PaintToolSlot`

Proof of Concept: #126030

Bring back previous PaintToolSlot design, adopted to brush assets.

Array of PaintToolSlot items stored in Paint of each mode (scene data), indexed by brush type
Store AssetWeakReference instead of brush directly, used brushes are now restored from asset libraries on load, not saved to files
Was nice to have gotten rid of this, some hundred lines of code and some devs didn't really understand what this was doing
However, code can be simplified quite a bit still because of brush assets design. Can also do further improvements (readability, documentation, etc).
Index access might not be that reliable over version changes
toolsystem_ref_link() updates active brush on tool changes

Solution 2: Variation of `PaintToolSlot`

Instead of array, use linked list and identify item by brush type name
Essentially models a name -> asset-reference map

Solution 3: Store in the Tool Itself

Store AssetWeakReference in bToolRef
Should be much simpler, and can be reused for by other tools that might want to store some kind of active asset
Problem: Primitive tools would all store their own brush, not be synced anymore

Open Questions

How to set the default brush for a tool? Probably fine to hardcode it, but where does it make sense? (Could also imagine passing this to the tool definition.)
All solutions use asset references that can get invalid when the asset name or path changes. Can we update them?
For solution 1 & 2: How to store the "default" brush, like the one used by the "Brush" tool? Ideas:
- Temporary store a restore_brush in Paint
- Add a override_brush to Paint and return that from BKE_paint_brush() when set.

> ### Open Question: How to Connect Tools With Brushes? > > We need to somehow...: > > - ... remember the brush used for a tool > - ... set a default brush for each tool (e.g. soft eraser) > - ... set the remembered brush on tool changes > - ... update the tool when active brush changes ### Solution 1: Bring back `PaintToolSlot` Proof of Concept: #126030 Bring back previous `PaintToolSlot` design, adopted to brush assets. - Array of `PaintToolSlot` items stored in `Paint` of each mode (scene data), indexed by brush type - Store `AssetWeakReference` instead of brush directly, used brushes are now restored from asset libraries on load, not saved to files - Was nice to have gotten rid of this, some hundred lines of code and some devs didn't really understand what this was doing - However, code can be simplified quite a bit still because of brush assets design. Can also do further improvements (readability, documentation, etc). - Index access might not be that reliable over version changes - `toolsystem_ref_link()` updates active brush on tool changes ### Solution 2: Variation of `PaintToolSlot` - Instead of array, use linked list and identify item by brush type name - Essentially models a `name -> asset-reference` map ### Solution 3: Store in the Tool Itself - Store `AssetWeakReference` in `bToolRef` - Should be much simpler, and can be reused for by other tools that might want to store some kind of active asset - Problem: Primitive tools would all store their own brush, not be synced anymore ## Open Questions - How to set the default brush for a tool? Probably fine to hardcode it, but where does it make sense? (Could also imagine passing this to the tool definition.) - All solutions use asset references that can get invalid when the asset name or path changes. Can we update them? - For solution 1 & 2: How to store the "default" brush, like the one used by the "Brush" tool? Ideas: - Temporary store a `restore_brush` in `Paint` - Add a `override_brush` to `Paint` and return that from `BKE_paint_brush()` when set.

Nika Kutsniashvili commented

2024-08-07 13:16:42 +02:00

When activating a brush incompatible with the current tool (e.g. draw brush with the eraser active), the brush tool is activated
Never automatically switch to another tool. E.g. when activating an eraser brush while the brush tool is active doesn't select the eraser

This confuses me. So if you you have Eraser tool selected and click draw brush in asset shelf it will select Brush tool, but if you have Brush tool selected and click eraser brush in asset shelf it will NOT switch to eraser tool?

> When activating a brush incompatible with the current tool (e.g. draw brush with the eraser active), the brush tool is activated > Never automatically switch to another tool. E.g. when activating an eraser brush while the brush tool is active doesn't select the eraser This confuses me. So if you you have Eraser tool selected and click draw brush in asset shelf it will select Brush tool, but if you have Brush tool selected and click eraser brush in asset shelf it will NOT switch to eraser tool?

Julian Eisel commented

2024-08-07 16:26:20 +02:00

This confuses me. So if you you have Eraser tool selected and click draw brush in asset shelf it will select Brush tool, but if you have Brush tool selected and click eraser brush in asset shelf it will NOT switch to eraser tool?

Yes exactly. You can do all painting with just the brush tool and don't have to switch back and forth to get all brushes available, only if you choose to use the tool system. Of course needs testing & feedback to see how it works in practice.

> This confuses me. So if you you have Eraser tool selected and click draw brush in asset shelf it will select Brush tool, but if you have Brush tool selected and click eraser brush in asset shelf it will NOT switch to eraser tool? Yes exactly. You can do all painting with just the brush tool and don't have to switch back and forth to get all brushes available, only if you choose to use the tool system. Of course needs testing & feedback to see how it works in practice.

Nika Kutsniashvili commented

2024-08-07 16:33:21 +02:00

This confuses me. So if you you have Eraser tool selected and click draw brush in asset shelf it will select Brush tool, but if you have Brush tool selected and click eraser brush in asset shelf it will NOT switch to eraser tool?

Yes exactly. You can do all painting with just the brush tool and don't have to switch back and forth to get all brushes available, only if you choose to use the tool system. Of course needs testing & feedback to see how it works in practice.

It is bit confusing imho. I think if you pick eraser from asset shelf it should switch to eraser tool (and same for bucket), otherwise its confusing when tool gets switched and when not. Photoshop brush system works the same - all brushes for all tools are always available in list, and when you pick a brush that is for different tool tool changes automatically. It makes sense and is good, proven workflow. Otherwise you have to know extra thing about system.

> > This confuses me. So if you you have Eraser tool selected and click draw brush in asset shelf it will select Brush tool, but if you have Brush tool selected and click eraser brush in asset shelf it will NOT switch to eraser tool? > > Yes exactly. You can do all painting with just the brush tool and don't have to switch back and forth to get all brushes available, only if you choose to use the tool system. Of course needs testing & feedback to see how it works in practice. It is bit confusing imho. I think if you pick eraser from asset shelf it should switch to eraser tool (and same for bucket), otherwise its confusing when tool gets switched and when not. Photoshop brush system works the same - all brushes for all tools are always available in list, and when you pick a brush that is for different tool tool changes automatically. It makes sense and is good, proven workflow. Otherwise you have to know extra thing about system.

Julian Eisel referenced this issue

2024-08-26 18:42:26 +02:00

Refactor: Sculpt/Paint: Rename brush "tool" to "brush type" #126796

Julian Eisel referenced this issue from a commit

2024-08-30 17:00:34 +02:00

Sculpt/paint: Write brush types to brush asset metadata

Julian Eisel referenced this issue from a commit

2024-09-03 15:21:00 +02:00

Refactor: Sculpt/Paint: Rename brush "tool" to "brush type"

Julian Eisel referenced this issue

2024-09-05 18:16:33 +02:00

Grease Pencil: Bring back paint mode tools using brush assets #125449