close
Skip to content

Implement extra cursors on macOS#1425

Closed
mantognini wants to merge 2 commits into
masterfrom
feature/more_cursors
Closed

Implement extra cursors on macOS#1425
mantognini wants to merge 2 commits into
masterfrom
feature/more_cursors

Conversation

@mantognini

Copy link
Copy Markdown
Member

Superseds #1413 & #1401.

@JonnyPtn when carefully reviewing your PR I noticed a few issues in my original code so I've fixed it here. Feedback welcome.

I've removed Wait and SizeAll cursors as they don't look nice (Wait is not spinning and produces a broken rendering, SizeAll is a simple white cursor as you mentioned.) I think it would be more a disappointment/confusion or a source of bug reports to keep those.

It should also be more robust w.r.t. memory handling and reduce unwanted selector warnings.

@eliasdaler

eliasdaler commented May 10, 2018

Copy link
Copy Markdown
Contributor

Look at how SDL does it. Maybe we can do the same too?

https://github.com/spurious/SDL-mirror/blob/a4e95f81625b16d7454a7edb4010e7d37f72cebf/src/video/cocoa/SDL_cocoamouse.m#L136

I'm not sure about SDL_SYSTEM_CURSOR_SIZENWSE and SDL_SYSTEM_CURSOR_SIZENESW choice (as other resize* cursors work well for that!), but it's better to have closed hand as SizeAll then nothing.

@mantognini

Copy link
Copy Markdown
Member Author

but it's better to have closed hand as SizeAll then nothing.

I'd rather let the users decide on this themselves than impose an arbitrary and imperfect solution on them. ;-)

@JonnyPtn

Copy link
Copy Markdown
Contributor

Isn’t it an arbitrary and imperfect solution whether we used closed hand or leave it as normal arrow cursor?

I’d also say closed hand is better than unchanged

@mantognini

Copy link
Copy Markdown
Member Author

The current choice is between the closed hand (or any non optimal cursor) and returning false to explicitly let the user choose another cursor.

@JonnyPtn

Copy link
Copy Markdown
Contributor

Ah my mistake, returning false is more logical

@eliasdaler

Copy link
Copy Markdown
Contributor

Agreed.

@eXpl0it3r

eXpl0it3r commented Aug 9, 2018

Copy link
Copy Markdown
Member

Since this doesn't change any public-facing API or files, I've moved this from "feature" to "bugfix" and will include it in SFML 2.5.1.

@mantognini Let me know if I should rebase it or whether you want to do it. 🙂

@eXpl0it3r eXpl0it3r force-pushed the feature/more_cursors branch 2 times, most recently from c9c0ee6 to f6f6990 Compare August 13, 2018 20:40
@eXpl0it3r eXpl0it3r force-pushed the feature/more_cursors branch from f6f6990 to e043fc2 Compare August 27, 2018 19:31
JonnyPtn and others added 2 commits August 27, 2018 21:42
 - remove Wait and SizeAll cursors as they don't look nice
   (Wait is not spining and produces a broken rendering,
    SizeAll is a simple white cursor.)
 - fix memory management for NSCursor.
 - ignore selector warnings.
@eXpl0it3r eXpl0it3r force-pushed the feature/more_cursors branch from e043fc2 to aeca3dc Compare August 27, 2018 19:43
@eXpl0it3r

Copy link
Copy Markdown
Member

Merged in aeca3dc

@eXpl0it3r eXpl0it3r closed this Aug 27, 2018
@eXpl0it3r eXpl0it3r deleted the feature/more_cursors branch August 27, 2018 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants