Rewrite the conversion traits to work more like From and Into #176

Ogeon · 2020-03-28T19:04:34Z

This replaces the old FromColor and IntoColor traits, as well as the implementations of From and Into between color types, with new traits that are more flexible. The new FromColor and IntoColor traits will also clamp the output to prevent out-of-bounds results.

It's a very breaking change. To migrate from the old system, any failing uses from and into should be replaced with from_color and into_color. Any custom color that used to implement IntoColor should switch to implementing FromColorUnclamped.

A new trait, called Transparency, has also been added to make it possible to implement FromColorUnclamped for Alpha without running into conflicting implementations. It makes it possible to convert from any color type that implements Transparency, and not just Alpha<A, T> to Alpha<B, T>, for example.

The semantics of the new traits are a bit different, so it may affect type inference. It may also be necessary to convert using more than one step. This is mostly for the RGB family and Luma, when changing RGB space or standard.

The syntax for the helper attributes were also changed to the more common #[palette(...)] pattern. It helped when implementing them and looks a bit nicer, IMO.

Closes #41, fixes #111.

Ogeon · 2020-03-28T19:08:02Z

It's finally here, after all this time. I don't know how many times I rewrote the derives and how many versions of the traits I went through, but I'm quite happy with the current result. I was also helped a lot by more recent language features, such as not having to implement Into* for the "foreign" types.

I'm leaving it up for a while, in case anyone want to help reviewing. I may very well have forgotten something along the way.

okaneco · 2020-03-30T01:09:08Z

palette/src/alpha/mod.rs

+    type WithAlpha: Transparency<A, Color = Self::Color, WithAlpha = Self::WithAlpha>;
+
+    /// Transforms the color into a transparent color with the provided
+    /// alpha value. If `Self` already has a transparency, that one is


Suggested change

/// alpha value. If `Self` already has a transparency, that one is

/// alpha value. If `Self` already has a transparency, that value is

I think the instances of "that one" in this doc section would be better as "that value" or "that field". I try to generally avoid pronouns and vague words like "it" and "one" in tech writing when possible.

okaneco · 2020-03-30T01:09:12Z

palette/src/alpha/mod.rs

+/// color type. The type itself doesn't need to store the transparency
+/// value, as long as it can be transformed into or wrapped in a type that
+/// does have a representation of transparency. This would typically be done
+/// by wrapping it in an [`Alpha`](struct.Alpha.html) instance.


/// `Transparency` is an interface for adding, removing and setting the alpha /// channel of a color type. The type itself doesn't need to store the /// transparency value as it can be transformed into or wrapped in a type that /// has a representation of transparency. This would typically be done by /// wrapping it in an [`Alpha`](struct.Alpha.html) instance.

okaneco · 2020-03-30T01:09:19Z

palette/src/alpha/mod.rs

+    fn with_alpha(self, alpha: A) -> Self::WithAlpha;
+
+    /// Removes the transparency from the color. If `Self::Color` has
+    /// an internal transparency field, that one should be set to


Suggested change

/// an internal transparency field, that one should be set to

/// an internal transparency field, that field will be set to

similar change in L119

okaneco · 2020-03-30T01:09:25Z

palette/src/alpha/mod.rs

+    /// ```
+    fn without_alpha(self) -> Self::Color;
+
+    /// Removes the color into separate color and transparency values.


Suggested change

/// Removes the color into separate color and transparency values.

/// Splits the color into separate color and transparency values.

okaneco · 2020-03-30T01:16:20Z

palette/src/convert.rs

+//! * `rgb_space = "some::rgb_space::Type"`: Sets the RGB space
+//! type that should be used when deriving. The default is to either use `Srgb`
+//! or a best effort to convert between spaces, so sometimes it has to be set
+//! to a specific type. This does also accept type parameters.


Suggested change

//! to a specific type. This does also accept type parameters.

//! to a specific type. This also accepts type parameters.

okaneco · 2020-03-30T01:18:48Z

palette/src/convert.rs

+/// such as allowing implementing `FromColor<Rgb<S2, T>> for Rgb<S1, T>`.
+/// * Implementing `FromColorUnclamped` and `Limited` is enough to get all the other conversion
+/// traits, while `From` and `Into` would not be possible to blanket implement in the same way.
+/// This does also reduce the work that need to be done by macros.


Suggested change

/// This does also reduce the work that need to be done by macros.

/// This also reduces the work that needs to be done by macros.

okaneco · 2020-03-30T01:19:52Z

palette/src/convert.rs

+/// See the [`convert`](index.html) module for how to implement `FromColorUnclamped` for
+/// custom colors.
+pub trait FromColor<T>: Sized {
+    ///Convert from T with values clamped to the color defined bounds.


Suggested change

///Convert from T with values clamped to the color defined bounds.

/// Convert from T with values clamped to the color defined bounds.

okaneco · 2020-03-30T01:20:25Z

palette/src/convert.rs

+/// See the [`convert`](index.html) module for how to implement `FromColorUnclamped` for
+/// custom colors.
+pub trait FromColorUnclamped<T>: Sized {
+    ///Convert from T. The resulting color might be invalid in its color space.


Suggested change

///Convert from T. The resulting color might be invalid in its color space.

/// Convert from T. The resulting color might be invalid in its color space.

okaneco · 2020-03-30T01:30:23Z

This looks great 🎉

I don't have much to add other than small doc suggestions.

Since the file's being modified, the README's description section up top can probably be shortened to something like A Rust library that makes linear color calculations and color space conversion easy and accessible for anyone. since there's not a general color type anymore?

Ogeon · 2020-03-30T10:45:05Z

Thank you for taking a look at this! Those are good suggestions and feedback. I value any improvements to my writing, especially since English isn't my first language. I'll implement the suggestions and a couple of other improvements. I somehow forgot that "lossless" is a word.

Ogeon · 2020-04-04T10:43:53Z

I applied the suggested documentation changes (and a few more) and rebased to get the latest changes from master.

Ogeon · 2020-04-05T10:55:33Z

I decided to rename the Transparency to WithAlpha to keep the terminology more consistent. I was on the fence at first, but everything in and around it uses "alpha", so in the end I thought it would be weirder not to. This covers the last thing I wanted to change before squash+merge.

Ogeon · 2020-04-05T11:09:13Z

bors r+

bors · 2020-04-05T11:32:17Z

Build succeeded

continuous-integration/travis-ci/push

okaneco reviewed Mar 30, 2020

View reviewed changes

clarkmoody mentioned this pull request Apr 2, 2020

Color Enhancements iced-rs/iced#200

Merged

4 tasks

Ogeon force-pushed the conversion_traits branch from ab927db to 9084414 Compare April 4, 2020 10:42

Rewrite the conversion traits to work more like From and Into

3fe3039

Ogeon force-pushed the conversion_traits branch from b7b45e0 to 3fe3039 Compare April 5, 2020 11:08

bors bot merged commit dd7645d into master Apr 5, 2020

bors bot deleted the conversion_traits branch April 5, 2020 11:32

NicolasKlenert mentioned this pull request Mar 17, 2021

Conversion between colors with ::from not implemented if local crate is used #204

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite the conversion traits to work more like From and Into #176

Rewrite the conversion traits to work more like From and Into #176

Ogeon commented Mar 28, 2020 •

edited

Ogeon commented Mar 28, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco Mar 30, 2020

okaneco commented Mar 30, 2020

Ogeon commented Mar 30, 2020

Ogeon commented Apr 4, 2020 •

edited

Ogeon commented Apr 5, 2020

Ogeon commented Apr 5, 2020

bors bot commented Apr 5, 2020

	/// alpha value. If `Self` already has a transparency, that one is
	/// alpha value. If `Self` already has a transparency, that value is

	/// an internal transparency field, that one should be set to
	/// an internal transparency field, that field will be set to

	/// Removes the color into separate color and transparency values.
	/// Splits the color into separate color and transparency values.

	//! to a specific type. This does also accept type parameters.
	//! to a specific type. This also accepts type parameters.

	/// This does also reduce the work that need to be done by macros.
	/// This also reduces the work that needs to be done by macros.

	///Convert from T with values clamped to the color defined bounds.
	/// Convert from T with values clamped to the color defined bounds.

	///Convert from T. The resulting color might be invalid in its color space.
	/// Convert from T. The resulting color might be invalid in its color space.

Rewrite the conversion traits to work more like From and Into #176

Rewrite the conversion traits to work more like From and Into #176

Conversation

Ogeon commented Mar 28, 2020 • edited

Ogeon commented Mar 28, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

okaneco commented Mar 30, 2020

Ogeon commented Mar 30, 2020

Ogeon commented Apr 4, 2020 • edited

Ogeon commented Apr 5, 2020

Ogeon commented Apr 5, 2020

bors bot commented Apr 5, 2020

Build succeeded

Ogeon commented Mar 28, 2020 •

edited

Ogeon commented Apr 4, 2020 •

edited