FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

[color] Very generic colors: Rgb, Hsv and Gray by TomSaw · Pull Request #781 · modm-io/modm · GitHub

/ modm Public

[color] Very generic colors: Rgb, Hsv and Gray - #781

Open
TomSaw wants to merge 2 commits into
modm-io:developfrom
TomSaw:feature/very-generic-colors
ConversationCommits (2)ChecksFiles changed
Open

[color] Very generic colors: Rgb, Hsv and Gray#781
TomSaw wants to merge 2 commits into
modm-io:developfrom
TomSaw:feature/very-generic-colors

Conversation

TomSaw commented Nov 15, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

These Colors may be used on their own. F.e. passed to a colored LED driver or being emitted by color-sensor drivers.

They also represent single pixels for modm::graphic::Buffer and modm::graphic::Display (Work in progress, see feature/rewrite-graphic

Features

  • Bit granularity for color::Gray. Same for color::Rgb and color::Hsv cause they're made from 3x color::Gray
    • Proportional conversion between grays with different digits. 2-bit Gray ---> 4-bit Gray f.e. converts as follows:
      • 0b00 --> 0b0000
      • 0b01 --> 0b0101 // No stupid lshift, the gap is filled up
      • 0b10 --> 0b1010
      • 0b11 --> 0b1111 // Saturated stays saturated, white stays white
  • C++20 concepts for colortypes and colorgroups (currently ColorPlane and ColorPalletizing). These concepts support overall readability and enabled very nice overload resolution modulation for modm::graphic::Buffer and modm::graphic::Display (Coming soon, see feature/rewrite-graphic
  • Any imaginable color conversion works - type, granularity, palletized or planar... doesn't matter.
  • Operators with saturation arithmetics

TODO

  • Requires [fix] modm::Saturated #780 being merged
  • Implement some missing operators
  • Optional range check for value in ProportionalUnsigned::ProportionalUnsigned(T value)
  • Update header Timestamps

TomSaw mentioned this pull request Nov 15, 2021
Closed
5 of 20 tasks
TomSaw changed the title [ui:color] Very generic colors: Rgb, Hsv and Gray [] Very generic ui:color: Rgb, Hsv and Gray Nov 16, 2021
TomSaw changed the title [] Very generic ui:color: Rgb, Hsv and Gray [color] Very generic colors: Rgb, Hsv and Gray Nov 16, 2021
TomSaw force-pushed the feature/very-generic-colors branch from 984e341 to e5c2b15 Compare November 23, 2021 07:55
TomSaw force-pushed the feature/very-generic-colors branch 3 times, most recently from 4bb19da to c5e651d Compare January 9, 2022 13:31

TomSaw commented Jan 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

This is ready for a first review @salkinium
I also got my fingers on modm::Saturated again, improved the API and added another test SaturationTest::test__uint8_t_ref.

salkinium self-requested a review January 9, 2022 20:26
salkinium reviewed Jan 10, 2022
View reviewed changes

salkinium left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

So far I like it a lot, very nice design with the generic bit width!

Comment thread src/modm/math/proportional_unsigned.hpp Outdated
Comment thread src/modm/math/proportional_unsigned.hpp Outdated
Comment thread src/modm/math/saturation/saturated.hpp Outdated
Comment thread src/modm/math/utils/integer_traits.hpp Outdated
Comment thread src/modm/ui/color/concepts.hpp Outdated
Comment thread src/modm/ui/color/gray.hpp Outdated
Comment thread src/modm/ui/color/gray.hpp Outdated
Comment thread src/modm/ui/color/gray.hpp
Comment thread src/modm/ui/color/hsv.hpp Outdated
Comment thread test/modm/math/saturation/saturation_test.hpp Outdated
TomSaw force-pushed the feature/very-generic-colors branch 9 times, most recently from 67b645a to 7498e57 Compare January 11, 2022 17:56
salkinium reviewed Jan 12, 2022
View reviewed changes
Comment thread src/modm/math/proportional_unsigned.hpp Outdated
Comment thread src/modm/math/proportional_unsigned.hpp Outdated
TomSaw force-pushed the feature/very-generic-colors branch 7 times, most recently from 7d115fe to 4bb51f8 Compare January 12, 2022 14:38

TomSaw commented Jan 12, 2022 •
edited by salkinium
Loading

Copy link
Copy Markdown
Contributor Author

Apologize my enduring "push --force" thunderstorm. I'm transitioning from poke mode to test local, push after success mode 😬

Tests passed @salkinium! What's left is a good solution for the ProportionalUnsigned constructor 🙄

TomSaw requested a review from chris-durand March 8, 2022 16:08
TomSaw force-pushed the feature/very-generic-colors branch 3 times, most recently from 1775d67 to d31c8f9 Compare March 10, 2022 09:27
TomSaw force-pushed the feature/very-generic-colors branch 2 times, most recently from 76a0520 to 623b9db Compare March 22, 2022 17:31
TomSaw force-pushed the feature/very-generic-colors branch 3 times, most recently from d4aa227 to d30e633 Compare April 13, 2022 15:02
TomSaw force-pushed the feature/very-generic-colors branch 6 times, most recently from cb45e64 to ce8a2d6 Compare June 13, 2022 13:51

TomSaw commented Jun 13, 2022

Copy link
Copy Markdown
Contributor Author

These colors have been proven in the field. Adding code snippets to the docs (color.md) rounds up the package.

TomSaw requested a review from salkinium June 13, 2022 14:59

chris-durand commented Jun 13, 2022

Copy link
Copy Markdown
Member

Please have a look on my solution @chris-durand when you find a minute.

I'll try to find some time tomorrow to look at the PR.

chris-durand reviewed Jun 17, 2022
View reviewed changes
Comment thread src/modm/math/utils/misc.hpp Outdated
Comment thread src/modm/math/utils/misc.hpp Outdated
chris-durand reviewed Jun 17, 2022
View reviewed changes
Comment thread src/modm/math/proportional_unsigned.hpp Outdated

TomSaw commented Jun 22, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Wanna get some stuff off the table. This PR feels robust, if you find a minute @salkinium 😅...

salkinium reviewed Jun 22, 2022
View reviewed changes

salkinium left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm fine with these changes, modulo my doc comment.
I'm two weeks from handing in my thesis, so cannot give you a technical review, but if @chris-durand approves this PR, I'll merge it.

Comment thread src/modm/ui/color/color.md
Only the Hue component of Hsv ColorType wraps around just like integers do.

## Flexible Widgets
- [ ] Complete this snippet and make the code actually working

salkinium Jun 22, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

?

We generally don't promise future code in the documentation, cos we usually don't get it done in time ;-P

chris-durand commented Jun 22, 2022

Copy link
Copy Markdown
Member

@TomSaw Did you see what I wrote in #781 (comment) ? The discussion got so lengthy that github hides most of it by default 😅

TomSaw commented Jun 23, 2022

Copy link
Copy Markdown
Contributor Author

@TomSaw Did you see what I wrote in #781 (comment) ? The discussion got so lengthy that github hides most of it by default 😅

Nope. Totally missed that and check it tomorrow.

TomSaw force-pushed the feature/very-generic-colors branch from ce9bf1f to f3b91c9 Compare September 9, 2023 18:53

salkinium commented Sep 9, 2023

Copy link
Copy Markdown
Member

I'm still very interested in merging this, unfortunately, I've completely lost the overview of the review. I think it's almost done, perhaps you want to address the least comments and then we can merge it?

TomSaw commented Sep 9, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Hey there. Yes it was almost done. Only chris requested to extract the "arbitrary integer" logic into its own class for good reasons, see #781 (comment) and #781 (comment)

Simple task but then, a strong force pulled me into another universe. One with a ton of PHP actually 🤕

I've just read the processing::fibers readme and it sounds gorgeous 🤩 congrats for completing this!

TomSaw commented Sep 10, 2023

Copy link
Copy Markdown
Contributor Author

Nothing changed yet.. just cleaned up the tree

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

salkinium salkinium left review comments
rleh rleh left review comments
chris-durand chris-durand left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

advanced 🤯 feature 🚧

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

Back | FazBrowse Home | New Git URL