search

saalfeldlab / paintera

GNU General Public License v2.0
100 stars 17 forks source link

Always change brush radius by multiplication and remove upper bound for brush radius #355

Closed hanslovsky closed 4 years ago

hanslovsky commented 4 years ago

Fixes #354

I removed the upper bound for the brush radius completely but it may make sense to actually have an upper bound, e.g. the longest diagonal of the dataset in world space.

hanslovsky commented 4 years ago

Brush radius (> 0) and resize scale factor (> 1) can now be adjusted through the UI in the Canvas tab of a label source.

hanslovsky commented 4 years ago

@axtimwalde @igorpisarev What do you think, should the brush radius have an upper bound? I removed it because it was buggy. On the other hand, it does not make any sense to paint with a larger brush than the source, so we could limit the brush radius to the longest spatial diagonal.

axtimwalde commented 4 years ago

That's true but also very large. Not sure what the benefit over not having an upper bound would be?

hanslovsky commented 4 years ago

Mostly, it is unclear what the upper bound should be. The current screen size? At least in the current (master branch) implementation, this will run into issues, e.g. zooming in after setting the brush size, it could happen that now the radius is bigger than the viewer window, and the brush radius would not get updated. If there is a upper bound, it should probably not depend on a variable quantity like the viewer window size but on the data somehow. But I have no good idea, what this upper limit could be.

igorpisarev commented 4 years ago

I find the current screen size to be a reasonable upper limit, since in my understanding it doesn't make sense for the user to paint outside the visible area. We could also adjust the brush size whenever the viewer window size changes. Or is there something that I'm missing?

hanslovsky commented 4 years ago

The current upper limit is buggy (using the width and height of the viewer in viewer space but does not transform to world space). I agree that an upper limit makes sense but I do not like to implicitly change the brush size. That is something the user should have control over. Finding a good upper limit can be discussed in a separate issue though. If there are no concerns with everything else in this PR, I will go ahead and merge.