Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Optionally support serde. #61

Merged
merged 1 commit into from
Nov 24, 2024
Merged

Optionally support serde. #61

merged 1 commit into from
Nov 24, 2024

Conversation

waywardmonkeys
Copy link
Collaborator

This adds support for serde::Deserialize and serde::Serialize using serde-derive for AlphaColor, OpaqueColor, PremulColor, Rgba8.

This does not provide it for DynamicColor to avoid dealing with the non-exhaustive ColorSpaceTag.

It uses derive rather than hand-implementing the traits as the latter is very verbose and most things using serde will already have enabled the derive feature. (This is also true for the other Linebender crates.)

@waywardmonkeys
Copy link
Collaborator Author

The fact that this uses the derive feature for serde doesn't change the decision to not use it for bytemuck.

In particular, crates like resvg, tiny-skia, and those from Makepad do use bytemuck, don't use serde and very much do not want a dependency on the whole proc macro world that serde-derive builds on.

This adds support for `serde::Deserialize` and `serde::Serialize` using
`serde-derive` for `AlphaColor`, `OpaqueColor`, `PremulColor`, `Rgba8`.

This does not provide it for `DynamicColor` to avoid dealing with
the non-exhaustive `ColorSpaceTag`.

It uses `derive` rather than hand-implementing the traits as the
latter is very verbose and most things using `serde` will already
have enabled the `derive` feature. (This is also true for the
other Linebender crates.)
Copy link
Contributor

@raphlinus raphlinus left a comment

Choose a reason for hiding this comment

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

Looks straightforward, and I agree with the decision not to hand-derive. People who use serde are opting in to that whole thing.

My personal feeling is that if we're annotating these types with serde, then DynamicColor belongs also, but I understand the concern about non-exhaustive and can defer that discussion to later.

@waywardmonkeys waywardmonkeys merged commit cb11a56 into main Nov 24, 2024
15 checks passed
@waywardmonkeys waywardmonkeys deleted the derive-serde branch November 24, 2024 04:34
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants