Make default cursor resource work for Cocoa (PR #22391)
oneeyeman1
With this it will be possible to load a cur file on Cocoa from the Bundle resources.
I am putting this more for discussing this possibility.
If this code is OK'ed I might add the possibility to fall back to the png if the cur file is not available.
Could someone please take a look?
Thx in advance.
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/22391
Commit Summary
- e811232 Make default cursor resource work for Cocoa
File Changes
(3 files)
- M include/wx/osx/cocoa/private.h (1)
- M src/osx/carbon/cursor.cpp (2)
- M src/osx/carbon/utilscocoa.mm (20)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/22391.patch
- https://github.com/wxWidgets/wxWidgets/pull/22391.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 1 commit.
- db6e3e4 Attempt to fix iPhone build
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@csomor Could you please look at this? I have no idea about how cursors are supposed to be handled under Mac.
—
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.
why wouldn't you just use the std-path of the Resources and use a wxImage to read in the CUR file and create a wxCursor from that ?
In src/osx/carbon/utilscocoa.mm:
> + NSDictionary *properties = (__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex( image, 0, nil ); + CGFloat x = [[properties objectForKey:@"hotspotX"]floatValue];
I think you could use a wxCFDictionaryRef here, no need to go to NSDictionary. And then a GetValue("hotspotX").GetValue(&x)
In src/osx/carbon/utilscocoa.mm:
> @@ -613,6 +613,27 @@ void wxMacCocoaShowCursor()
{
[NSCursor unhide];
}
+
+WX_NSCursor wxMacCocoaCreateCursorFromResource(const wxString &cursor_file, wxBitmapType flags)
+{
+ NSString *path;
+ NSBundle *bundle = [NSBundle mainBundle];
+ if( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
+ path = [bundle pathForResource:wxCFStringRef( cursor_file ).AsNSString() ofType:@"cur"];
+ if( path )
+ {
+ NSURL *url = [NSURL fileURLWithPath:path];
+ CGImageSourceRef image = CGImageSourceCreateWithURL( (__bridge CFURLRef) url, nil );
+ NSDictionary *properties = (__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex( image, 0, nil );
+ CGFloat x = [[properties objectForKey:@"hotspotX"]floatValue];
+ CGFloat y = [[properties objectForKey:@"hotspotY"]floatValue];
+ CFBridgingRelease(image);
We only need a simple CFRelease(image) here I think ...
In src/osx/carbon/utilscocoa.mm:
> +
+WX_NSCursor wxMacCocoaCreateCursorFromResource(const wxString &cursor_file, wxBitmapType flags)
+{
+ NSString *path;
+ NSBundle *bundle = [NSBundle mainBundle];
+ if( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
+ path = [bundle pathForResource:wxCFStringRef( cursor_file ).AsNSString() ofType:@"cur"];
+ if( path )
+ {
+ NSURL *url = [NSURL fileURLWithPath:path];
+ CGImageSourceRef image = CGImageSourceCreateWithURL( (__bridge CFURLRef) url, nil );
+ NSDictionary *properties = (__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex( image, 0, nil );
+ CGFloat x = [[properties objectForKey:@"hotspotX"]floatValue];
+ CGFloat y = [[properties objectForKey:@"hotspotY"]floatValue];
+ CFBridgingRelease(image);
+ CFBridgingRelease((__bridge CFTypeRef _Nullable)(properties));
if we use a wxCFDictionaryRef no explicit release is needed anymore
In src/osx/carbon/utilscocoa.mm:
> @@ -613,6 +613,27 @@ void wxMacCocoaShowCursor()
{
[NSCursor unhide];
}
+
+WX_NSCursor wxMacCocoaCreateCursorFromResource(const wxString &cursor_file, wxBitmapType flags)
+{
+ NSString *path;
+ NSBundle *bundle = [NSBundle mainBundle];
+ if( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
I wouldn't say a MACCURSOR is typically a .CUR file ... this rang belongs to .PNG IMHO
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 commented on this pull request.
In src/osx/carbon/utilscocoa.mm:
> @@ -613,6 +613,27 @@ void wxMacCocoaShowCursor()
{
[NSCursor unhide];
}
+
+WX_NSCursor wxMacCocoaCreateCursorFromResource(const wxString &cursor_file, wxBitmapType flags)
+{
+ NSString *path;
+ NSBundle *bundle = [NSBundle mainBundle];
+ if( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
@csomor,
I can make it to work with PNG.
I was not sure if the "CUR" file can be easily converted to "PNG".
I will also see if I can address other comments.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
why wouldn't you just use the std-path of the Resources and use a
wxImageto read in the CUR file and create awxCursorfrom that ?
I was hoping to retrieve the hotspot coordinates and construct the cursor this way.
I don't know how OS identifies those coordinates, so...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
> + NSDictionary *properties = (__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex( image, 0, nil ); + CGFloat x = [[properties objectForKey:@"hotspotX"]floatValue];
@csomor ,
I tried following code:
wxCFDictionaryRef properties( (wxCFDictionaryRef) CGImageSourceCopyPropertiesAtIndex( image, 0, nil ) );
properties.GetValue( "hotspotX" ).GetValue( &x );
properties.GetValue( "hotspotY" ).GetValue( &y );
and it failed on the retrieval of the x - BAD-EXEC exception.
Any suggestions?
—
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.
In src/osx/carbon/utilscocoa.mm:
> + NSDictionary *properties = (__bridge NSDictionary *)CGImageSourceCopyPropertiesAtIndex( image, 0, nil ); + CGFloat x = [[properties objectForKey:@"hotspotX"]floatValue];
A PNG does not have a hotspot, that's why you have to indicate it in the other parameters of the wxCursor from file parameter, an CUR does however, so the wxImage constructed from it already does know about the hotspot. We have everything in the our code already.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
why wouldn't you just use the std-path of the Resources and use a
wxImageto read in the CUR file and create awxCursorfrom that ?I was hoping to retrieve the hotspot coordinates and construct the cursor this way.
I don't know how OS identifies those coordinates, so...
You don't have to, as wx code already knows how to do that, only in the case eg of a png I'd add the hotspot from the parameters via
image.SetOption(wxIMAGE_OPTION_CUR_HOTSPOT_X, hotSpotX);
image.SetOption(wxIMAGE_OPTION_CUR_HOTSPOT_X, hotSpotY);
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
Thank you for the input. I understand now.
Is it OK to make it a PNG by default, fall back to CUR if PNG is not found and if both are not there - give a message to the developer?
Or just implement in terms of PNG and that's it?
Thx.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 1 commit.
- dad5dbe Implement PR suggestions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
I reimplemented this as you suggested.
For right now, I did it just for PNG.
Let me know if we need a fallback or message box is inappropriate.
I will modify the docs after I hear OK from either you or Vadim.
Thank you.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
Can you take a look?
Maybe it can be included in 3.1.7 then...
TIA!
—
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.
> + m_refData->DecRef() ; + m_refData = NULL ;
for PNG we need to set the hotspot explicitly in the image as in the else clause, also bracket with wxUSE_IMAGE
> @@ -278,7 +278,16 @@ wxCursor::wxCursor(const wxString& cursor_file, wxBitmapType flags, int hotSpotX
if ( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
{
#if wxOSX_USE_COCOA
- wxFAIL_MSG( wxT("Not implemented") );
+ wxString fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png";
+ wxImage image( fileName, wxBITMAP_TYPE_PNG );
+ if( image.IsOk() )
+ {
+ m_refData->DecRef() ;
+ m_refData = NULL ;
+ InitFromImage( image );
+ }
+ else
+ wxMessageBox( "No PNG cursor image found in Resources" );
we should log an error IMHO, or wxFAIL_MSG, but definitely not pop-up a message box
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 commented on this pull request.
@csomor ,
I will verify it tonight, but I think it will be set in the "InitFromImage()".
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
> @@ -278,7 +278,16 @@ wxCursor::wxCursor(const wxString& cursor_file, wxBitmapType flags, int hotSpotX
if ( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
{
#if wxOSX_USE_COCOA
- wxFAIL_MSG( wxT("Not implemented") );
+ wxString fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png";
+ wxImage image( fileName, wxBITMAP_TYPE_PNG );
+ if( image.IsOk() )
+ {
+ m_refData->DecRef() ;
+ m_refData = NULL ;
+ InitFromImage( image );
+ }
+ else
+ wxMessageBox( "No PNG cursor image found in Resources" );
Yes, will change that. Probably use wxFAIL_MSG.
Do you think it will make sense to use CUR file as fall back?
—
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.
> + m_refData->DecRef() ; + m_refData = NULL ;
No wxImage code can only set it for a format that itself does know about the hotspot location, eg CUR, but not for PNG which does not carry this information
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 commented on this pull request.
@csomor,
Got it.
Do yo think it is a good idea to add a fallback to CUR file if PNG is not found?
—
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.
> @@ -278,7 +278,16 @@ wxCursor::wxCursor(const wxString& cursor_file, wxBitmapType flags, int hotSpotX
if ( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
{
#if wxOSX_USE_COCOA
- wxFAIL_MSG( wxT("Not implemented") );
+ wxString fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png";
+ wxImage image( fileName, wxBITMAP_TYPE_PNG );
+ if( image.IsOk() )
+ {
+ m_refData->DecRef() ;
+ m_refData = NULL ;
+ InitFromImage( image );
+ }
+ else
+ wxMessageBox( "No PNG cursor image found in Resources" );
Do you think it will make sense to use CUR file as fall back?
this would be a good idea especially as CUR does support multi-res - although my current macOS code does not yet translate this properly
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 1 commit.
- bf05bea More PR suggestions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor,
Hopefully this now is good.
Take a look and let me know.
If it is - I will try to modify the documentation so that it can be included in the upcoming 3.1.7 release.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 1 commit.
- 2be2c50 Fix the documentation for this functionality
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
I tried to fix documentation but I'm sure my words can be improved.
Also, I don't know if the unit test is needed here.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
P.S.: If you could please look at #22412 and give me a clue of where to look for the fix it would be great.
Sorry, I don't have the time to do that at the moment, it may very well be that macOS is taking over in cursor management
—
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.
thanks we are getting closer I think, @vadz could you please have a look as well, Thanks
> @@ -138,11 +138,8 @@ class wxCursor : public wxGDIObject
(to load a cursor from a .ico icon file) and @c wxBITMAP_TYPE_ANI
(to load a cursor from a .ani icon file).
- under MacOS, it defaults to @c wxBITMAP_TYPE_MACCURSOR_RESOURCE;
- when specifying a string resource name, first the color cursors 'crsr'
- and then the black/white cursors 'CURS' in the resource chain are scanned
- through. Note that resource forks are deprecated on macOS so this
- is only available for legacy reasons and should not be used in
- new code.
+ when specifying a string resource name, first 'png' and then 'cur'
+ image are searched in resources.
I'm not an authority on wording, it's a foreign language to me as well, but I think either we go
.. first a PNG and then a CUR image is searched or
... first PNG and then CUR images are searched
I'm slightly preferring the first variant
> m_refData->DecRef() ;
m_refData = NULL ;
InitFromImage( image );
}
else
- wxMessageBox( "No PNG cursor image found in Resources" );
+ {
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".cur";
+ wxImage image( fileName, wxBITMAP_TYPE_CUR );
+ if( image.IsOk() )
+ {
+ InitFromImage( image );
I think here we need a
m_refData->DecRef() ;
m_refData = NULL ;
as well
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
P.S.: If you could please look at #22412 and give me a clue of where to look for the fix it would be great.
Sorry, I don't have the time to do that at the moment, it may very well be that macOS is taking over in cursor management
Whenever, you have time.
I wasn't sure if you saw the report as only Vadim commented on it.
And yes - you may be right. Running under lldb I see the cursor is changing, but then CaptureMouse() is called...
If not for an immediate fix - maybe a quick workaround..
And I can test a possible fix as well...
TIA
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 commented on this pull request.
> m_refData->DecRef() ;
m_refData = NULL ;
InitFromImage( image );
}
else
- wxMessageBox( "No PNG cursor image found in Resources" );
+ {
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".cur";
+ wxImage image( fileName, wxBITMAP_TYPE_CUR );
+ if( image.IsOk() )
+ {
+ InitFromImage( image );
Yes, sorry about that.
Will fix after Vadim approves documentation wording.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 2 commits.
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 commented on this pull request.
> @@ -138,11 +138,8 @@ class wxCursor : public wxGDIObject
(to load a cursor from a .ico icon file) and @c wxBITMAP_TYPE_ANI
(to load a cursor from a .ani icon file).
- under MacOS, it defaults to @c wxBITMAP_TYPE_MACCURSOR_RESOURCE;
- when specifying a string resource name, first the color cursors 'crsr'
- and then the black/white cursors 'CURS' in the resource chain are scanned
- through. Note that resource forks are deprecated on macOS so this
- is only available for legacy reasons and should not be used in
- new code.
+ when specifying a string resource name, first 'png' and then 'cur'
+ image are searched in resources.
@csomor ,
I followed you first suggestion and fixed it.
Hopefully someone will be able to push this before 3.1.7...
Thx.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I'm still very confused by the logic of this PR. Why do we need to support CUR files under macOS? It's not a natural format for them there, AFAIK nobody uses them there. If you want to allow any file format, as the existing code using wxImage::LoadFile() does, just do it like this. If you just want to use PNGs -- this is fine too. But supporting only PNG and CUR doesn't make much sense to me.
Next point is that, of course, we shouldn't duplicate the existing code in the else branch. Just construct the file path differently in this case and then reuse the existing code...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
What would be the best fallback format if PNG is not available?
Or do you think we can use wxImage in this case and only then fail?
I'm not really familiar with the internals of OSX, therefore I'd like to ask...
And Vadim' point is probably a good one - CUR file format will look weird under OSX.
Any pointers?
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@vadz ,
Do you think we need a unit test for this?
I mean both MSW and OSX will have a support for that and so we can try to test if this works.
And maybe for GTK as well.
Thank you.
—
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 , What would be the best fallback format if PNG is not available? Or do you think we can use wxImage in this case and only then fail?
using wxImage is fine if it can deduce the format, perfect, we should just avoid code duplication, Vadim is right there. CUR as a format is not so unreasonable because it is in itself multi-resolution capable, although our code does not deal with it right now, but of course multi-res PNGs are way more frequently used.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
So do you suggest to keep the CUR format under OSX?
Thank you.
—
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 , So do you suggest to keep the CUR format under OSX? But it will be just as fallback if it's not available...
I think Vadim is right, we can support all formats, by just using wxImage::LoadFile if the PNG attempt fails, and have no explicit test with a CUR extension, thus supporting every format (wxBITMAP_TYPE_ANY being default), so to keep the code small, I'just interleave this perhaps along these lines (I haven't tested it, so perhaps I overlooked something):
wxImage image( fileName, wxBITMAP_TYPE_PNG );
if( !image.IsOk() )
{
fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file;
image.LoadFile(filename)
}
if( image.IsOk() )
...
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 1 commit.
- f0d71ab Implement PR suggestion
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@vadz ,
Any idea what happened with the failed build here?
It looks like it failed to install something on the build VM...
Thx.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Scott Talbert
@vadz , Any idea what happened with the failed build here? It looks like it failed to install something on the build VM...
Thx.
Actually, it looks like the version of pip being installed (19.1.1) is not compatible with Python 3.10. collections.Mapping was moved to collections.abc in Python 3.10.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
@vadz , Any idea what happened with the failed build here? It looks like it failed to install something on the build VM...
Thx.Actually, it looks like the version of pip being installed (19.1.1) is not compatible with Python 3.10.
collections.Mappingwas moved tocollections.abcin Python 3.10.
so should I downgrade python to 3.9 or upgrade pip
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Sorry, I just don't have time to look at this now, but I suspect we have to use latest pip for this macOS version then. Of course, just using latest pip everywhere doesn't work, or at least didn't, in the past, see e.g. this comment: https://github.com/wxWidgets/wxWidgets/blob/8c9b09dfa95a5ea8d4d1df2da01183b2dc1cb3ce/build/tools/httpbin.sh#L60-L62
I really, really hate Python ecosystem with a passion.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Scott Talbert
Sorry, I just don't have time to look at this now, but I suspect we have to use latest pip for this macOS version then. Of course, just using latest pip everywhere doesn't work, or at least didn't, in the past, see e.g. this comment:
I really, really hate Python ecosystem with a passion.
I can take a stab at fixing it - I have an idea which should make everything much simpler.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
Stefan Csomor
@vadz , Any idea what happened with the failed build here? It looks like it failed to install something on the build VM...
Thx.Actually, it looks like the version of pip being installed (19.1.1) is not compatible with Python 3.10.
collections.Mappingwas moved tocollections.abcin Python 3.10.
I just checked system python is at 3.9 on the self-hosted runner
I realize that cmake task is not using our self-hosted runner ... sorry for the noise
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@vadz,
I think it is time to create a unit test to check if the cursor loading from the resource on all platforms works.
I can add this here or make a new PR - whatever your prefer.
Let me know what is the best way.
(If you'd want this PR to be updated - it will have to wait).
Thank you.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
@vadz requested changes on this pull request.
Sorry, but this is still not ready to be applied unfortunately.
> @@ -17,9 +17,9 @@
#include "wx/icon.h"
#include "wx/image.h"
#endif // WX_PRECOMP
-
+#include "wx/msgdlg.h"
Not needed:
⬇️ Suggested change-#include "wx/msgdlg.h"
> + wxString fileName; + wxImage image; + fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png"; + image.LoadFile( fileName, wxBITMAP_TYPE_PNG );
This would be more readable
⬇️ Suggested change- wxString fileName; - wxImage image; - fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png"; - image.LoadFile( fileName, wxBITMAP_TYPE_PNG ); + const wxFileName fileName(wxStandardPaths::Get().GetResourcesDir(), cursor_file, ".png"); + wxImage image(fileName.GetFullPath(), wxBITMAP_TYPE_PNG);
> @@ -278,7 +278,28 @@ wxCursor::wxCursor(const wxString& cursor_file, wxBitmapType flags, int hotSpotX
if ( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
{
#if wxOSX_USE_COCOA
- wxFAIL_MSG( wxT("Not implemented") );
+ wxString fileName;
+ wxImage image;
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png";
+ image.LoadFile( fileName, wxBITMAP_TYPE_PNG );
+ if( image.IsOk() )
+ {
+ image.SetOption( wxIMAGE_OPTION_CUR_HOTSPOT_X, hotSpotX ) ;
+ image.SetOption( wxIMAGE_OPTION_CUR_HOTSPOT_Y, hotSpotY ) ;
+ }
+ else
+ {
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".cur";
Should also use wxFileName() here.
> + if( image.IsOk() )
+ {
+ m_refData->DecRef() ;
+ m_refData = NULL ;
This really shouldn't be done, it makes zero sense to allocate new wxCursorRefData just to delete it immediately after.
> + image.SetOption( wxIMAGE_OPTION_CUR_HOTSPOT_X, hotSpotX ) ;
+ image.SetOption( wxIMAGE_OPTION_CUR_HOTSPOT_Y, hotSpotY ) ;
+ }
+ else
+ {
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".cur";
+ image.LoadFile( fileName, wxBITMAP_TYPE_CUR );
+ }
+ if( image.IsOk() )
+ {
+ m_refData->DecRef() ;
+ m_refData = NULL ;
+ InitFromImage( image );
+ }
+ else
+ wxFAIL_MSG( "No PNG cursor image found in Resources" );
This probably should be wxLogError(). The error message is also misleading because it tries both PNG and CUR.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 commented on this pull request.
> @@ -278,7 +278,28 @@ wxCursor::wxCursor(const wxString& cursor_file, wxBitmapType flags, int hotSpotX
if ( flags == wxBITMAP_TYPE_MACCURSOR_RESOURCE )
{
#if wxOSX_USE_COCOA
- wxFAIL_MSG( wxT("Not implemented") );
+ wxString fileName;
+ wxImage image;
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".png";
+ image.LoadFile( fileName, wxBITMAP_TYPE_PNG );
+ if( image.IsOk() )
+ {
+ image.SetOption( wxIMAGE_OPTION_CUR_HOTSPOT_X, hotSpotX ) ;
+ image.SetOption( wxIMAGE_OPTION_CUR_HOTSPOT_Y, hotSpotY ) ;
+ }
+ else
+ {
+ fileName = wxStandardPaths::Get().GetResourcesDir() + "/" + cursor_file + ".cur";
@vadz ,
I'm getting the following:
../src/osx/carbon/cursor.cpp:290:13: error: type 'wxFileName' does not provide a call operator
fileName( wxStandardPaths::Get().GetResourcesDir(), cursor_file, ".cur" );
^~~~~~~~
../src/osx/carbon/cursor.cpp:291:13: error: type 'wxImage' does not provide a call operator
image( fileName.GetFullPath(), wxBITMAP_TYPE_CUR );
^~~~~
2 errors generated.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@csomor ,
How do I add the cursor file (png) to the Bundle during the build?
I want to add the test code for this....
Thank you.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@oneeyeman1 pushed 1 commit.
- d90f315 Implement PR suggestions
—
View it on GitHub or unsubscribe.
You are receiving this because you are subscribed to this thread.
oneeyeman1
@vadz ,
For some reason 1 build got cancelled and now the log file is unaccessible.
Should it be restarted?
Thank you.
—
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 , How do I add the cursor file (png) to the Bundle during the build? I want to add the test code for this....
in the Makefile.in of the tests you see how the icns file is copied into xxx.app/Contents/Resources
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
Closed #22391.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.
VZ
I think this got replaced by #24374.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.