Implement many of the core traits for immutable and mutable refs

[BroderickCarlin]

Thank you for helping out with embedded-graphics development! Please:

• Check that you've added passing tests and documentation
• Add a CHANGELOG.md entry in the Unreleased section under the appropriate heading (Added, Fixed, etc) if your changes affect the public API
• Run rustfmt on the project
• Run just build (Linux/macOS only) and make sure it passes. If you use Windows, check that CI passes once you've opened the PR.

PR description

This PR implements many of the core traits of e-g for both immutable and mutable refs. This change is done primarily to aid ergonomics.

These changes were split off from #735

[rfuest]

IMO the most important traits to have implemented for references, if you want to use it in higher level crates, would be Drawable and Dimensions. Not being able to implement Dimensions for &T limits the usefulness of this change.

The impl<T> Dimensions for T where T: OriginDimensions, which makes it impossible to implement Dimensions for &T, has been problematic before and made us unable to refactor some code. I don't remember the details, but this could also explain my hesitation in #735 😉. Perhaps this is a sign that we should remove OriginDimensions in the next release with breaking changes. Unfortunately OriginDimensions is in e-g-core and breaking e-g-core would break all dependent creates again.

[BroderickCarlin]

IMO the most important traits to have implemented for references, if you want to use it in higher level crates, would be Drawable and Dimensions

I strongly agree the most value comes from these 2 traits - one of which (Drawable) is included in this PR. Dimensions, for reasons outlined numerous times across numerous PRs, is not included.

Not being able to implement Dimensions for &T limits the usefulness of this change.

I agree with this to an extent. I think the changes in this PR still carry significant value in improving ergonomics and flexibility. As mentioned in #735 , wrapper types that wrap shared references is an immediate place where improvement would be noticed. I do also agree, however, that not supporting Dimensions for &T is a very rough edge at the moment.

These changes are also highly related with the PR and associated discussion within #706 - wanted to just briefly mention this as the conversation there is highly relevant and includes valuable discussion.

One observation of mine (which, admittedly may be entire inaccurate) is that there seems to be some conflation of relation between Dimensions and OriginDimensions as a result of their naming. These two traits look to be conveying similar, but very different pieces of information - with Dimensions conveying information about the position and size of an element within global space and OriginDimensions conveys information about the elements self-reported, or desired, size.

Of course, it would be a more significant change, but it seems that what e-g is attempting to accomplish with Dimensions and OriginDimensions could be distilled down to a set of traits:

• Dimensions that just has a single size(&self) -> Size method
• Position that has a single method along the lines of position(&self) -> Point
• BoundingBox has a single method bounding_box(&self) -> Rectangle and can be derived for all types that implement both Dimensions and Positioned
  • this may also benefit from being a sealed trait
  • this could expand to provide other methods, such as a bottom_right()

I realize this is potentially getting a bit off track of what this PR is proposing, and this conversation appears pretty fragmented at the moment. It does seem that this is an issue that has been reoccurring and is blocking larger work efforts so I would suggest that we look into opening a PR specifically to explore what this would look like, and which can house all the conversation

[rfuest]

I agree with this to an extent. I think the changes in this PR still carry significant value in improving ergonomics and flexibility. As mentioned in #735 , wrapper types that wrap shared references is an immediate place where improvement would be noticed. I do also agree, however, that not supporting Dimensions for &T is a very rough edge at the moment.

I didn't want to imply that the changes in this PR wouldn't be useful without the Dimensions impl for &T. But that even the example you provided #735 did use the Dimensions trait showed me that it would be better to fix the OriginDimensions problem to get the full benefits of this change.

These changes are also highly related with the PR and associated discussion within #706 - wanted to just briefly mention this as the conversation there is highly relevant and includes valuable discussion.

Thanks for linking this, I did vaguely remember this discussion but couldn't remember where it was.

One observation of mine (which, admittedly may be entire inaccurate) is that there seems to be some conflation of relation between Dimensions and OriginDimensions as a result of their naming. These two traits look to be conveying similar, but very different pieces of information - with Dimensions conveying information about the position and size of an element within global space and OriginDimensions conveys information about the elements self-reported, or desired, size.

It's not entirely accurate. OriginDimensions does also specify the position, but the position is always (0, 0). The intention of the trait was to have a type level guarantee that the object is located at (0, 0). But I don't think we need this at type level.

Of course, it would be a more significant change, but it seems that what e-g is attempting to accomplish with Dimensions and OriginDimensions could be distilled down to a set of traits:

* `Dimensions` that just has a single `size(&self) -> Size` method

* `Position` that has a single method along the lines of `position(&self) -> Point`

* `BoundingBox` has a single method `bounding_box(&self) -> Rectangle` and can be derived for all types that implement both `Dimensions` and `Positioned`
  
  * this may also benefit from being a sealed trait
  * this could expand to provide other methods, such as a `bottom_right()`

I realize this is potentially getting a bit off track of what this PR is proposing, and this conversation appears pretty fragmented at the moment. It does seem that this is an issue that has been reoccurring and is blocking larger work efforts so I would suggest that we look into opening a PR specifically to explore what this would look like, and which can house all the conversation

While I kind of like the idea of this split into multiple traits I'm not sure it it would beneficial in practice. The only thing I can think of at the moment, which would have a size and not position, would be a ImageDrawable. For that trait the new Dimensions trait, that only specifies the size, would be better than the OriginDimensions trait, because an ImageDrawable doesn't have a position. The position of an ImageDrawable is instead specified by wrapping it in an Image. But if this is the only case it would be easier to just add a size() method to the ImageDrawable trait.

[BroderickCarlin]

I was also going to bring up the point of external utility of the traits such that from the perspective of an external developer how might I utilize these existing, or new trait methods. On the surface it seems convenient to be able to just retrieve the .size() or .position() of a drawable graphical element, but I do actually wonder if these are ever useful pieces of information without the context of the other - thus in practice you'd always fall back to utilizing .bounding_box(). On the contrary, any non-positioned graphical primitive would benefit from providing just a .size() and allow it's position to be provided through a wrapper type - as is done with images, today.

if this is the only case it would be easier to just add a size() method to the ImageDrawable trait.

My initial reaction to this is that it may unintentionally over-constrain future types that may also want to convey a size but not have position. An example of this may be a non-mono sized font that may want to convey information about the size of each glyph. Sprite sheets may be another instance where it would be beneficial to have graphical information with a size but not a position. I admit, I'm unsure if these are valid arguments but I do think it something to consider.

But I don't think we need this at type level.

I would tend to agree with this. If it is determined to be desirable, however, I think other approaches could be taken to achieve this result without the constraints of the current design

[rfuest]

I've created a draft PR #737 that removes the OriginDimensions trait.

[rfuest]

My initial reaction to this is that it may unintentionally over-constrain future types that may also want to convey a size but not have position. An example of this may be a non-mono sized font that may want to convey information about the size of each glyph. Sprite sheets may be another instance where it would be beneficial to have graphical information with a size but not a position. I admit, I'm unsure if these are valid arguments but I do think it something to consider.

I can't see a use case for having a common trait to get the size at the moment. What would the benefit of having a common trait to determine a glyph size and the size of a sprite sheet be, if there is no common API to draw them or do anything else with them?

[BroderickCarlin]

What would the benefit of having a common trait to determine a glyph size and the size of a sprite sheet be, if there is no common API to draw them or do anything else with them?

If the API extended to be able to convey the size of non-drawables, such as the display itself, it could be utilized for layout purposes.

I admit I have been wavering over this after thinking about it, though. This is obviously a much larger fundamental change and I don't believe that the experimentation or discussion around such a change should impede the more immediate change outlined in these few PRs

[rfuest]

If the API extended to be able to convey the size of non-drawables, such as the display itself, it could be utilized for layout purposes.

The display size is already defined by the Dimensions trait, which is required by the DrawTarget trait.

[BroderickCarlin]

The display size is already defined by the Dimensions trait, which is required by the DrawTarget trait.

The usage of the Dimensions trait in this case implies that a DrawTarget has a position. Maybe this is where my understanding is faltering, but I've been under the impression that if I create a Rectangle at the origin that it will always draw in the top-left of the DrawTarget. DrawTarget having a position seems to imply that this is not actually always the case - and instead the DrawTarget is also abstracted away from the more abstract idea of the global canvas coordinate system. I'm not seeing clarity within the docs, but I do recall a brief mention of this in a past PR or issue that I now can't seem to locate. If it is true that "a Rectangle at the origin will not always draw in the top left corner of a DrawTarget" then I would agree that a specific Size-esque trait is unnecessary and provides minimal, to no, value

[rfuest]

The usage of the Dimensions trait in this case implies that a DrawTarget has a position.

DrawTargets have a position. You can use a translated coordinate system via https://docs.rs/embedded-graphics/latest/embedded_graphics/draw_target/trait.DrawTargetExt.html#tymethod.translated, in which case Dimensions wouldn't return a rectangle with top_left == Point::zero() anymore.

[BroderickCarlin]

DrawTargets have a position. You can use a translated coordinate system via...

Not sure why I completely blanked on that - yeah, with this clarity I don't see much benefit in breaking up into pointed traits at this time. I think just eliminating OriginDimensions is a major step in the right direction