Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor modm:ui:color #616

Merged
merged 1 commit into from
May 21, 2021
Merged

Refactor modm:ui:color #616

merged 1 commit into from
May 21, 2021

Conversation

TomSaw
Copy link
Contributor

@TomSaw TomSaw commented Apr 17, 2021

While working on #604 the fragmentation of Colors (modm::ui::color and modm::glcd::Color) forced me to this PR.

Key changes

  • Integrated modm::glcd::Color (Rgb565) with modm::ui::color
    • Backwards compatible: named constructors still work glcd::Color::black(), glcd::Color::white() a.s.o.
  • New "Color" Brightness to be used with Grayscale and Monochrome Displays, Simple dimmed LEDs
  • New Color-Factory color::RgbHtml returning color::Rgb by by HTML-Colornames
  • Moved colors into own dir ../modm/ui/color and seperate *.hpp
  • Replaced ::toHsv(...), ::toRgb(...) with constexpr conversion constructors.
    • You can now simply type color_display.setColor(color::Rgb(255, 0, 0)) or color_display.setColor(color::Hsv(0, 100, 100)) to submit pure Red in Rgb565 format.
    • Also check /src/modm/ui/led/rgb.hpp where setColor(Hsv color) and fadeTo(Hsv color, uint16_t time) has become rudimentary.

Performance

Using conversion constructors is defacto more elegant. I've compared the compilates: No differences ...

  • ... no matter if you compare old vs new implementation
  • ... no matter if you compare various constructors for one and the same color:
    • color_display.setColor(color::Rgb565(0xF800))
    • vs color_display.setColor(color::Rgb(255, 0, 0))
    • vs color_display.setColor(color::Hsv(0, 100, 100))

@TomSaw TomSaw changed the title Refactor modm::color Refactor modm::ui::color Apr 17, 2021
@TomSaw TomSaw force-pushed the refactor-color branch 12 times, most recently from eb21fa7 to a122503 Compare April 18, 2021 11:11
@TomSaw TomSaw mentioned this pull request Apr 18, 2021
7 tasks
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

So far so good!

examples/arduino_nano/color/main.cpp Show resolved Hide resolved
src/modm/ui/color/hsv.hpp Outdated Show resolved Hide resolved
src/modm/ui/color/hsv.hpp Outdated Show resolved Hide resolved
src/modm/ui/color/rgb_impl.hpp Show resolved Hide resolved
src/modm/ui/color/rgb565.hpp Outdated Show resolved Hide resolved
src/modm/ui/color/rgb565.hpp Outdated Show resolved Hide resolved
src/modm/ui/color/rgb.hpp Outdated Show resolved Hide resolved
@TomSaw TomSaw force-pushed the refactor-color branch 2 times, most recently from 7e6d12a to 1688174 Compare April 18, 2021 17:51
@TomSaw TomSaw force-pushed the refactor-color branch 4 times, most recently from e3814e9 to 6c43130 Compare April 20, 2021 14:39
@TomSaw
Copy link
Contributor Author

TomSaw commented Apr 20, 2021

Hey @salkinium please check color::Brightness. I've quickly sketched the RgbT(BrightnessT<_T> brightness) conversion constructor. What's the right reverse of the formula???

8bpp Grayscale
8bits per pixel, but grayscale! This uses the following formula to map colors onto their perceived brightness:
Luminence: (0.2125 * red) + (0.7154 * green) + (0.0721 * blue)

/test/modm/ui/color/color_test.hpp unveils asymetries on forward-backward conversion (Rgb -> Hsv -> Rgb). I think it's normal cause information is lost on conversion but really that much?

Edit: Got some new ideas and investigate this in another session... color_test.hpp is work in progress Stay tuned...

@TomSaw TomSaw force-pushed the refactor-color branch 2 times, most recently from 3c4956c to 779c042 Compare April 20, 2021 15:18
@TomSaw TomSaw force-pushed the refactor-color branch 4 times, most recently from 81645e0 to 631610d Compare May 1, 2021 13:49
@TomSaw
Copy link
Contributor Author

TomSaw commented May 4, 2021

Have you looked into the upainter colors? They are already implemented for a lot of formats and tested and fairly fast with some basic bit optmizations and include implementations for all of the Porter&Duff composition operations (original paper). Some of these operations are also supported in hardware by the STM32 DMA2D, which is why I initially implemented them.

I'm busy with work so this slowed down. Let me finish up the color-tests + CI-claims so you can merge the state. The impros fit the goal of "refactoring color" and are worth being shared with the folks! @salkinium

These Rgb-Calculations, Saturated-stuff and upainter-integrations then receive their personal silent hours in a fresh PR.

@TomSaw TomSaw force-pushed the refactor-color branch 2 times, most recently from b9b04a8 to 48b9734 Compare May 7, 2021 10:49
src/modm/ui/color/rgb565.hpp Outdated Show resolved Hide resolved
src/modm/ui/color/rgbhtml.hpp Show resolved Hide resolved
src/modm/ui/color/rgb.hpp Show resolved Hide resolved
src/modm/ui/color/rgb.hpp Outdated Show resolved Hide resolved
src/modm/math/utils/concepts.hpp Outdated Show resolved Hide resolved
@salkinium
Copy link
Member

I refactored your branch a little myself instead of giving you a bazillion comments. I mostly changed style and removed some top-level using namespace foo; in headers which would leak into the whole code.

Apart from the relative/normalize color functions I'm happy with this now!

@TomSaw
Copy link
Contributor Author

TomSaw commented May 8, 2021

I'll check it firstly in the morning + and gonna add these tests so we can clean the table:

Got pretty good drive today optimizing the dusty graphic algorithms witch is lots more fun.

@TomSaw
Copy link
Contributor Author

TomSaw commented May 8, 2021

Didn't want to touch the relative/normalize stuff in Rgb since it doesn't hurt me (to much), I actually have no usage for it and especially didn't want to break someone's algos.

@TomSaw
Copy link
Contributor Author

TomSaw commented May 8, 2021

Few more sessions general improvements on the graphics API and my head is ready to study upainter and see what we can make with all of it.

@TomSaw TomSaw force-pushed the refactor-color branch 2 times, most recently from 1c24c4d to 20a4842 Compare May 19, 2021 18:05
src/modm/ui/color/rgb.hpp Outdated Show resolved Hide resolved
@TomSaw TomSaw force-pushed the refactor-color branch 4 times, most recently from 37fcd15 to 3bd752e Compare May 20, 2021 05:19
@TomSaw TomSaw requested a review from salkinium May 20, 2021 17:08
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

I extracted the weird RGB normalize into their own standalone function.

@TomSaw
Copy link
Contributor Author

TomSaw commented May 20, 2021

I extracted the weird RGB normalize into their own standalone function.

Wunderbar. I've been very dismotivated to do that... thanks @salkinium 🍬

@TomSaw
Copy link
Contributor Author

TomSaw commented May 20, 2021

... instead, i've made great strides with the graphics functions. Anything has become so badly fast.. coming soon. need to cleanu and prepare a good presentation cause i wanna change the APIs.

@salkinium salkinium added the ci:hal Triggers the exhaustive HAL compile CI jobs label May 20, 2021
@salkinium salkinium merged commit a105072 into modm-io:develop May 21, 2021
@TomSaw TomSaw deleted the refactor-color branch May 21, 2021 06:29
@salkinium salkinium added this to the 2021q2 milestone Jun 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:hal Triggers the exhaustive HAL compile CI jobs refactor 🔥
Development

Successfully merging this pull request may close these issues.

3 participants