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

Add color themes #69

Closed
wants to merge 6 commits into from
Closed

Add color themes #69

wants to merge 6 commits into from

Conversation

tjhorner
Copy link
Contributor

@tjhorner tjhorner commented Sep 2, 2017

These changes will add color themes to Epsilon. Because, you know, everyone loves customization :)

I rarely touch C++, so please do give me honest criticism about the code quality!

Also, I have only tested this on the simulator. So if anyone has a device, please test to see if themes work and persist with reboots.

Here are the themes (change it in Settings -> Theme):

Yellow (Default)

Yellow Theme

Blue

Blue Theme

Green

Green Theme

Red

Red Theme

@CLAassistant
Copy link

CLAassistant commented Sep 2, 2017

CLA assistant check
All committers have signed the CLA.

@rhaamo
Copy link
Contributor

rhaamo commented Sep 2, 2017

Great idea thanks !

Just tried on device :

  • the setting line to ignore the "update pop-up" on start seems broken, IIRC it's a switch and here it's display a classic arrow for submenu, clicking on it does not do anything. (you can activate this in build/platform.simulator.mak by putting a 1 to the two last lines)
  • after selecting a theme, the calc seems to switch in sleep mode, and needs to press the power button, but the theme change works

It will not survive a reset, but it will survive the "shutdown" mode.

@clifward
Copy link

clifward commented Sep 2, 2017

Is it possible to have something like that ?

29992867-f7c98216-8f5a-11e7-98f1-92c7fd8118e1

@adriweb
Copy link
Contributor

adriweb commented Sep 2, 2017

@clifward : At this point, I’m wondering telling the LCD to rotate the palette would be better :) (However it might not be using a palette in this mode...)

@tjhorner
Copy link
Contributor Author

tjhorner commented Sep 2, 2017

@rhaamo I believe I've fixed the LCD going into sleep when you change the theme, can you test? I'll see what I can do about the update popup. Probably just need to change a few indexes around in apps/settings/main_controller.cpp.

@clifward Yes, changing the icon's colors would be ideal to match the theme, but the only way I could think of doing this is to create multiple pngs with all the different colors. But @adriweb brings up a good point -- maybe a pseudo-palette-change on the images could be implemented? The ImageView could color shift them upon rendering, perhaps.

And I just realized: the exam mode icon also "breaks" when you change a theme:

image

So an ImageView color shift would be nice in this case as well :)

@tjhorner
Copy link
Contributor Author

tjhorner commented Sep 2, 2017

And the "Update pop-up" setting should be fixed with the latest commit.

@rhaamo
Copy link
Contributor

rhaamo commented Sep 2, 2017

@tjhorner confirming, fixed for my two issues ! thanks

@tjhorner
Copy link
Contributor Author

tjhorner commented Sep 3, 2017

So, how should the different color icons be implemented? I am seeing a couple routes:

  • Ideal method: Modify the ImageView to implement a "color/hue shift" method. This method would shift the hue of all the colors in an image by a certain amount. It would be called during runtime, and each theme would have its own amount to shift by, based on the color we want.
  • Less ideal method. Have the image inliner generate multiple copies of the image, one for each theme. This would take up more space on the device, but less CPU time. It's better than the method below, though, since the icons are generated automatically.
  • Least ideal method. Modify each image icon manually (in Photoshop, for example) and make them each their own icon. This is the least preferred, since if new icons were to be added, the icon author would need to manually change the icon's hue for each theme, and it might be inconsistent with the other themes. The images would also considerably raise the size of the OS, and with 1MB of flash storage, size is sacred.

I'd like to hear your thoughts on these.

@blackketter
Copy link
Contributor

One fourth route:

  • Add color table maps which would map image colors from set to another. This could be added along with support for smaller PNGs (4-bit, 8-bit) with color tables to also save flash space.

@adriweb
Copy link
Contributor

adriweb commented Sep 3, 2017

Yet another idea, although more cumbersome: don't actually use images for apps icons, but draw them programmatically.

@tjhorner
Copy link
Contributor Author

tjhorner commented Sep 3, 2017

@adriweb Technically, they are already drawn programmatically, since the inliner converts them into a C++ class. But I see what you mean, we could use themeDarkColor() / themeLightColor() instead of a hard-coded color.

@boricj
Copy link
Contributor

boricj commented Sep 3, 2017

Wouldn't the best way be icons in a palettized format, where the first couple indexes are set aside for shades of the main color of the theme, replaced at run-time?

Otherwise I'm worried how to make this work properly with arbitrary third-party apps...

@Ecco
Copy link
Contributor

Ecco commented Sep 4, 2017

Hi! Thank you very much for this suggestion and for taking your time to write some code.
This is a fun idea, and I can see why it would be nice to have, but I hope you'll agree it's not a feature everyone will need. It can even get a bit confusing for newcomers.

That's why I believe this feature would be best as a compile-time feature.

It would also make the job a lot easier: you could imagine using ImageMagick in a Makefile step to pre-process images before they are built.

@tjhorner
Copy link
Contributor Author

tjhorner commented Sep 4, 2017

@Ecco That's definitely a good point, and I agree with you that making it compile-time would lessen confusion. Especially with adriweb's suggestion of a "build-your-own-OS" tool, it would give us even more flexibility (custom hex colors, perhaps?)

I'll see what I can do to refactor this into a compile-time feature.

@Ecco
Copy link
Contributor

Ecco commented Sep 5, 2017

Thank you very much @tjhorner :)

@Ecco
Copy link
Contributor

Ecco commented Jan 16, 2018

We will most likely not merge a runtime theme engine, so I'm going to close this PR. The idea is interesting though, so feel free to reopen (or create another PR) if you'd like to work on a compile-time version.

@Ecco Ecco closed this Jan 16, 2018
gbraad referenced this pull request in gbraad/numworks-firmwares Mar 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants