sirjuddington / SLADE

It's a Doom editor
https://slade.mancubus.net
GNU General Public License v2.0
686 stars 104 forks source link

Replace deprecated wxBitmap methods (fix build on wxWidgets 3.3) #1561

Closed joanbm closed 6 months ago

joanbm commented 10 months ago

Stop using some deprecated wxBitmap methods (wxBitmap::SetWidth, wxBitmap::SetHeight) which have been deprecated since wxWidgets 3.1.2 and are removed in the current v3.3 development trunk.

With this SLADE builds fine on the current wxWidgets 3.3 development trunk. I also verified the changes against wxWidgets (wxGTK) 3.0.5 and 3.2.4.

sirjuddington commented 9 months ago

Unfortunately this breaks tooltips in windows. It looks like there is some kind of bug with the wxBitmap copy constructor (unless there's something I'm missing) - the line buffer_ = scratch_.GetSubBitmap(... makes buffer_ just an empty black bitmap for me on wxMSW 3.2.2.

I managed to get around the issue by removing buffer_ altogether, keeping a size member variable and using scratch_.GetSubBitmap directly with the size when drawing the tooltip. Another alternative is to do the same thing but just draw scratch_ at full size directly without GetSubBitmap, which avoids creating a new bitmap each time it's redrawn. I suppose with this workaround scratch_ could also just be renamed back to buffer_ :P

joanbm commented 8 months ago

There's indeed something going wrong on Windows with the bitmaps - I'll take a look...

joanbm commented 8 months ago

FWIW, the fact that wxBitmap::GetSubBitmap returns a black bitmap isn't due to a wxWidgets bug but rather due an API misuse in the PR - the wxBitmap must be selected out of the wxMemoryDC before reading the image data.

joanbm commented 6 months ago

PR updated, Windows works now.

I kept the same approach of using wxBitmap::GetSubBitmap, but properly managing the lifetime of the DC associated to the bitmap now. Though I think your suggestion of drawing the entire 1000x1000 bitmap directly with wxDC::DrawBitmap should also work (if I understand correctly the DC should clip the out of bounds contents).

PS: I also dropped the part of the PR fixing SToolBarGroup::addActionGroup since that was already done by 99d044ef0e7ea0887470a7aa0667f026eb727d18.