C3C0 / GravityBox

This repository is deprecated. Official one is maintained under GravityBox organization
https://github.com/GravityBox/GravityBox
92 stars 354 forks source link

Circle Battery color #82

Closed Wikiwix closed 10 years ago

Wikiwix commented 10 years ago

The Circle Battery is too bright and I think the centered clock too. For the circle I looked through the code. It is a fork of the CM implementation and the color is even commented. But not fixed due theme compatibility. Since GB is for stock ROMs I think hardcoding the color would be alright?

C3C0 commented 10 years ago

I think we should stick to default system colors when icon coloring option is off. There's one place where it's hard coded (StatusbarIconManager) and I was already thinking about changing it to follow default system color. Hard coding these things is not a good practice.

Wikiwix commented 10 years ago

I'm with you, but the different colour really distresses me... Another way would be allowing the colour to be set in the settings? Or a checkbox to choose hardcoded option? Or don't you like that idea?

C3C0 commented 10 years ago

Can you post a screenshot for me to see where your difference is? You have icon coloring turned off, right?

Wikiwix commented 10 years ago

Yes, everything is stock except GB. colour Sometimes it's not that obvious, but in some lighting conditions the difference is really stark

C3C0 commented 10 years ago

BTW, you can use icon coloring option to adjust the color according to your preference. All of the icons will share the same color.

Wikiwix commented 10 years ago

I see, but I don't like the different icons...

Wikiwix commented 10 years ago

What do you think of this solution? (I have not compiled and tried, and whether the color is prefect I do not know)

https://github.com/Wikiwix/GravityBox/blob/Circle-blue/src/com/ceco/gm2/gravitybox/CmCircleBattery.java

C3C0 commented 10 years ago

So as I understand, this affects only a specific ROM having that particular res ID for that color. If this is what you want to achieve, then yes. It should work.

Wikiwix commented 10 years ago

Yes, since most ROMs will leave the colors unchanged (especially the target devices, near AOSP ones) there can be used the darker blue to match the icons. But if anything unexpected happens, like modding it behaves as if it was not hardcoded. Therefore this solution combines the better color fitting and the robustness of not-hardcoding.

C3C0 commented 10 years ago

But your if is comparing resource id, not color :) resource Id is just unique identifier and it can change every time you recompile rom. This makes your mod work on one particular device model and one particular ROM (yours)

Wikiwix commented 10 years ago

My code is bullshit, I will look into it again... Hopefully with more sensible code afterwards...

Wikiwix commented 10 years ago

https://github.com/Wikiwix/GravityBox/blob/Circle-blue/src/com/ceco/gm2/gravitybox/CmCircleBattery.java

This should make way more sense, idea is the same as before, but now right coded(although the holo_blue_dark color code is from ics if it changed it has to be modified..)

C3C0 commented 10 years ago

I've made some improvements to statusbar icon color handling. You can check commit: https://github.com/C3C0/GravityBox/commit/27b580b706a3b653df672c625d493b69fd62c6df

The problem was that the CM circle battery color was always replaced by hardcoded DEFAULT_ICON_COLOR regardless what color was set directly in CmCircleBattery class. Built apk for this change is available here for you to check if things got better on your system: http://ceco.sk.eu.org/GooMirror/GravityBox/GravityBox_wikiwix.apk

Wikiwix commented 10 years ago

Ok, now if I would use the statusbar icon coloring, everything has the same color. (btw your hardcoded color was not holo_blue_dark...) But you did not impact the circle battery...

The point, why the colors do not fit is, that the standard battery/wifi/signal indicator are icons, and not drawn like your circle battery. And for whatever reason this blue is not as dark as holo_blue_dark (makes sense since this blue is very bright on an amoled and it kind of disturbs the hole UI, but should then be in the references too...) And that is why it would be great to change the color the circle battery is drawn with to the one the other stock symbols have. Everyone with the stock icons should have that issue (but for example on my Nexus 7 it's not that visible except on high brightness level) (I do not want to say you did not know this, I just want to make sure we agree in the reasons)

My suggestion would be to check whether holo_dark_blue is what it is normally and if yes the color of the icons is used. If there is anything not as it is expected trust the system resources. To make a clean change this behavior should be used for the time, AM/PM, day of week battery percentage and default custom statusbar icon colors too.

C3C0 commented 10 years ago

Yeah, I see your point. Anyway, the previous implementation really had impact on circle battery since although holo_dark_blue was set directly in CmCircleBattery constructor it got replaced by ModStatusBarColor at startup: https://github.com/C3C0/GravityBox/blob/v2.4.1/src/com/ceco/gm2/gravitybox/ModStatusbarColor.java#L535

Now it uses original, not hardcoded holo_blue_dark. Even if we added your logic to CmCircleBattery, it would get replaced by ModStatusbarColor with default color that comes from StatusbarIconManager.

I think the best approach would be to:

Wikiwix commented 10 years ago

Sorry, I did not know that... Your approach sounds great. I do not know how you are going to implement that, but I would guess using the stock battery(instead of wifi icon) is better... Since the Wifi icon uses many different shades of blue because it has very small areas of blue, whereas the battery icon (if full) has the same shade of blue for most of the area.

btw: is the clock color changed too? and if yes where?

C3C0 commented 10 years ago

That's exactly what I was thinking. Take battery icon and get center pixel of it (based on width and height). The probability the pixel has correct color is 99.9% :)

Clock color is set in the same place as CmCircleBattery. But there's a different approach. At startup, before GB sets default color from icon manager, it reads and stores the original clock color. https://github.com/C3C0/GravityBox/blob/master/src/com/ceco/gm2/gravitybox/ModStatusbarColor.java#L530

Wikiwix commented 10 years ago

I see, thanks for the explanation. The clock is funny, since EVEN in stock it uses holo_dark_blue and it does not look like the icons next to it.... I only noticed it because of the circle battery... From now on I will see it on every stock device every time :D for the clock it's not that obvious since it's that slim... so you have to decide leave the clock stock or fix it...

C3C0 commented 10 years ago

Hi. What do you think? https://github.com/C3C0/GravityBox/commit/d5873c7b244a13c213eb0c5ae6f8f4ff726d11cf Tried it and while toggling new option on/off I can now clearly see the difference in colors :)

Wikiwix commented 10 years ago

Great! But I have to say, since proper holo_blue_dark was used, the difference was not more that big. But it's still great to have a perfect match :)

Moreover your approach seems to be be good in the advent of KitKat...