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

Only spawn Resized event when window size changes. #878

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@TankOs
Member

TankOs commented May 8, 2015

No description provided.

@kimci86

This comment has been minimized.

Show comment
Hide comment
@kimci86

kimci86 May 8, 2015

Contributor

I just tested it.
Now, moving the window does not push Resized events.

However, some problems remain: one event is pushed on window creation (initializing properly m_previousSize could solve it) and two events are pushed when the window is maximized.
Only one is pushed when the window is restored though, which is good.

Contributor

kimci86 commented May 8, 2015

I just tested it.
Now, moving the window does not push Resized events.

However, some problems remain: one event is pushed on window creation (initializing properly m_previousSize could solve it) and two events are pushed when the window is maximized.
Only one is pushed when the window is restored though, which is good.

@TankOs TankOs self-assigned this May 8, 2015

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs May 8, 2015

Member

one event is pushed on window creation

I don't really consider this to be a bug.

two events are pushed when the window is maximized

Thanks, will look into it.

Member

TankOs commented May 8, 2015

one event is pushed on window creation

I don't really consider this to be a bug.

two events are pushed when the window is maximized

Thanks, will look into it.

@TankOs TankOs added the s:undecided label May 8, 2015

@binary1248 binary1248 added this to the 2.3.1 milestone May 11, 2015

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs May 12, 2015

Member

I talked with @kimci86 at IRC about this. At some place he got indeed multiple Resized events, but in a way that can't be avoided: kimci's window manager reported size changes, and that's what the SFML event is for. ;-)

Keep in mind that manually altering the window might often result in 2 events, as the window manager takes over and adjusts the size, so that the window decoration fits etc.

Member

TankOs commented May 12, 2015

I talked with @kimci86 at IRC about this. At some place he got indeed multiple Resized events, but in a way that can't be avoided: kimci's window manager reported size changes, and that's what the SFML event is for. ;-)

Keep in mind that manually altering the window might often result in 2 events, as the window manager takes over and adjusts the size, so that the window decoration fits etc.

@TankOs TankOs added s:accepted and removed s:undecided labels May 12, 2015

@TankOs

This comment has been minimized.

Show comment
Hide comment
@TankOs

TankOs May 12, 2015

Member

Feel free to merge 👍 🐈

Member

TankOs commented May 12, 2015

Feel free to merge 👍 🐈

@mantognini

This comment has been minimized.

Show comment
Hide comment
@mantognini

mantognini May 12, 2015

Member

BTW, some time ago, we discussed a similar issue. The conclusion was to let the user deal with multiple events. Now, sure, it's appreciated not to fire 1000 events in the face of the user at once. But it's alright to have a few dups.

Member

mantognini commented May 12, 2015

BTW, some time ago, we discussed a similar issue. The conclusion was to let the user deal with multiple events. Now, sure, it's appreciated not to fire 1000 events in the face of the user at once. But it's alright to have a few dups.

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 12, 2015

Member

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

Member

eXpl0it3r commented May 12, 2015

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

@eXpl0it3r

This comment has been minimized.

Show comment
Hide comment
@eXpl0it3r

eXpl0it3r May 22, 2015

Member

Merged in 608b4fb on branch 2.3.x

Member

eXpl0it3r commented May 22, 2015

Merged in 608b4fb on branch 2.3.x

@eXpl0it3r eXpl0it3r closed this May 22, 2015

@eXpl0it3r eXpl0it3r deleted the bugfix/unix_window_resize branch May 22, 2015

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