Skip to content
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

MacOS: When initializing an openGL view with an existing window, call finishInit #1760

Merged
merged 1 commit into from Mar 20, 2021

Conversation

jkeller51
Copy link
Contributor

@jkeller51 jkeller51 commented Mar 19, 2021

See issue #1759 for a description of the problem.

For DPI-scaling reasons, I think it is the sf::RenderWindow's "business" if the scaling factor changes, so my change signs it up to receive the same notifications on the window as it would if it had created the window.

  • Discussed in an issue
  • Does the code follow the SFML Code Style Guide?
  • Have you provided some example/test code for your changes?

Description

Please describe your pull request.

This PR is related to the issue #1759

Tasks

  • Tested on macOS

How to test this PR?

See below

@jkeller51
Copy link
Contributor Author

Code example:

#include <SFML/Window.hpp>
#include <SFML/Graphics.hpp>
#include <iostream>

#define USE_PARENT_WINDOW 0

int main() {
    
#if USE_PARENT_WINDOW
    sf::Window parentWindow(sf::VideoMode(600,400), "SFML Test");
    sf::RenderWindow window(parentWindow.getSystemHandle());
#else
    sf::RenderWindow window(sf::VideoMode(600,400), "SFML Test");
#endif
    
    while (window.isOpen()) {
        sf::Event event;
        while (window.pollEvent(event)) {
            switch(event.type) {
                case sf::Event::Closed:
                    window.close(); // will not get here when USE_PARENT_WINDOW==1
                    // *this is expected* because the parentWindow will (and should)
                    // receive this event and decide what to do with it, which in this
                    // case is nothing
                    break;
                case sf::Event::Resized:
                    std::cout << "Window resized to " << event.size.width << " x " <<
                        event.size.height << std::endl;
                    
                    break;
                default:
                    break;
            }
            window.clear(sf::Color::White);
            window.display();
        }
        
        
    }
    
    
    return 0;
}

Try flipping USE_PARENT_WINDOW and dragging the window between screens and the problem will become clear.

Using my change, the problem is solved.

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation Mar 19, 2021
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone Mar 19, 2021
@eXpl0it3r eXpl0it3r moved this from Discussion to Review & Testing in SFML 2.6.0 Mar 19, 2021
SFML 2.6.0 automation moved this from Review & Testing to Ready Mar 20, 2021
Copy link
Member

@eXpl0it3r eXpl0it3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparing it to the original implementation, this seems mostly an oversight to me.

Thanks for catching it and creating a PR! 🙂

@eXpl0it3r eXpl0it3r merged commit 788ac2e into SFML:master Mar 20, 2021
SFML 2.6.0 automation moved this from Ready to Done Mar 20, 2021
@eXpl0it3r
Copy link
Member

Would be great, if you could retest this. The change makes sense, but in my personal tests I was still unable to close the window even with the fix applied.

@jkeller51
Copy link
Contributor Author

You still won't be able to -- the window closing takes a different path. Clicking the close button calls the NSWindow's close methods, and the parentWindow in my example is the NSWindow which receives those function calls and decides how to respond. In this example, I have not handled events for the parentWindow so it will not close.

The resizing changes I am targeting are notifications that do not pay attention to hierarchy -- anybody can sign up to receive these notifications. Other notifications that will be fixed by this PR are related to focusing and unfocusing the window. We can probably find other solutions to address other window-specific events that will not be affected by this change like window closing, etc.

But it makes sense to me that parentWindow is responsible for handling window closing before anyone else. Technically when USE_PARENT_WINDOW=1 in my example, the sf::RenderTexture window is just an NSView inside of an NSWindow, so in that context it makes sense that it wouldn't handle window closing. But it might be useful for the sf::RenderWindow to still receive a notification just before the window closes. I would propose a separate issue for that though.

@eXpl0it3r
Copy link
Member

Ohhh, I read the comment in the code wrong. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
SFML 2.6.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants