Skip to content

fix: size paint brushes in voxels - #981

Merged
PaulHax merged 1 commit into
Kitware:mainfrom
PaulHax:brush-size-label
Oct 21, 2026
Merged

PaulHax merged 1 commit into
Kitware:mainfrom
PaulHax:brush-size-label

Conversation

@PaulHax

@PaulHax PaulHax commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Small voxels can make the minimum brush paint several voxels, while the fixed maximum of 50 ignores image dimensions. Measure brush diameter relative to the image's finest voxel spacing so size 1 paints a single voxel. Cap the size at 10% of the largest voxel dimension, rounded up with a minimum of 1, to keep small brushes easy to select.

Show the current value while dragging and explain the units. The slider, keyboard shortcuts, and modifier-wheel adjustments share the same integer limits, and the preview respects the image under the cursor.

@netlify

netlify Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for volview-dev ready!

Name Link
🔨 Latest commit da3acc8
🔍 Latest deploy log https://app.netlify.com/projects/volview-dev/deploys/6ad80a799a77b286d81e99c3
😎 Deploy Preview https://deploy-preview-981--volview-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@PaulHax PaulHax changed the title fix: show brush size while adjusting slider fix: size paint brushes in voxels Oct 20, 2026
@PaulHax
PaulHax force-pushed the brush-size-label branch 2 times, most recently from c610eea to a888c96 Compare October 20, 2026 19:44

@PaulHax PaulHax left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Hopeing we can smplify all this work

let checkIfPointerInView = false;
const pointerInView = ref(false);
watch(
[() => paintStore.brushSize, imageMetadata, viewAxisIndex, pointerInView],

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We really should prefer watch effect over these manual watch definitions. It needs to be justified.

max="50"
step="1"
/>
<div class="brush-size-control">

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This whole change on this block seems a bit much just to get normal spacing. Why are we doing grid? Just to have a row with even spacing on the slider there. Seems like overkill, this whole big change here. Try to simplify this. Prefer Beautify utility classes, of course, to avoid, like, defining new classes if we don't have to.

Comment thread src/store/tools/paint.ts Outdated
getBrushSizeLimit(currentImageMetadata.value.dimensions)
);

watch([currentImageData, brushSizeMax], ([image, max]) => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Again, we're defining the manual things to watch, prefer, watch effect, if it's even needed, you know.

@PaulHax
PaulHax merged commit c46c4ea into Kitware:main Oct 21, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant