Skip to content

icon: refactor Icon to be dyn#4182

Merged
kchibisov merged 2 commits intomasterfrom
kchibisov/dyn-icon
Apr 20, 2025
Merged

icon: refactor Icon to be dyn#4182
kchibisov merged 2 commits intomasterfrom
kchibisov/dyn-icon

Conversation

@kchibisov
Copy link
Copy Markdown
Member

@kchibisov kchibisov commented Mar 23, 2025

Same as for CustomCursor. However, the API uses dyn stuff only because of Windows backend at the time of writing, generally, platforms should just have a separate method that deals with all of that, and e.g. top-level winit only guarantees Rgba.

Part of #3433.

  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality

@kchibisov kchibisov force-pushed the kchibisov/dyn-icon branch 3 times, most recently from 8c35137 to d612131 Compare March 23, 2025 18:06
Copy link
Copy Markdown
Member

@madsmtm madsmtm 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 overall, fine with cleaning this up properly later

@madsmtm madsmtm added the S - api Design and usability label Mar 27, 2025
@kchibisov kchibisov force-pushed the kchibisov/dyn-icon branch 2 times, most recently from 8a34247 to d5678e0 Compare April 6, 2025 06:44
@kchibisov kchibisov requested a review from madsmtm April 6, 2025 06:46
@kchibisov kchibisov force-pushed the kchibisov/dyn-icon branch 2 times, most recently from 3fbacec to 4f32416 Compare April 6, 2025 07:56
Same as for `CustomCursor`. However, the API uses `dyn` stuff only
because of `Windows` backend at the time of writing, generally, platforms
should just have a separate method that deals with all of that, and e.g.
top-level winit only guarantees `Rgba`.
@kchibisov kchibisov force-pushed the kchibisov/dyn-icon branch from 4f32416 to 67dbd9d Compare April 15, 2025 07:58
@kchibisov
Copy link
Copy Markdown
Member Author

@madsmtm any chance to give one more look?

@kchibisov kchibisov merged commit ed4d70f into master Apr 20, 2025
57 checks passed
@kchibisov kchibisov deleted the kchibisov/dyn-icon branch April 20, 2025 01:48
@madsmtm
Copy link
Copy Markdown
Member

madsmtm commented Apr 20, 2025

Sorry, missed the ping. But it looks fine to me.

@madsmtm madsmtm mentioned this pull request May 15, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S - api Design and usability

Development

Successfully merging this pull request may close these issues.

2 participants