Skip to content

Brush tool improvements - #173

Open
StickmanRed wants to merge 55 commits into
developmentfrom
StickmanRed-brush-resolution-setting
Open

StickmanRed wants to merge 55 commits into
developmentfrom
StickmanRed-brush-resolution-setting

Conversation

@StickmanRed

@StickmanRed StickmanRed commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Features

  • Improved brush settings UI in Inspector
  • Brush shape options
  • Brush spacing and resolution sliders
  • Randomized brush offset and rotation options

@StickmanRed StickmanRed linked an issue Jul 14, 2026 that may be closed by this pull request
@StickmanRed
StickmanRed marked this pull request as ready for review July 23, 2026 20:27
@StickmanRed StickmanRed changed the title Brush resolution setting Brush tool improvements Aug 20, 2026
@StickmanRed StickmanRed added the enhancement New feature or request label Aug 20, 2026
@StickmanRed StickmanRed linked an issue Aug 20, 2026 that may be closed by this pull request
@Alani25
Alani25 requested a review from BaronAWC September 7, 2026 21:23

@InternetAstronaut InternetAstronaut left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@BaronAWC BaronAWC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all I wanna say this is awesome, thanks for putting in all this effort. It works well but I wanna see if we can improve the experience. It's a long and scary list but I think these are all important to address.

Clarity

  • "Reso" (resolution) doesn't really make sense as a name from the user's perspective. As devs we know why it's resolution, but it would be nice if resolution was renamed to something that everyone can understand. My suggestion would be "quality".
  • Spread should not have a button to toggle a slider between 0.01 and the max, it should just be a slider between 0 and the max. Also, it's called "scatter" in one place and "spread" in another, so let's make sure that the name is standardized.
  • We should consider removing some of the preset shapes and/or putting them in a dropdown. While they're nice to have, it creates the illusion of so many buttons that it's overwhelming. The plus sign also makes me think that it's a button to add a new brush shape, which is not how it works. I think that, at minimum, we should remove the rough, scatter, and plus. The dropdown is so that we can hide away all the buttons when we don't really need them.
  • The "delete brush" button should be grayed out and non-interactive if there is only one brush.
  • There are a bunch of buttons that should be moved or removed.
    • In the brush selector, the "add brush" and "upload brush" buttons should be related. This could mean putting them in a dropdown where all the other brushes are, or 2 large buttons where "edit brush" is now.
    • Also in the brush selector, "edit brush" and "delete brush" should be similar. In my opinion it's weird that the edit button is big while the delete button is small. They should probably go together at the bottom.
    • The way that "delete brush" and "delete shape" are both trash can buttons in the same location is really confusing. The previous checkbox already has my suggestion for delete brush. For delete shape, it really shouldn't be in the same row of buttons as everything else since it serves a completely different function. I don't know where exactly it should go... but not where it is now.
    • When editing a brush, the pressure, relative size, and mode buttons should not be there. They are accessible when outside of the brush editor because they can change at any time without "altering the fundamental brush". Therefore, the brush editor should not show these, as it's only for the "important" settings.

Customizability

  • Some shapes like the thin rectangle rotate to follow the cursor as it moves, but other brushes like the square don't. This makes me think that the rotation thing is defined in the brush shape, which doesn't make sense. I think there should be 3 rotation options: set offset angle (default is zero), follow mouse (with offset angle), and randomize angle.

Visual Improvements

  • When dragging the sliders, the preview flickers quite a bit, which is distracting. Hopefully there's a way to remove the flickering?
  • I found some inconsistencies in the shapes. The sparkle in the preview is in an "X" shape, but it's in a "+" shape everywhere else. The moon looks different in all 3 places: on the canvas it's closest to a "C" shape, on the button it's not as sharp, and in the preview it's cut weirdly.
  • When using a big brush size, the brush path before releasing the mouse is pretty low resolution. Is it possible to increase the resolution so that it looks nicer?
  • The triangle shape is not equilateral... I don't know, that kinda bugs me. (If we fix this, make the centroid/center of mass the middle of the shape if that's even possible)

Safeguards

  • We should have preset brushes that cannot be significantly modified or deleted. For the standard brush, the only customizable settings should what we offer in the current version of Candlestick, plus maybe the resolution/smoothing.
  • It's a little dangerous to be able to delete some of the fundamental shapes, especially the circle. I toyed with deleting stuff and I had to clear my cookies to bring back the default shapes. I'd say that the default shapes should not be allowed to be deleted, or at bare minimum the circle.
  • There should be a cancel button to exit the brush config without saving it.

@Alani25

Alani25 commented Sep 7, 2026

Copy link
Copy Markdown
Member

"Reso" (resolution) doesn't really make sense as a name from the user's perspective. As devs we know why it's resolution, but it would be nice if resolution was renamed to something that everyone can understand. My suggestion would be "quality".

I was aiming for a shorter name to not have it cut off and reso sounded catchy lol
that said yeh quality is definitely more descriptive

Spread should not have a button to toggle a slider between 0.01 and the max

It started as a button, later turned to a slider, then I increased lower limit to 0.01 because having it at 0 bugged something— that said it makes a lot more sense to have it be just a slider correct yeh and I'll just turn it off if it's 0.

Also, it's called "scatter" in one place and "spread" in another

You're right! Idk how I missed that ahh few weeks back I called it scatter later in the code I just decided to call it spread when I made the slider 😓

We should consider removing some of the preset shapes and/or putting them in a dropdown.

A dropdown is possible but an easier option I was actually considering is horizontal scroll 🤔

When editing a brush, the pressure, relative size, and mode buttons should not be there. They are accessible when outside of the brush editor because they can change at any time without "altering the fundamental brush". Therefore, the brush editor should not show these, as it's only for the "important" settings.

Good point here this I can do
Also, in regards to all the points with the trash can button… might be a good idea to have a mini "x" that appears when hovering over the brush presets/ shape options…

3 rotation options: set offset angle (default is zero), follow mouse (with offset angle), and randomize angle.

Agree with you there

When dragging the sliders, the preview flickers quite a bit, which is distracting
I noticed… kinda a given since we're updating an image element and re-rendering. I've thought of that and a way to stop it— was just too lazy to work on it but should be doable

When using a big brush size, the brush path before releasing the mouse is pretty low resolution. Is it possible to increase the resolution so that it looks nicer?
I'll say "no" to that for now… but will see what i can do there

We should have preset brushes that cannot be significantly modified or deleted
I can agree with that that said we'd need to have some indicator there or just Idk— that said an "adobe brush" has been demanded and would be great to have as a preset option

There should be a cancel button to exit the brush config without saving it.
Perhaps adding a "<-" button up there next to the name parameter, then replacing the save brush button with a "reset settings" 🤔

@Alani25
Alani25 requested a review from BaronAWC September 13, 2026 03:48
@Alani25

Alani25 commented Sep 13, 2026

Copy link
Copy Markdown
Member

Note to self: Allow drag and dropping .cbrush files to import brush assets

@BaronAWC

BaronAWC commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Few more things I think we should change:

  • The default brush should be plain
  • There is an old bit of code that adds 1 to the brush size, which we should remove
  • Brush scatter internally has some magic numbers, being 1.4 and 2.8 somewhere. After i figure out what they're there for, we should consider removing them, rounding them to nicer numbers, etc.
  • Relative brush size should be off by default
  • The sparkle behaves poorly if it's too small. We should consider making it a little less thin on the edges
  • The highest quality should(?) be higher than it currently is
  • The preview should show a slightly more smaller, zoomed-out version. It's hard to see the true effect of settings when it's too zoomed in
  • The preview doesn't look very accurate, and it seems to have a LOT of magic numbers. After I give it a closer look I'd like to reduce the magic numbers as much as possible
  • The brush creator/editor should not have a setting for size and lead
  • The scatter brush shape should be removed, as it's basically a redundant version of the circle with built-in spread. We could probably bring back a different shape
  • I think the UI still has room for improvement but I'm not 100% sure what we should do right now... I'll do some research and talk over button placement when the time comes
    • A brush should be selected the first time you enable the brush tool
    • The inspector title should(?) say "Brush Tool" instead of the brush's name, to match how other titles work (e.g. clips with names)
    • Downloading a brush (and maybe deleting it) should not be hidden behind editing it
    • The fixed angle mode should make the brush's icon set at the fixed angle, in the brush selector and in the canvas (e.g. a 45-degree offset square brush should show as a diamond)
    • The path-angle mode should make the brush's canvas preview rotate to follow the brush

I'll do a couple of these really quick tonight. I'll see what else I can do over this upcoming week.

brush is a normal circle brush
brushSize+1 is removed, since that is a remnant of the old wick brush
default brush rotation is "fixed"
brush rotation dropdown reordered to "fixed", "path", "random" to follow the natural order from no-rotation to all-rotation
"new brush" tooltip renamed to "create"

@BaronAWC BaronAWC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code-related comments:

Main comments here are about moving code out of Inspector.jsx and into either Brush.js or a new file(s).

Additionally, in the code itself, we should standardize terms like spread/scatter, resolution/quality, etc.

// Default brushes — structured like a parsed .cbrush JSON for easy migration later.
// Each field matches the keys stored/loaded by saveBrush / applyBrush.
// FIXME: these "default" values are in 3 places (search for this same comment)
const DEFAULT_BRUSHES = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this default brush doesn't have to be in Inspector.jsx, it makes more sense either in Brush.js or possibly somewhere else

},
];

const BRUSH_SHAPES = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would imagine this is redundant, since the brush shapes exist somewhere else. When creating a new brush shape, the editor also knows how to create the preview, so I'd imagine it's doable for these default shapes too.

svg: <><circle cx="14" cy="14" r="4"/><circle cx="7" cy="8" r="3"/><circle cx="21" cy="8" r="2.5"/><circle cx="7" cy="20" r="3"/><circle cx="21" cy="20" r="2.5"/></> },
];

const ROTATION_MODE_OPTIONS = [

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These only seem to be used once, so I wonder if it makes more sense to hardcode

}

// BRUSH PREVIEW— we're literally drawing a brush stroke here -H.A.
drawBrushPreview = () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the brush preview go to a separate file? It's not really suitable in the inspector file

if (this.brushFileInputRef.current) this.brushFileInputRef.current.click();
}

handleBrushFileImport = (e) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import/export could probably go into a different file - doesn't make sense in inspector

@@ -70,10 +70,6 @@ class ToolSettings extends Component {
return (
<div className='settings-input-container'>
{this.renderBrushSize()}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we may not even need brush size here, since it's in the inspector now

)
}

renderBrushResolution = () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unused?

Comment thread engine/src/tools/Brush.js
}

// Crescent needs compositing — handle separately
if (shape === 'crescent') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the brush shapes go into a separate file?

@BaronAWC

Copy link
Copy Markdown
Collaborator

I quickly fixed the default brush settings in one final place, which should make it consistent with the other 3 places. (Speaking of which, we should try to only have it in one place)

Also, I'm not sure how to reproduce this, but sometimes the fill color of the brush preview will disappear entirely, leaving just the outline. Has anyone else encountered this?

@StickmanRed

Copy link
Copy Markdown
Collaborator Author

I quickly fixed the default brush settings in one final place, which should make it consistent with the other 3 places. (Speaking of which, we should try to only have it in one place)

I think we should try to move all the brush shape-drawing logic into an engine file, maybe tools/BrushUtils.js. The logic is replicated several times, just with different color, position, or rotation, but relative to those factors, the shape-drawing commands should be the same. This could be a function Wick.Brush.Utils.drawBrushShape(ctx, size, shape, args) that can be used for the cursor image, brush tip, and brush preview, saving duplicated logic and ensuring a consistent brush shape.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] brush precision decreases with smaller brushes New Brushes

4 participants