OpenCPN / OpenCPN

A concise ChartPlotter/Navigator. A cross-platform ship-borne GUI application supporting * GPS/GPDS Postition Input * BSB Raster Chart Display * S57 Vector ENChart Display * AIS Input Decoding * Waypoint Autopilot Navigation
https://opencpn.org/
GNU General Public License v2.0
1.04k stars 495 forks source link

wxBitmap not handled properly #3286

Open leamas opened 1 year ago

leamas commented 1 year ago

Describe the bug The code is littered with dynamically allocated wxBitmap objects. This makes for a lot of trouble since these are Copy On Write classes designed to be used by value.

To Reproduce

Grep for new wxBitmap and find lot's of matches.

Expected behavior There should be no new wxBitmap. Instead, there should be plain wxBitmap objects which lets wxWidgets handle lifecycle, assigments, etc. This will create faster and more robust code.

Additional context

Docs: https://docs.wxwidgets.org/3.0/overview_refcount.html

There is even examples of using Unshare in the code; this is documented as a tool which should be used when creating custom COW objects based on wxWidgets (which there is absolutely no need for in in a C++11 context).

The current situation is said to be a work around for problems with wxWidgets COW implementation in certain old compilers. This is history since long.

rgleason commented 1 year ago

It appears that some plugins use a lot of wxBitmap. Is this a problem for plugins too?

leamas commented 1 year ago

yes.

However, we have the fact that several functions in the API also uses wxBitmap* which adds some more complexity.

leamas commented 1 month ago

The bug is valid, and we need to at least avoid new allocations. However, this needs to be handled over time.