GNOME Bugzilla – Bug 150227
PIPE_SELECT_VELOCITY unimplemented for painting with brush pipes.
Last modified: 2006-01-31 20:21:05 UTC
While doing my homework for BUG #127663, I found out that the velocity option was not used in the GIMP. Just open app/core/gimpbruspipe.c and look down in gimp_brush_pipe_select. Selection by velocity (which should work more or less like the pen tool) just is not implemented there, although it is handled in .GIH file save. I've made a small patch to implement it.
Created attachment 30581 [details] [review] Implements brush-pipe select by velocity in the GIMP.
While editing this file, I found out that the random generator is created on for every brush stroke in gimp_brush_pipe_select_brush . It would not hurt to have it to be created once with the pipe brush class, and to be destroyed with it, I think.
The above patch, although implementing roughly the functionality, doesn't work fine. It does work if the strokes are being painted in realtime. If, however, stroke rendering is delayed, due to a (not so) large brush, or slow machinne, the timing information received is useless, and the speed is calculated as if it was the minimum. For this to work properly, it would have to implement time smoothing techniques just as the ink tool does.
Moin. Moving this to future myself. The attached patch works fine, until the slowdown effects reported in bug #148205 kick in. At this point, the coordinates passed to the painting function are interpolated, and one can no longer guess the pointer speed from them. To properly implement this, the time smoothing code in the Ink tool should be used. As such, it will have to be factored out from there to a more general place. Therefore Bug #143216 and bug #119240 will possibly be implemented as well. These are good enhancements for the GIMP and have nothing to do with GEGL, so they might be taken on early on the next devel. cycle.
Comment on attachment 30581 [details] [review] Implements brush-pipe select by velocity in the GIMP. Not sure I understand the bug, but from Joao's last comment, he doesn't think this should be committed.
Indeed. To sum up: the implementation in this patch is machine speed and available memory dependant. The bug itself is a simple request: The animated brushes have several ways to select which image to use to stroke with. They may be selectable randomly, sequentialy, by pressure, or by the angle of the stroke. There is also, in the enumerations, the option of selecting the brush by the speed of the stroke. (It also works in the .GIH save settings) Handling brush stroking by velocity, however, is not implemented in the CVS code.
Status of this is unclear. Comment #4 indicates that bug #148205 was blocking the usefulness of the patch, but that bug is now fixed. Comment #4 also refers to two other bugs, but it isn't clear how important they are. So, is the patch an improvment over the current code at this point?
So, guess what? I never changed this file again after I posted this patch here, it is still on my personnal GIMP...and it is working fine now! :-) hah...code de-obsolescence brought to you by the GIMP Team. Thanks guys! I will recreate the patch, because the file has been changed in CVS - I remember having read about someone fixing the issue with the random generator above. The other two bugs I mention, IIRC, are other brush/painting related enhacements that depend on this, rather than the other way around.
Created attachment 37954 [details] [review] Updated patch - no changes in the code. It is working finenow. I tried it with a 480x480x9 brush on the same machinne it was not ok with a 64x64x9 brush on previous tests.
Great! Removing NEEDINFO; now just needs code review.
Comment on attachment 37954 [details] [review] Updated patch - no changes in the code. I am not sure but it appears to me that there's a possibility for a division by zero or is it guaranteed that spacing never becomes zero? Otherwise this looks good to me.
This should probably be committed since that's what has been decided last year already, but another review probably wouldn't hurt.
We shouldn't let this patch get any older, IMO. If no one else steps forward, I'll try to apply it to my tree tonight and will report back.
Created attachment 58433 [details] gih velocity examples The good news: the patch applies and velocity is now applicable for GIH brushes. The bad news: it is almost unusable on my system. When doing straight line strokes, it doesn't seem got get the endpoints correct. For free strokes, it seems to be impossible to maintain a velocity. There's also no possiblity for the user to adjust the sensitivity, is there? See the attached screenshot for examples.
Created attachment 58434 [details] brush used for examples
Hi schumanl! So, this is why I have not pushed it anymore - not of much use, is it? I get more or less the same results. However, thanks. I guess it is better this than nothing. A way to calibrate the speed sounds nice, indeed. For sure the day the GIMP implements BUG #119240 we would have a working speed calibration. Hmmm..maybe speed is a good candidate to begin implementation of that one. A finner than a second time resolution for GDK events would be really nice, though.
Maybe we should commit this patch anyway - it works; it is not perfect, but it hasn't been available yet so this shouldn't have an impact on existing brushes (and if it has, those probably have to be changed). Additionally, tweakable stuff might attract more new developers than things that have to be created from scratch.
2006-01-31 Michael Schumacher <schumaml@cvs.gnome.org> * app/core/gimpbrushpipe.c: applying a patch by Joao S. O. Bueno Calligaris which implements PIPE_SELECT_VELOCITY for brush pipes. Fixes bug #150227.