New issue

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

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

Already on GitHub? Sign in to your account

Improve OS X Implementation: performance and events handling #617

Merged
merged 3 commits into from Jun 4, 2014

Conversation

Projects
None yet
2 participants
@mantognini
Member

mantognini commented May 26, 2014

What this does:

  • Optimised OS X implementation regarding scaling factor:
    • before: WindowImplCocoa's scaling function took ~20% of running time on a benchmark,
    • now: it takes << 1%.
  • The scaling factor is adapted when the window is moved to another screen with another scaling factor.
    • Well, I hope so. I have no external monitor to test this...
  • Fixed (un)focus events sent to all windows instead of only one.
  • Fixed sf::Mouse::(get|set)Position functions.

mantognini added some commits May 24, 2014

Optimised OS X implementation regarding scaling factor
This also adds support for changing the screen profile or moving the window to another screen.
Improved OS X implementation
It makes sure the notifications sent to SFOpenGLView are only from its window.

@mantognini mantognini added this to the 2.2 milestone May 26, 2014

@mantognini mantognini self-assigned this May 26, 2014

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 26, 2014

Member

Need someone with OS X knowledge to review this.

Member

eXpl0it3r commented May 26, 2014

Need someone with OS X knowledge to review this.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 28, 2014

Member

Just merging it feels a bit uncomfortable, but I guess unless someone with OS X knowledge gets to review it, we don't have another option, besides it seems to have worked fine in the past. 😉

Member

eXpl0it3r commented May 28, 2014

Just merging it feels a bit uncomfortable, but I guess unless someone with OS X knowledge gets to review it, we don't have another option, besides it seems to have worked fine in the past. 😉

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini May 29, 2014

Member

Yes, it's not an optimal reviewing process if no one else in the team can review it...

I'm pretty confident it will work but just in case let's wait a few more days; maybe someone from the community has something to say regarding the code or simply a typo in a comment.

Member

mantognini commented May 29, 2014

Yes, it's not an optimal reviewing process if no one else in the team can review it...

I'm pretty confident it will work but just in case let's wait a few more days; maybe someone from the community has something to say regarding the code or simply a typo in a comment.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini May 31, 2014

Member

I suggest we (I mean, you the directors ;-) merge it on Monday (or later at your convenience) so people would have had one full week to comment on it.

Member

mantognini commented May 31, 2014

I suggest we (I mean, you the directors ;-) merge it on Monday (or later at your convenience) so people would have had one full week to comment on it.

Fixed OS X implementation of sf::Mouse::(get|set)Position
The code was not updated at all when support for retina display was introduced.
@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Jun 1, 2014

Member

I realized that I forgot to update sf::Mouse::(get|set)Position a few weeks back... Now that's done.

Member

mantognini commented Jun 1, 2014

I realized that I forgot to update sf::Mouse::(get|set)Position a few weeks back... Now that's done.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Jun 4, 2014

Member

If you don't have any last minute concerns, I'm going to merge this.

Member

eXpl0it3r commented Jun 4, 2014

If you don't have any last minute concerns, I'm going to merge this.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Jun 4, 2014

Member

Please, do. :-)

Member

mantognini commented Jun 4, 2014

Please, do. :-)

@eXpl0it3r eXpl0it3r merged commit 46be215 into master Jun 4, 2014

@eXpl0it3r eXpl0it3r deleted the bugfix/osx-implementation branch Jun 4, 2014

@eXpl0it3r eXpl0it3r added the resolved label Jun 4, 2014

@mantognini mantognini removed their assignment Apr 30, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment