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.

Sign up for GitHub

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

Jump to bottom

Slight increase performance and code refactor #1732

Merged
merged 1 commit into from Jan 5, 2021
Merged

Slight increase performance and code refactor #1732

merged 1 commit into from Jan 5, 2021

Conversation

GermanAizek
Copy link
Contributor

  • Has this change been discussed on the forum or in an issue before?
  • Does the code follow the SFML Code Style Guide?
  • Have you provided some example/test code for your changes?
  • If you have additional steps which need to be performed list them as tasks!

Tasks

  • Tested on Linux
  • Tested on Windows
  • Tested on macOS
  • Tested on iOS
  • Tested on Android

How to test this PR?

Performed a check with minimal build.

Bromeon
Bromeon requested changes Jan 3, 2021
View reviewed changes
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

Some of the changes are controversial due to micro-optimization vs. code clarity trade-off. In that case I tend to prefer clearer code, unless the optimization is proven to have an impact. I've commented some of these places.

src/SFML/Graphics/Font.cpp Outdated Show resolved Hide resolved
src/SFML/Graphics/Font.cpp Outdated Show resolved Hide resolved
src/SFML/Graphics/Text.cpp Outdated Show resolved Hide resolved
src/SFML/Window/GlContext.cpp Outdated Show resolved Hide resolved
src/SFML/Window/WindowImpl.cpp Show resolved Hide resolved
Bromeon
Bromeon approved these changes Jan 4, 2021
View reviewed changes
Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

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

Looks good to me, feedback from someone else?

@eXpl0it3r eXpl0it3r added this to Discussion in SFML 2.6.0 via automation Jan 4, 2021
@eXpl0it3r eXpl0it3r added this to the 2.6 milestone Jan 4, 2021
@eXpl0it3r eXpl0it3r moved this from Discussion to Ready in SFML 2.6.0 Jan 4, 2021
LaurentGomila
LaurentGomila requested changes Jan 4, 2021
View reviewed changes
src/SFML/Graphics/Font.cpp Outdated Show resolved Hide resolved
src/SFML/Window/Win32/WindowImplWin32.cpp Outdated Show resolved Hide resolved
@eXpl0it3r eXpl0it3r merged commit 9029118 into SFML:master Jan 5, 2021
SFML 2.6.0 automation moved this from Ready to Done Jan 5, 2021
@dogunbound dogunbound mentioned this pull request May 17, 2023
Merged
@ChrisThrasher ChrisThrasher mentioned this pull request Jun 23, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Bromeon Bromeon Bromeon approved these changes

@LaurentGomila LaurentGomila LaurentGomila approved these changes

@eXpl0it3r eXpl0it3r eXpl0it3r approved these changes

@binary1248 binary1248 binary1248 approved these changes

Assignees
No one assigned
Labels
feature m:sfml-audio m:sfml-network m:sfml-window
Projects
No open projects
SFML 2.6.0
  
Done
Milestone
2.6
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@GermanAizek @LaurentGomila @Bromeon @eXpl0it3r @binary1248