Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

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

Merged
merged 1 commit into from
Jan 2, 2024
Merged

Conversation

joanbm
Copy link
Contributor

@joanbm joanbm commented Aug 30, 2023

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
Copy link
Owner

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 joanbm marked this pull request as draft October 19, 2023 23:00
@joanbm
Copy link
Contributor Author

joanbm commented Oct 19, 2023

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

@joanbm
Copy link
Contributor Author

joanbm commented Oct 23, 2023

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.

wxBitmap::SetWidth and wxBitmap::SetHeight have been deprecated since
wxWidgets 3.1.2 and are removed in the current v3.3 development trunk.
Use wxBitmap::GetSubBitmap as an alternative.
@joanbm joanbm changed the title Two small wxWidgets deprecation cleanups & future-proofing Replace deprecated wxBitmap methods (fix build on wxWidgets 3.3) Jan 2, 2024
@joanbm joanbm marked this pull request as ready for review January 2, 2024 00:30
@joanbm
Copy link
Contributor Author

joanbm commented Jan 2, 2024

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 99d044e.

@sirjuddington sirjuddington merged commit 7e3ea4f into sirjuddington:master Jan 2, 2024
4 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants