close
Skip to content

Allow setting window icon with sf::Image#2417

Merged
ChrisThrasher merged 1 commit into
SFML:masterfrom
metaquarx:feature/seticon-image
Feb 21, 2023
Merged

Allow setting window icon with sf::Image#2417
ChrisThrasher merged 1 commit into
SFML:masterfrom
metaquarx:feature/seticon-image

Conversation

@metaquarx

Copy link
Copy Markdown
Contributor

Description

Currently, setting window icon is only allowed using a size & pixel pointer.

This overload wraps that, allowing users to set the window icon using an sf::Image directly.
sf::Image has the same 32bit RGBA pixel format as required by setIcon.
This is only implemented for sf::RenderWindow, as sf::Image is part of the graphics module.

Tasks

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

@codecov

codecov Bot commented Feb 16, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2417 (cc2a8ab) into master (41aa062) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2417      +/-   ##
==========================================
- Coverage   23.20%   23.18%   -0.02%     
==========================================
  Files         212      212              
  Lines       18059    18063       +4     
  Branches     4411     4412       +1     
==========================================
- Hits         4190     4188       -2     
+ Misses      13415    13149     -266     
- Partials      454      726     +272     
Impacted Files Coverage Δ
include/SFML/Graphics/RenderWindow.hpp 0.00% <ø> (ø)
src/SFML/Graphics/RenderWindow.cpp 0.00% <0.00%> (ø)
src/SFML/Network/Win32/SocketImpl.cpp 78.37% <0.00%> (-2.71%) ⬇️
src/SFML/Network/IpAddress.cpp 94.00% <0.00%> (-1.00%) ⬇️
src/SFML/Network/TcpSocket.cpp 43.39% <0.00%> (-0.63%) ⬇️
src/SFML/Audio/Music.cpp 0.00% <0.00%> (ø)
src/SFML/Audio/Sound.cpp 0.00% <0.00%> (ø)
src/SFML/Network/Ftp.cpp 0.00% <0.00%> (ø)
src/SFML/Network/Http.cpp 74.50% <0.00%> (ø)
src/SFML/Audio/ALCheck.cpp 0.00% <0.00%> (ø)
... and 50 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41aa062...cc2a8ab. Read the comment docs.

Comment thread include/SFML/Graphics/RenderWindow.hpp

@ChrisThrasher ChrisThrasher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can drop the sf:: namespace in a few places

Comment thread include/SFML/Graphics/RenderWindow.hpp Outdated
Comment thread src/SFML/Graphics/RenderWindow.cpp Outdated
@metaquarx metaquarx force-pushed the feature/seticon-image branch from 5343438 to e9d8448 Compare February 16, 2023 21:36
Comment thread include/SFML/Graphics/RenderWindow.hpp
@ChrisThrasher

Copy link
Copy Markdown
Member

Is there a particular use case that motivated you to add this?

@metaquarx

Copy link
Copy Markdown
Contributor Author

It's a somewhat common point of confusion, both on Discord & the forums. The advice is always to load it as an image, and pass in the size and pointer (as the PR does). There seems to be no particular reason for it not to be included in the first place.

I would assume the majority of users would be setting the icon using a loaded sf::Image, rather than using something like an embedded binary resource, due to its simplicity.

@metaquarx metaquarx force-pushed the feature/seticon-image branch from e9d8448 to 2529c22 Compare February 16, 2023 22:16
@ChrisThrasher

Copy link
Copy Markdown
Member

I'm open to this idea. I'm generally a fan of these little helpers that add convenience and make it easier to do functional programming where function return values can be directly passed to other functions. This change is in the same vein as the PRs that replaced (float, float) APIs with Vector2f which have aged well.

Comment thread src/SFML/Graphics/RenderWindow.cpp Outdated
@metaquarx metaquarx force-pushed the feature/seticon-image branch from 2529c22 to cc2a8ab Compare February 17, 2023 13:02
@Bambo-Borris

Copy link
Copy Markdown
Contributor

Love this as a convenience feature for users, I think basically all of my projects with SFML would have used this. It may be a small change, but it's a QoL one for sure.

@Bromeon Bromeon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Great addition!

@ChrisThrasher ChrisThrasher merged commit 474de4d into SFML:master Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants