HaikuArchives / ArtPaint

ArtPaint is a painting and image processing program.
https://haikuarchives.github.io/ArtPaint/
29 stars 18 forks source link

kbeitrag.deBrushTool: move drawing functions into Brush class instead of BrushTool #497

Closed dsizzle closed 1 year ago

dsizzle commented 1 year ago

Make ToolManager store brush and pass it to BrushTool ToolManager::GetCurrentBrush() returns default brush if brush is not defined Add composite_func option to BitmapUtilities::CompositeBitmapOnSource Add BrushView to StatusView in main window; commented out for now

part of #177; still a work-in-progress but wanted to push part of it.

These changes should be invisible.

humdingerb commented 1 year ago

So, WIPpy no mergy, right?

I played around with the sliders of the Brush setup and after a bit of twddling I get a crash. I think while moving the size slider. I managed to reproduce 3 times: ArtPaint_debug-15307-debug-29-03-2023-12-26-36.report.txt ArtPaint_debug-17976-debug-29-03-2023-12-32-04.report.txt ArtPaint_debug-19344-debug-29-03-2023-12-33-02.report.txt

I then reverted to master, and lo!, crashes too. Here just one report: ArtPaint_debug-19734-debug-29-03-2023-12-34-14.report.txt

Open a ticket for it, or are you investigating in the course of this PR?

dsizzle commented 1 year ago

Yes, please open a ticket for the crash. It’s been around for a while and I keep forgetting about it. I’ve only seen it when doing a lot of brush editing. It’s either a race of some kind that we occasionally lose, or it’s simply because of the way brushes are implemented. Moving a brush slider is rapidly allocating and deallocating memory, and depending on the brush size it’s more memory than you may expect. Each brush has basically 9 copies of itself: the main one, and then one for each of the 8 directions it can move so that it draws correctly with no weird overlaps. I’ve never been a fan of how it works and so I’ve spent time trying to just keep 1 copy, but so far I haven’t gotten the math to work right. Anyway I said all that to say that the crash could be that we are simply allocing and freeing memory fast enough to just chew it all up?

humdingerb commented 1 year ago

Ticket created: #498 Tell me once this is ready to merge.

PeteCA commented 1 year ago

On Wed, Mar 29, 2023 at 05:43:55AM -0700, humdinger wrote:

So, WIPpy no mergy, right?

I played around with the sliders of the Brush setup and after a bit of twddling I get a crash. I think while moving the size slider.

Might be worth reporting that I get apparently the same crash. I didn't report it because I'm still running a fairly ancient version of Haiku (54154 June 2020). I cloned v2.5 from 8 March, and the main program mostly runs fine, but only 10 of the add-ons actually load -- and selected areas come out all-black, which I suspect is not intended! I've been trying to get around to updating Haiku, but so far haven't managed that.

I don't see the crash happening with a circular brush, but change to an elliptical aspect, and move the size slider. it happens fairly quickly.

dsizzle commented 1 year ago

the main program mostly runs fine, but only 10 of the add-ons actually load -- and selected areas come out all-black, which I suspect is not intended!

@PeteCA it's definitely not intended; the black selections/canvas issues were fixed ages ago so you definitely shouldn't see that. I didn't think it was related to the Haiku version.

Tell me once this is ready to merge.

@humdingerb as long as you don't see any problems besides the aforementioned crash, please merge. Thanks!

PeteCA commented 1 year ago

On Wed, Mar 29, 2023 at 08:23:12PM -0700, Dale Cieslak wrote:

the main program mostly runs fine, but only 10 of the add-ons actually load -- and selected areas come out all-black, which I suspect is not intended!

@PeteCA it's definitely not intended; the black selections/canvas issues were fixed ages ago so you definitely shouldn't see that. I didn't think it was related to the Haiku version.

Yeah, I was surprised, as I remember that was one of the first things you fixed. I don't know why it still happens with my older version, but I'm not going to worry about it until I actualy manage to update Haiku. (Everything else works as far as I could tell, except the missing add-ons.)