OSX make iOS and macOS share more drawing code (PR #25860)
Stefan Csomor
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/25860
Commit Summary
- e073d8d fix for wxRegionGeneric behavior
- e21d727 fixing icon sizes on iOS
- 0c6e9d7 have an availability macro for iOS code as well
- 3d38cfe make macOS and iOS share more image drawing code
- 007a2c6 add native artprovider for macOS and iOS
- dc7ab21 have system color closer to macOS
File Changes
(8 files)
- M include/wx/osx/core/private.h (6)
- M include/wx/osx/private/available.h (3)
- M src/common/dcgraph.cpp (4)
- M src/generic/regiong.cpp (4)
- M src/osx/artmac.cpp (117)
- M src/osx/carbon/graphics.cpp (201)
- M src/osx/carbon/utilscocoa.mm (68)
- M src/osx/iphone/settings.mm (106)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/25860.patch
- https://github.com/wxWidgets/wxWidgets/pull/25860.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> @@ -375,7 +375,9 @@ bool wxRegionGeneric::DoOffset(wxCoord x, wxCoord y)
bool wxRegionGeneric::IsEmpty() const
{
- wxASSERT(m_refData);
+ if ( m_refData == nullptr )
Do we really want to do this? I think invalid and empty regions are not the same thing, just as transparent and invalid pens/brushes, for example.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
@csomor commented on this pull request.
> @@ -375,7 +375,9 @@ bool wxRegionGeneric::DoOffset(wxCoord x, wxCoord y)
bool wxRegionGeneric::IsEmpty() const
{
- wxASSERT(m_refData);
+ if ( m_refData == nullptr )
Sorry, I wrote this twice as I've somehow managed to loose a set of commits, the reason behind it is the documented behavior:
https://docs.wxwidgets.org/3.3/classwx_region.html#a1edc6768118cf02749b46774a0ca37f9
a freshly created wxRegion is invalid but also empty
and apparently other platforms behave like this, already, I just ran into an assertion when setting the background color on a wxFrame on iOS, which doesn't have a native region class.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz commented on this pull request.
> @@ -375,7 +375,9 @@ bool wxRegionGeneric::DoOffset(wxCoord x, wxCoord y)
bool wxRegionGeneric::IsEmpty() const
{
- wxASSERT(m_refData);
+ if ( m_refData == nullptr )
Ha, I had even added this comment to the docs myself back in dd4eefc (Document and test behaviour of wxRegion methods when it is invalid., 2011-10-18) but, of course, completely forgot about it. And, as the commit says, there is even a unit test ("wxRegion::Validity") checking for this, so this would have been discovered if we ran unit tests under iOS...
Anyhow, sorry, for not checking this first.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
@csomor commented on this pull request.
> @@ -375,7 +375,9 @@ bool wxRegionGeneric::DoOffset(wxCoord x, wxCoord y)
bool wxRegionGeneric::IsEmpty() const
{
- wxASSERT(m_refData);
+ if ( m_refData == nullptr )
no problem, I really appreciate you looking at things closely
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
@csomor pushed 1 commit.
- 73d6d04 add scaling to retrieved system graphics
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
@csomor pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
csomor left a comment (wxWidgets/wxWidgets#25860)@vadz Sorry, but I need your help, I don't understand why the cirrus ci / FreeBSD suddenly fails, I don't see how I could have changed something in GTK land, can you see a reason ?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
vadz left a comment (wxWidgets/wxWidgets#25860)It looks like they have updated GTK version which now (since 2.44 which is too new to even have docs for it online apparently) deprecates a bunch of gdk_pixbuf_loader_xxx() functions. This is completely unrelated to your changes and I'll try to fix this as soon as I can in master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
csomor left a comment (wxWidgets/wxWidgets#25860)It looks like they have updated GTK version which now (since 2.44 which is too new to even have docs for it online apparently) deprecates a bunch of
gdk_pixbuf_loader_xxx()functions. This is completely unrelated to your changes and I'll try to fix this as soon as I can in master.
Thanks a lot, ok I ll dare to slash merge then
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
Merged #25860 into master.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Václav Slavík
vslavik left a comment (wxWidgets/wxWidgets#25860)The wxArtProvider part of these changes broke loading of system (incl. stored in Resources or asset catalog) bitmaps, which was part of wx since at least 3.0. Specifically, the removal of
return wxOSXCreateSystemBitmapBundle(id, client, size);from CreateBitmapBundle: 20ff462#diff-3acbe1af29f31859a4eb1e08c767105fb787e8f23e4cf62add7a05605c8db887L40-L44
Can the old behavior please be restored?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
csomor left a comment (wxWidgets/wxWidgets#25860)Hi @vslavik sorry about that, I'll look into it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.