Skip to content
This repository has been archived by the owner on Jul 10, 2023. It is now read-only.

window: Add support for setting the app icon on the Mac. #86

Merged
merged 1 commit into from
May 13, 2016

Conversation

pcwalton
Copy link

@pcwalton pcwalton commented May 11, 2016

Requires servo/cocoa-rs#124.

r? @paulrouget


This change is Reviewable

@mbrubeck
Copy link

Should this go upstream first?

@pcwalton
Copy link
Author

I'd rather get it reviewed here first.

pcwalton added a commit to pcwalton/servo that referenced this pull request May 11, 2016
This makes the app easier to pick out in Instruments.app and so forth.

Requires servo/glutin#86, which itself requires servo/cocoa-rs#124.
/// If present, this path must reference a PNG file.
///
/// The default is `None`.
pub icon: Option<PathBuf>,
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this belongs in PlatformSpecificWindowBuilderAttributes instead.

Copy link
Author

Choose a reason for hiding this comment

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

Well, I was thinking other platforms (e.g. Linux, Windows) could use it eventually.

Copy link
Member

@emilio emilio May 11, 2016

Choose a reason for hiding this comment

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

Humm... Yeah, I agree this should be useful for other platforms but, for example, in the case of Linux, to set the icon you need to pass a buffer of pixels (see the _NET_WM_ICON spec), which with this API would require Glutin to integrate a PNG decoder.

Wouldn't be such a problem for servo if glutin would use our same png decoder of course, and could not be so out of scope for glutin (I guess any serious app or game at some point would want this feature, and having to specify just a file would be really convenient), but...

Copy link
Author

Choose a reason for hiding this comment

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

I think it'd be fine to have Glutin have an optional dependency on a PNG decoder.

@paulrouget
Copy link

r=me with cocoa-rs update.

@emilio
Copy link
Member

emilio commented May 12, 2016

FWIW I added support for changing the icon on X11 on top of this here, feel free to cherry-pick it, or I'll make another PR afterwards if not :P

screenshot from 2016-05-12 15-14-08

@pcwalton
Copy link
Author

@bors-servo: r=paulrouget

@bors-servo
Copy link

📌 Commit bac84fc has been approved by paulrouget

@bors-servo
Copy link

⌛ Testing commit bac84fc with merge 5a9ff10...

bors-servo pushed a commit that referenced this pull request May 13, 2016
window: Add support for setting the app icon on the Mac.

Requires servo/cocoa-rs#124.

r? @paulrouget

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/glutin/86)
<!-- Reviewable:end -->
@bors-servo
Copy link

☀️ Test successful - travis

@bors-servo bors-servo merged commit bac84fc into servo:servo May 13, 2016
bors-servo pushed a commit that referenced this pull request May 17, 2016
 	cocoa: Add some bare-bones menus on the Mac to conform better to the Apple Human Interface Guidelines.

Includes #86.

Requires servo/cocoa-rs#125.

r? @paulrouget

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/glutin/88)
<!-- Reviewable:end -->
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants