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

Fixed an issue in InputImpl::getSFOpenGLViewFromSFMLWindow (see issue #782) #792

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@MoustacheFarmer

MoustacheFarmer commented Feb 3, 2015

When a window is created with fullscreen mode (see SFWindowController::setupFullscreenViewTithMode) a NSView called masterview is created with the SFLOpenGLView added as subview of this NSView. This NSView is then set as the window's contentview.

This means that whenever InputImp::getSFOpenGLViewFromSFMLWindow is called during fulllscreen mode it'll try to get the SFOpenGLView from the contentview which fails because it's a regular NSView, not a SFOpenGLView. This fix attempts to get the SFOpenGLView from the contentview's subviews instead of the contentview itself.

Thom Robinson (Macbook Pro) Thom Robinson (Macbook Pro)
Fixed an issue in InputImpl::getSFOpenGLViewFromSFMLWindow failing to…
… retrieve the SFOpenGLView from the contentview's subview when using fullscreen (see issue #782).
@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Feb 4, 2015

Member

Thanks @MoustacheFarmer.

This supersedes #782. Tested and approved. Ready for review by others.

Member

mantognini commented Feb 4, 2015

Thanks @MoustacheFarmer.

This supersedes #782. Tested and approved. Ready for review by others.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Feb 10, 2015

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Feb 10, 2015

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

@MoustacheFarmer

This comment has been minimized.

Show comment
Hide comment
@MoustacheFarmer

MoustacheFarmer Feb 11, 2015

I think the if([view isKindOfClass:[NSView class]]) statement I put in is rather redundant so that can be removed (it will always return true). What I'd like to do is check if the window is fullscreen and if that's the case check the view's subviews for an SFOpenGLView instead of doing it after failing to get the SFOpenGLView directly from the view itself. I'm not quite familiar with the SFML internals so I'm having a hard time figuring out how to check if the window is running in fullscreen mode. If anyone has any ideas then please share them.

MoustacheFarmer commented Feb 11, 2015

I think the if([view isKindOfClass:[NSView class]]) statement I put in is rather redundant so that can be removed (it will always return true). What I'd like to do is check if the window is fullscreen and if that's the case check the view's subviews for an SFOpenGLView instead of doing it after failing to get the SFOpenGLView directly from the view itself. I'm not quite familiar with the SFML internals so I'm having a hard time figuring out how to check if the window is running in fullscreen mode. If anyone has any ideas then please share them.

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Feb 11, 2015

Member

It might be refactored in the future but for now I think for now it's relatively good and plays nice with the rest of the implementation.

Testing for fullscreen might not be a good idea: I didn't check carefully but it might be problematic when creating a sf::Window from a NSView.

Member

mantognini commented Feb 11, 2015

It might be refactored in the future but for now I think for now it's relatively good and plays nice with the rest of the implementation.

Testing for fullscreen might not be a good idea: I didn't check carefully but it might be problematic when creating a sf::Window from a NSView.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Feb 20, 2015

Member

So can this be merged?

Member

eXpl0it3r commented Feb 20, 2015

So can this be merged?

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini Feb 21, 2015

Member

I think so.

Member

mantognini commented Feb 21, 2015

I think so.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Feb 21, 2015

Member

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

Member

eXpl0it3r commented Feb 21, 2015

This PR has been added to my merge list, meaning it will be merged soon, unless someone raises any concerns.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r Feb 23, 2015

Member

Merged in d83ddd5

Member

eXpl0it3r commented Feb 23, 2015

Merged in d83ddd5

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