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

Color::get_{r,g,b,a}8() use truncate instead of round #47022

Closed
dalexeev opened this issue Mar 15, 2021 · 1 comment · Fixed by #47726
Closed

Color::get_{r,g,b,a}8() use truncate instead of round #47022

dalexeev opened this issue Mar 15, 2021 · 1 comment · Fixed by #47726

Comments

@dalexeev
Copy link
Member

Godot version:
3.2.4-rc4, 4.0-dev

OS/device including version:
Kubuntu 20.10

Issue description:
These methods use truncate instead of round, which causes inconsistencies with to_html in some cases (see example below). Here are the code sections responsible for this:

godot/core/math/color.h

Lines 199 to 206 in 6eef187

_FORCE_INLINE_ void set_r8(int32_t r8) { r = (CLAMP(r8, 0, 255) / 255.0); }
_FORCE_INLINE_ int32_t get_r8() const { return int32_t(CLAMP(r * 255.0, 0.0, 255.0)); }
_FORCE_INLINE_ void set_g8(int32_t g8) { g = (CLAMP(g8, 0, 255) / 255.0); }
_FORCE_INLINE_ int32_t get_g8() const { return int32_t(CLAMP(g * 255.0, 0.0, 255.0)); }
_FORCE_INLINE_ void set_b8(int32_t b8) { b = (CLAMP(b8, 0, 255) / 255.0); }
_FORCE_INLINE_ int32_t get_b8() const { return int32_t(CLAMP(b * 255.0, 0.0, 255.0)); }
_FORCE_INLINE_ void set_a8(int32_t a8) { a = (CLAMP(a8, 0, 255) / 255.0); }
_FORCE_INLINE_ int32_t get_a8() const { return int32_t(CLAMP(a * 255.0, 0.0, 255.0)); }

godot/core/math/color.cpp

Lines 421 to 441 in 6eef187

String _to_hex(float p_val) {
int v = Math::round(p_val * 255);
v = CLAMP(v, 0, 255);
String ret;
for (int i = 0; i < 2; i++) {
char32_t c[2] = { 0, 0 };
int lv = v & 0xF;
if (lv < 10) {
c[0] = '0' + lv;
} else {
c[0] = 'a' + lv - 10;
}
v >>= 4;
String cs = (const char32_t *)c;
ret = cs + ret;
}
return ret;
}

Steps to reproduce:

tool
extends EditorScript

func _run():
    var c = Color.from_hsv(1/12.0, 0.25, 0.8)
    print(c.to_html(false))                    # ccb399
    print('%02x%02x%02x' % [c.r8, c.g8, c.b8]) # ccb299

Minimal reproduction project:

@dalexeev
Copy link
Member Author

Should be:

 _FORCE_INLINE_ void set_r8(int32_t r8) { r = (CLAMP(r8, 0, 255) / 255.0); } 
 _FORCE_INLINE_ int32_t get_r8() const { return int32_t(CLAMP(Math::round(r * 255.0), 0.0, 255.0)); } 
 _FORCE_INLINE_ void set_g8(int32_t g8) { g = (CLAMP(g8, 0, 255) / 255.0); } 
 _FORCE_INLINE_ int32_t get_g8() const { return int32_t(CLAMP(Math::round(g * 255.0), 0.0, 255.0)); } 
 _FORCE_INLINE_ void set_b8(int32_t b8) { b = (CLAMP(b8, 0, 255) / 255.0); } 
 _FORCE_INLINE_ int32_t get_b8() const { return int32_t(CLAMP(Math::round(b * 255.0), 0.0, 255.0)); } 
 _FORCE_INLINE_ void set_a8(int32_t a8) { a = (CLAMP(a8, 0, 255) / 255.0); } 
 _FORCE_INLINE_ int32_t get_a8() const { return int32_t(CLAMP(Math::round(a * 255.0), 0.0, 255.0)); } 

dalexeev added a commit to dalexeev/godot that referenced this issue Apr 9, 2021
@akien-mga akien-mga added this to the 4.0 milestone Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants