From ae03bc4d789b2367afbc4443774c275bef630628 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Thu, 7 Nov 2024 13:27:11 +0100 Subject: [PATCH 01/20] Modify color in ColorPicker in HSV Rectangle mode with keyaboard or joypad --- scene/gui/color_picker.cpp | 58 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 997120ff2519..6217682723e6 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1287,6 +1287,13 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { circle_mat->set_shader_parameter("v", v); } } + + if (c->has_focus()) { + // TODO: Add and use theme focus style for those controls + Color focus_color = Color::from_string("#FFFFFF", color); + Rect2 r = Rect2(Point2(), c->get_size()); + c->draw_rect(r, focus_color, false); + } } void ColorPicker::_slider_draw(int p_which) { @@ -1400,6 +1407,31 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { emit_signal(SNAME("color_changed"), color); } } + + // TODO: Is it better to cast and check it or to not check it and get vector? + Ref kev = p_event; + Ref jbev = p_event; + Ref jmev = p_event; + + if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { + // TODO: It should be done in process instead of input to handle joypads better + // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit + Vector2 color_change_vector = Input::get_singleton()->get_vector("ui_left", "ui_right", "ui_up", "ui_down"); + if (!Math::is_zero_approx(color_change_vector.length())) { + if (actual_shape == SHAPE_HSV_RECTANGLE) { + s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); + v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); + } + + accept_event(); + _copy_hsv_to_color(); + last_color = color; + set_pick_color(color); + + // TODO: Consider handling deffered mode + emit_signal(SNAME("color_changed"), color); + } + } } void ColorPicker::_w_input(const Ref &p_event) { @@ -1452,6 +1484,30 @@ void ColorPicker::_w_input(const Ref &p_event) { emit_signal(SNAME("color_changed"), color); } } + + // TODO: Is it better to cast and check it or to not check it and get axis? + Ref kev = p_event; + Ref jbev = p_event; + Ref jmev = p_event; + + if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { + // TODO: It should be done in process instead of input to handle joypads better + // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit + float color_change = Input::get_singleton()->get_axis("ui_up", "ui_down"); + if (!Math::is_zero_approx(color_change)) { + if (actual_shape == SHAPE_HSV_RECTANGLE) { + h = CLAMP(h + color_change / modes[MODE_HSV]->get_slider_max(0), 0, 1); + } + + accept_event(); + _copy_hsv_to_color(); + last_color = color; + set_pick_color(color); + + // TODO: Consider handling deffered mode + emit_signal(SNAME("color_changed"), color); + } + } } void ColorPicker::_slider_or_spin_input(const Ref &p_event) { @@ -1832,6 +1888,7 @@ ColorPicker::ColorPicker() { hb_edit->add_child(uv_edit); uv_edit->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(uv_edit)); uv_edit->set_mouse_filter(MOUSE_FILTER_PASS); + uv_edit->set_focus_mode(FOCUS_ALL); uv_edit->set_h_size_flags(SIZE_EXPAND_FILL); uv_edit->set_v_size_flags(SIZE_EXPAND_FILL); uv_edit->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(0, uv_edit)); @@ -1977,6 +2034,7 @@ ColorPicker::ColorPicker() { w_edit = memnew(Control); hb_edit->add_child(w_edit); + w_edit->set_focus_mode(FOCUS_ALL); w_edit->set_h_size_flags(SIZE_FILL); w_edit->set_v_size_flags(SIZE_EXPAND_FILL); w_edit->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_w_input)); From 68526dd3602ad9f516e519ca0b6e5adfaf627a55 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 10 Nov 2024 23:38:02 +0100 Subject: [PATCH 02/20] Modify color in ColorPicker in VHS Circle mode with keyboard or joypad --- scene/gui/color_picker.cpp | 31 +++++++++++++++++++++++++++++++ scene/gui/color_picker.h | 2 ++ 2 files changed, 33 insertions(+) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 6217682723e6..1a58eff34d29 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -391,6 +391,8 @@ void ColorPicker::_slider_value_changed() { h = sliders[0]->get_value() / 360.0; s = sliders[1]->get_value() / 100.0; v = sliders[2]->get_value() / 100.0; + + hsv_keyboard_picker_cursor_position = Vector2i(0,0); last_color = color; } @@ -808,6 +810,7 @@ void ColorPicker::_set_mode_popup_value(ColorModeType p_mode) { } else { set_color_mode(p_mode); } + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); } Variant ColorPicker::_get_drag_data_fw(const Point2 &p_point, Control *p_from_control) { @@ -1315,6 +1318,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t rad = center.angle_to_point(bev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; s = CLAMP(dist / center.x, 0, 1); + hsv_keyboard_picker_cursor_position = Vector2i(0,0); } else { return; } @@ -1382,6 +1386,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t rad = center.angle_to_point(mev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; s = CLAMP(dist / center.x, 0, 1); + hsv_keyboard_picker_cursor_position = Vector2i(0,0); } else { if (spinning) { real_t rad = center.angle_to_point(mev->get_position()); @@ -1422,6 +1427,25 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); } + else if (actual_shape == SHAPE_VHS_CIRCLE) { + Vector2 center = c->get_size() / 2.0; + + // TODO: It's a hack, as it messes up if I calculate it this way always + if (hsv_keyboard_picker_cursor_position.x == 0 && hsv_keyboard_picker_cursor_position.y == 0) { + hsv_keyboard_picker_cursor_position.x = center.x + (center.x * Math::cos(h * Math_TAU) * s); + hsv_keyboard_picker_cursor_position.y = center.y + (center.y * Math::sin(h * Math_TAU) * s); + } + + real_t potential_new_cursor_distance = center.distance_to(hsv_keyboard_picker_cursor_position+color_change_vector); + if (potential_new_cursor_distance <= center.x) { + hsv_keyboard_picker_cursor_position += color_change_vector; + } + + real_t dist = center.distance_to(hsv_keyboard_picker_cursor_position); + real_t rad = center.angle_to_point(hsv_keyboard_picker_cursor_position); + h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; + s = CLAMP(dist / center.x, 0, 1); + } accept_event(); _copy_hsv_to_color(); @@ -1498,6 +1522,9 @@ void ColorPicker::_w_input(const Ref &p_event) { if (actual_shape == SHAPE_HSV_RECTANGLE) { h = CLAMP(h + color_change / modes[MODE_HSV]->get_slider_max(0), 0, 1); } + else if (actual_shape == SHAPE_VHS_CIRCLE) { + v = CLAMP(v - color_change / modes[MODE_HSV]->get_slider_max(2), 0, 1); + } accept_event(); _copy_hsv_to_color(); @@ -1703,6 +1730,7 @@ void ColorPicker::_html_focus_exit() { } else { _update_text_value(); } + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); } void ColorPicker::set_can_add_swatches(bool p_enabled) { @@ -1885,6 +1913,7 @@ ColorPicker::ColorPicker() { hb_edit->set_v_size_flags(SIZE_SHRINK_BEGIN); uv_edit = memnew(Control); + uv_edit->set_name("UV EDIT"); hb_edit->add_child(uv_edit); uv_edit->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(uv_edit)); uv_edit->set_mouse_filter(MOUSE_FILTER_PASS); @@ -2028,7 +2057,9 @@ ColorPicker::ColorPicker() { wheel->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(2, wheel)); wheel_uv = memnew(Control); + wheel_uv->set_name("WHEEL UV"); wheel_margin->add_child(wheel_uv); + wheel_uv->set_focus_mode(FOCUS_ALL); wheel_uv->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(wheel_uv)); wheel_uv->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(0, wheel_uv)); diff --git a/scene/gui/color_picker.h b/scene/gui/color_picker.h index 59540d9ace08..70f5083da1da 100644 --- a/scene/gui/color_picker.h +++ b/scene/gui/color_picker.h @@ -121,6 +121,8 @@ class ColorPicker : public VBoxContainer { #endif int current_slider_count = SLIDER_COUNT; + // TODO: Think about better name or a way to not use it at all + Vector2i hsv_keyboard_picker_cursor_position; static const int MODE_BUTTON_COUNT = 3; bool slider_theme_modified = true; From f92189496c40951328b419e172aac534c917f9cd Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Mon, 11 Nov 2024 00:12:31 +0100 Subject: [PATCH 03/20] Modify color in ColorPicker in OKHSL Circle mode with keyboard or joypad --- scene/gui/color_picker.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 1a58eff34d29..9d80892511dc 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -743,6 +743,7 @@ void ColorPicker::set_picker_shape(PickerShapeType p_shape) { btn_shape->set_button_icon(shape_popup->get_item_icon(p_shape)); } + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); current_shape = p_shape; #ifdef TOOLS_ENABLED @@ -1427,7 +1428,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); } - else if (actual_shape == SHAPE_VHS_CIRCLE) { + else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { Vector2 center = c->get_size() / 2.0; // TODO: It's a hack, as it messes up if I calculate it this way always @@ -1522,7 +1523,7 @@ void ColorPicker::_w_input(const Ref &p_event) { if (actual_shape == SHAPE_HSV_RECTANGLE) { h = CLAMP(h + color_change / modes[MODE_HSV]->get_slider_max(0), 0, 1); } - else if (actual_shape == SHAPE_VHS_CIRCLE) { + else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { v = CLAMP(v - color_change / modes[MODE_HSV]->get_slider_max(2), 0, 1); } From e83b406d451860379c22997f9903086857e7f324 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Tue, 12 Nov 2024 13:14:09 +0100 Subject: [PATCH 04/20] Modify color in ColorPicker in HSV Wheel mode with keyboard or joypad --- scene/gui/color_picker.cpp | 58 ++++++++++++++++++++++++++++++++++++-- scene/gui/color_picker.h | 1 + 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 9d80892511dc..ef9bade35fc5 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1214,6 +1214,9 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { Size2 real_size(c->get_size().x - corner_x * 2, c->get_size().y - corner_y * 2); x = CLAMP(real_size.x * s, 0, real_size.x) + corner_x - (theme_cache.picker_cursor->get_width() / 2); y = CLAMP(real_size.y - real_size.y * v, 0, real_size.y) + corner_y - (theme_cache.picker_cursor->get_height() / 2); + if (hsv_wheel_focused_part_index == 1) { + focus_rect = Rect2(Point2(corner_x, corner_y), real_size); + } } c->draw_texture(theme_cache.picker_cursor, Point2(x, y)); @@ -1295,8 +1298,7 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { if (c->has_focus()) { // TODO: Add and use theme focus style for those controls Color focus_color = Color::from_string("#FFFFFF", color); - Rect2 r = Rect2(Point2(), c->get_size()); - c->draw_rect(r, focus_color, false); + c->draw_rect(focus_rect, focus_color, false); } } @@ -1327,12 +1329,13 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t corner_x = (c == wheel_uv) ? center.x - Math_SQRT12 * c->get_size().width * 0.42 : 0; real_t corner_y = (c == wheel_uv) ? center.y - Math_SQRT12 * c->get_size().height * 0.42 : 0; Size2 real_size(c->get_size().x - corner_x * 2, c->get_size().y - corner_y * 2); + hsv_wheel_focused_part_index = 1; if (bev->get_position().x < corner_x || bev->get_position().x > c->get_size().x - corner_x || bev->get_position().y < corner_y || bev->get_position().y > c->get_size().y - corner_y) { { real_t dist = center.distance_to(bev->get_position()); - + hsv_wheel_focused_part_index = 0; if (dist >= center.x * 0.84 && dist <= center.x) { real_t rad = center.angle_to_point(bev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; @@ -1420,6 +1423,24 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { Ref jmev = p_event; if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { + if (actual_shape == SHAPE_HSV_WHEEL) { + // TODO: Think about better way of handling this - hack because I cannot draw focus rect around wheel control + if (p_event->is_action_pressed("ui_focus_next") && hsv_wheel_focused_part_index == 0) { + hsv_wheel_focused_part_index = 1; + + wheel_uv->queue_redraw(); + accept_event(); + return; + } else if (p_event->is_action_pressed("ui_focus_prev") && hsv_wheel_focused_part_index == 1) { + hsv_wheel_focused_part_index = 0; + wheel_uv->queue_redraw(); + accept_event(); + return; + } else if (p_event->is_action_pressed("ui_focus_next")) { + hsv_wheel_focused_part_index = 0; + } + } + // TODO: It should be done in process instead of input to handle joypads better // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit Vector2 color_change_vector = Input::get_singleton()->get_vector("ui_left", "ui_right", "ui_up", "ui_down"); @@ -1447,6 +1468,37 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; s = CLAMP(dist / center.x, 0, 1); } + else if (actual_shape == SHAPE_HSV_WHEEL) { + if (hsv_wheel_focused_part_index == 1) { + s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); + v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); + } else if (hsv_wheel_focused_part_index == 0) { + int h_change = 0; + + if (Math::is_equal_approx(h, 0) || Math::is_equal_approx(h, 0.5f) || Math::is_equal_approx(h, 1)) { + color_change_vector.x = 0; + } else if (Math::is_equal_approx(h, 0.25f) || Math::is_equal_approx(h, 0.75f)) { + color_change_vector.y = 0; + } + + if (h > 0 && h < 0.5) { + h_change -= color_change_vector.x; + } else if (h > 0.5 && h < 1) { + h_change += color_change_vector.x; + } + + if (h > 0.25 && h < 0.75) { + h_change -= color_change_vector.y; + } else if (h < 0.25 || h > 0.75) { + h_change += color_change_vector.y; + } + + h_change = CLAMP(h_change, -1, 1); + // int tmp = ((h * 360) + h_change) % 360; + // h = tmp / 360.0; + h = Math::wrapf(h + h_change / modes[MODE_HSV]->get_slider_max(0), 0, 1); + } + } accept_event(); _copy_hsv_to_color(); diff --git a/scene/gui/color_picker.h b/scene/gui/color_picker.h index 70f5083da1da..b2a68bce8c92 100644 --- a/scene/gui/color_picker.h +++ b/scene/gui/color_picker.h @@ -123,6 +123,7 @@ class ColorPicker : public VBoxContainer { int current_slider_count = SLIDER_COUNT; // TODO: Think about better name or a way to not use it at all Vector2i hsv_keyboard_picker_cursor_position; + int hsv_wheel_focused_part_index = 0; static const int MODE_BUTTON_COUNT = 3; bool slider_theme_modified = true; From 3b9dd30ec9d046e90ae7645ef1b5efc98cd768ad Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Tue, 12 Nov 2024 13:14:09 +0100 Subject: [PATCH 05/20] Modify color in ColorPicker in HSV Wheel mode with keyboard or joypad --- scene/gui/color_picker.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index ef9bade35fc5..524b0e6ac98c 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1133,6 +1133,7 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { if (!c) { return; } + Rect2 focus_rect = Rect2(Point2(), c->get_size()); PickerShapeType actual_shape = _get_actual_shape(); if (p_which == 0) { From 540cad02cee8140a36f550c4fd4f7f42fcb5e2bc Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Tue, 12 Nov 2024 13:20:41 +0100 Subject: [PATCH 06/20] Check separate input actions instead of getting vector in ColorPicker --- scene/gui/color_picker.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 524b0e6ac98c..6097e2f75c82 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1444,7 +1444,19 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { // TODO: It should be done in process instead of input to handle joypads better // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit - Vector2 color_change_vector = Input::get_singleton()->get_vector("ui_left", "ui_right", "ui_up", "ui_down"); + // Vector2 color_change_vector = Input::get_singleton()->get_vector("ui_left", "ui_right", "ui_up", "ui_down"); + Vector2 color_change_vector = Vector2(); + if (p_event->is_action_pressed("ui_left", true)) { + color_change_vector.x -= 1; + } else if (p_event->is_action_pressed("ui_right", true)) { + color_change_vector.x += 1; + } else if (p_event->is_action_pressed("ui_up", true)) { + color_change_vector.y -= 1; + } else if (p_event->is_action_pressed("ui_down", true)) { + color_change_vector.y += 1; + } + + if (!Math::is_zero_approx(color_change_vector.length())) { if (actual_shape == SHAPE_HSV_RECTANGLE) { s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); From 32c287709689add26239f23a21f6ef4399324b75 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Fri, 15 Nov 2024 01:30:52 +0100 Subject: [PATCH 07/20] Use themable stylebox for ColorPicker shape focus --- scene/gui/color_picker.cpp | 7 ++++--- scene/gui/color_picker.h | 1 + 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 6097e2f75c82..b1422f2ae525 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1297,9 +1297,8 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { } if (c->has_focus()) { - // TODO: Add and use theme focus style for those controls - Color focus_color = Color::from_string("#FFFFFF", color); - c->draw_rect(focus_rect, focus_color, false); + RID ci = c->get_canvas_item(); + theme_cache.picker_focus->draw(ci, focus_rect); } } @@ -1945,6 +1944,8 @@ void ColorPicker::_bind_methods() { BIND_THEME_ITEM(Theme::DATA_TYPE_CONSTANT, ColorPicker, center_slider_grabbers); + BIND_THEME_ITEM(Theme::DATA_TYPE_STYLEBOX, ColorPicker, picker_focus); + BIND_THEME_ITEM(Theme::DATA_TYPE_ICON, ColorPicker, screen_picker); BIND_THEME_ITEM(Theme::DATA_TYPE_ICON, ColorPicker, expanded_arrow); BIND_THEME_ITEM(Theme::DATA_TYPE_ICON, ColorPicker, folded_arrow); diff --git a/scene/gui/color_picker.h b/scene/gui/color_picker.h index b2a68bce8c92..75ab49e48f1f 100644 --- a/scene/gui/color_picker.h +++ b/scene/gui/color_picker.h @@ -228,6 +228,7 @@ class ColorPicker : public VBoxContainer { bool center_slider_grabbers = true; + Ref picker_focus; Ref screen_picker; Ref expanded_arrow; Ref folded_arrow; From 58a0e629f1030662e1dfe61a2e17ba08dd2e576f Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Fri, 15 Nov 2024 01:31:47 +0100 Subject: [PATCH 08/20] Use another hack to draw focus stle around HSV wheel --- scene/gui/color_picker.cpp | 39 ++++++++++++++++---------------------- scene/gui/color_picker.h | 1 - 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index b1422f2ae525..58c61aaf1069 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -743,6 +743,12 @@ void ColorPicker::set_picker_shape(PickerShapeType p_shape) { btn_shape->set_button_icon(shape_popup->get_item_icon(p_shape)); } + if (p_shape == SHAPE_HSV_WHEEL) { + wheel_margin->set_focus_mode(FOCUS_ALL); + } else { + wheel_margin->set_focus_mode(FOCUS_NONE); + } + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); current_shape = p_shape; @@ -1215,7 +1221,7 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { Size2 real_size(c->get_size().x - corner_x * 2, c->get_size().y - corner_y * 2); x = CLAMP(real_size.x * s, 0, real_size.x) + corner_x - (theme_cache.picker_cursor->get_width() / 2); y = CLAMP(real_size.y - real_size.y * v, 0, real_size.y) + corner_y - (theme_cache.picker_cursor->get_height() / 2); - if (hsv_wheel_focused_part_index == 1) { + if (c == wheel_uv) { focus_rect = Rect2(Point2(corner_x, corner_y), real_size); } } @@ -1294,6 +1300,10 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { circle_mat->set_shader_parameter("v", v); } + } else if (p_which == 3) { + int margin = c->get_theme_constant("margin_bottom"); + focus_rect.set_position(Vector2(-margin, -margin)); + focus_rect.set_size(Vector2(focus_rect.get_size().x+margin*2, focus_rect.get_size().y+margin)); } if (c->has_focus()) { @@ -1329,13 +1339,12 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t corner_x = (c == wheel_uv) ? center.x - Math_SQRT12 * c->get_size().width * 0.42 : 0; real_t corner_y = (c == wheel_uv) ? center.y - Math_SQRT12 * c->get_size().height * 0.42 : 0; Size2 real_size(c->get_size().x - corner_x * 2, c->get_size().y - corner_y * 2); - hsv_wheel_focused_part_index = 1; if (bev->get_position().x < corner_x || bev->get_position().x > c->get_size().x - corner_x || bev->get_position().y < corner_y || bev->get_position().y > c->get_size().y - corner_y) { { real_t dist = center.distance_to(bev->get_position()); - hsv_wheel_focused_part_index = 0; + wheel_margin->grab_focus(); if (dist >= center.x * 0.84 && dist <= center.x) { real_t rad = center.angle_to_point(bev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; @@ -1423,24 +1432,6 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { Ref jmev = p_event; if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { - if (actual_shape == SHAPE_HSV_WHEEL) { - // TODO: Think about better way of handling this - hack because I cannot draw focus rect around wheel control - if (p_event->is_action_pressed("ui_focus_next") && hsv_wheel_focused_part_index == 0) { - hsv_wheel_focused_part_index = 1; - - wheel_uv->queue_redraw(); - accept_event(); - return; - } else if (p_event->is_action_pressed("ui_focus_prev") && hsv_wheel_focused_part_index == 1) { - hsv_wheel_focused_part_index = 0; - wheel_uv->queue_redraw(); - accept_event(); - return; - } else if (p_event->is_action_pressed("ui_focus_next")) { - hsv_wheel_focused_part_index = 0; - } - } - // TODO: It should be done in process instead of input to handle joypads better // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit // Vector2 color_change_vector = Input::get_singleton()->get_vector("ui_left", "ui_right", "ui_up", "ui_down"); @@ -1481,10 +1472,10 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { s = CLAMP(dist / center.x, 0, 1); } else if (actual_shape == SHAPE_HSV_WHEEL) { - if (hsv_wheel_focused_part_index == 1) { + if (c == wheel_uv) { s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); - } else if (hsv_wheel_focused_part_index == 0) { + } else if (c == wheel_margin) { int h_change = 0; if (Math::is_equal_approx(h, 0) || Math::is_equal_approx(h, 0.5f) || Math::is_equal_approx(h, 1)) { @@ -2117,6 +2108,8 @@ ColorPicker::ColorPicker() { wheel_margin = memnew(MarginContainer); wheel_margin->add_theme_constant_override("margin_bottom", 8); wheel_edit->add_child(wheel_margin); + // TODO: Hack - I cannot draw focus stylebox on wheel itself, as it's drawing based on shader + wheel_margin->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(3, wheel_margin)); wheel = memnew(Control); wheel_margin->add_child(wheel); diff --git a/scene/gui/color_picker.h b/scene/gui/color_picker.h index 75ab49e48f1f..ebd0e2f043d7 100644 --- a/scene/gui/color_picker.h +++ b/scene/gui/color_picker.h @@ -123,7 +123,6 @@ class ColorPicker : public VBoxContainer { int current_slider_count = SLIDER_COUNT; // TODO: Think about better name or a way to not use it at all Vector2i hsv_keyboard_picker_cursor_position; - int hsv_wheel_focused_part_index = 0; static const int MODE_BUTTON_COUNT = 3; bool slider_theme_modified = true; From 6ddefed809b90f2efd802ad3480e2160350df6f3 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sat, 16 Nov 2024 20:09:13 +0100 Subject: [PATCH 09/20] Handle ColorPicker focus change when picker was focused while chainging it's shape --- scene/gui/color_picker.cpp | 33 ++++++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 58c61aaf1069..f5f6376cff10 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -282,13 +282,25 @@ void ColorPicker::_update_controls() { switch (_get_actual_shape()) { case SHAPE_HSV_RECTANGLE: - wheel_edit->hide(); w_edit->show(); uv_edit->show(); btn_shape->show(); + if (wheel_uv->has_focus()) { + uv_edit->grab_focus(); + } else if (wheel_margin->has_focus()) { + w_edit->grab_focus(); + } + wheel_edit->hide(); + wheel_margin->set_focus_mode(FOCUS_NONE); break; case SHAPE_HSV_WHEEL: wheel_edit->show(); + wheel_margin->set_focus_mode(FOCUS_ALL); + if (w_edit->has_focus()) { + wheel_margin->grab_focus(); + } else if (uv_edit->has_focus()) { + wheel_uv->grab_focus(); + } w_edit->hide(); uv_edit->hide(); btn_shape->show(); @@ -297,20 +309,33 @@ void ColorPicker::_update_controls() { case SHAPE_VHS_CIRCLE: wheel_edit->show(); w_edit->show(); + if (uv_edit->has_focus()) { + wheel_uv->grab_focus(); + } else if (wheel_margin->has_focus()) { + w_edit->grab_focus(); + } uv_edit->hide(); btn_shape->show(); wheel->set_material(circle_mat); circle_mat->set_shader(circle_shader); + wheel_margin->set_focus_mode(FOCUS_NONE); break; case SHAPE_OKHSL_CIRCLE: wheel_edit->show(); w_edit->show(); + if (uv_edit->has_focus()) { + wheel_uv->grab_focus(); + } else if (wheel_margin->has_focus()) { + w_edit->grab_focus(); + } uv_edit->hide(); btn_shape->show(); wheel->set_material(circle_mat); circle_mat->set_shader(circle_ok_color_shader); + wheel_margin->set_focus_mode(FOCUS_NONE); break; case SHAPE_NONE: + wheel_margin->set_focus_mode(FOCUS_NONE); wheel_edit->hide(); w_edit->hide(); uv_edit->hide(); @@ -743,12 +768,6 @@ void ColorPicker::set_picker_shape(PickerShapeType p_shape) { btn_shape->set_button_icon(shape_popup->get_item_icon(p_shape)); } - if (p_shape == SHAPE_HSV_WHEEL) { - wheel_margin->set_focus_mode(FOCUS_ALL); - } else { - wheel_margin->set_focus_mode(FOCUS_NONE); - } - hsv_keyboard_picker_cursor_position = Vector2i(0, 0); current_shape = p_shape; From 7c3caa05e7c8e028a8d8552b635949e7ef522b01 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 00:08:15 +0100 Subject: [PATCH 10/20] Fix ColorPicker HSV wheel regression of not chainging hue by keyboard --- scene/gui/color_picker.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index f5f6376cff10..b68196efc588 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -2129,6 +2129,7 @@ ColorPicker::ColorPicker() { wheel_edit->add_child(wheel_margin); // TODO: Hack - I cannot draw focus stylebox on wheel itself, as it's drawing based on shader wheel_margin->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(3, wheel_margin)); + wheel_margin->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(wheel_margin)); wheel = memnew(Control); wheel_margin->add_child(wheel); From 8fe859093725df1234725af109d220317bbb92fc Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 00:10:40 +0100 Subject: [PATCH 11/20] Focus on picker shape if HTML textedit is not visible when activating ColorPicker --- scene/gui/color_picker.cpp | 23 ++++++++++++++++++++++- scene/gui/color_picker.h | 1 + 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index b68196efc588..cebb9f002de2 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -248,6 +248,25 @@ void ColorPicker::set_focus_on_line_edit() { callable_mp((Control *)c_text, &Control::grab_focus).call_deferred(); } +void ColorPicker::set_focus_on_picker_shape() { + switch (_get_actual_shape()) { + case SHAPE_HSV_RECTANGLE: + callable_mp(uv_edit, &Control::grab_focus).call_deferred(); + break; + case SHAPE_HSV_WHEEL: + callable_mp(wheel_uv, &Control::grab_focus).call_deferred(); + break; + case SHAPE_VHS_CIRCLE: + callable_mp(wheel_uv, &Control::grab_focus).call_deferred(); + break; + case SHAPE_OKHSL_CIRCLE: + callable_mp(wheel_uv, &Control::grab_focus).call_deferred(); + break; + default: { + } + } +} + void ColorPicker::_update_controls() { int mode_sliders_count = modes[current_mode]->get_slider_count(); @@ -2256,7 +2275,9 @@ void ColorPickerButton::pressed() { float v_offset = show_above ? -minsize.y : get_size().y; popup->set_position(get_screen_position() + Vector2(h_offset, v_offset)); popup->popup(); - if (DisplayServer::get_singleton()->has_hardware_keyboard()) { + if (!picker->is_hex_visible() && picker->get_picker_shape() != ColorPicker::SHAPE_NONE) { + picker->set_focus_on_picker_shape(); + } else if (DisplayServer::get_singleton()->has_hardware_keyboard()) { picker->set_focus_on_line_edit(); } } diff --git a/scene/gui/color_picker.h b/scene/gui/color_picker.h index ebd0e2f043d7..b9cce332b150 100644 --- a/scene/gui/color_picker.h +++ b/scene/gui/color_picker.h @@ -371,6 +371,7 @@ class ColorPicker : public VBoxContainer { bool is_hex_visible() const; void set_focus_on_line_edit(); + void set_focus_on_picker_shape(); ColorPicker(); ~ColorPicker(); From e4b8f5ab794b69e8cf2502f0fc55043a45dd8bd3 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 00:49:48 +0100 Subject: [PATCH 12/20] Use default button focus style for ColorPicker shape focus --- editor/themes/editor_theme_manager.cpp | 2 ++ scene/theme/default_theme.cpp | 2 ++ 2 files changed, 4 insertions(+) diff --git a/editor/themes/editor_theme_manager.cpp b/editor/themes/editor_theme_manager.cpp index 32079f3753fa..e498ccdbf39f 100644 --- a/editor/themes/editor_theme_manager.cpp +++ b/editor/themes/editor_theme_manager.cpp @@ -1751,6 +1751,8 @@ void EditorThemeManager::_populate_standard_styles(const Ref &p_the p_theme->set_constant("label_width", "ColorPicker", 10 * EDSCALE); p_theme->set_constant("center_slider_grabbers", "ColorPicker", 1); + p_theme->set_stylebox("picker_focus", "ColorPicker", p_config.button_style_focus); + p_theme->set_icon("screen_picker", "ColorPicker", p_theme->get_icon(SNAME("ColorPick"), EditorStringName(EditorIcons))); p_theme->set_icon("shape_circle", "ColorPicker", p_theme->get_icon(SNAME("PickerShapeCircle"), EditorStringName(EditorIcons))); p_theme->set_icon("shape_rect", "ColorPicker", p_theme->get_icon(SNAME("PickerShapeRectangle"), EditorStringName(EditorIcons))); diff --git a/scene/theme/default_theme.cpp b/scene/theme/default_theme.cpp index f5065e8de10e..df3889b7fe97 100644 --- a/scene/theme/default_theme.cpp +++ b/scene/theme/default_theme.cpp @@ -1026,6 +1026,8 @@ void fill_default_theme(Ref &theme, const Ref &default_font, const theme->set_constant("label_width", "ColorPicker", Math::round(10 * scale)); theme->set_constant("center_slider_grabbers", "ColorPicker", 1); + theme->set_stylebox("picker_focus", "ColorPicker", focus); + theme->set_icon("folded_arrow", "ColorPicker", icons["arrow_right"]); theme->set_icon("expanded_arrow", "ColorPicker", icons["arrow_down"]); theme->set_icon("screen_picker", "ColorPicker", icons["color_picker_pipette"]); From deaf7a394a8265ef9e042d70a2505873900dcab1 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 19:06:22 +0100 Subject: [PATCH 13/20] Use dedicated control to draw focus stylebox over HSV wheel in ColorPicker --- scene/gui/color_picker.cpp | 45 +++++++++++++++++++++++--------------- scene/gui/color_picker.h | 1 + 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index cebb9f002de2..8f63af8bd7da 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -304,22 +304,26 @@ void ColorPicker::_update_controls() { w_edit->show(); uv_edit->show(); btn_shape->show(); + if (wheel_uv->has_focus()) { uv_edit->grab_focus(); - } else if (wheel_margin->has_focus()) { + } else if (wheel_h_focus_display->has_focus()) { w_edit->grab_focus(); } + wheel_edit->hide(); - wheel_margin->set_focus_mode(FOCUS_NONE); + wheel_h_focus_display->hide(); break; case SHAPE_HSV_WHEEL: wheel_edit->show(); - wheel_margin->set_focus_mode(FOCUS_ALL); + wheel_h_focus_display->show(); + if (w_edit->has_focus()) { - wheel_margin->grab_focus(); + wheel_h_focus_display->grab_focus(); } else if (uv_edit->has_focus()) { wheel_uv->grab_focus(); } + w_edit->hide(); uv_edit->hide(); btn_shape->show(); @@ -328,33 +332,37 @@ void ColorPicker::_update_controls() { case SHAPE_VHS_CIRCLE: wheel_edit->show(); w_edit->show(); + if (uv_edit->has_focus()) { wheel_uv->grab_focus(); - } else if (wheel_margin->has_focus()) { + } else if (wheel_h_focus_display->has_focus()) { w_edit->grab_focus(); } + uv_edit->hide(); btn_shape->show(); wheel->set_material(circle_mat); circle_mat->set_shader(circle_shader); - wheel_margin->set_focus_mode(FOCUS_NONE); + wheel_h_focus_display->hide(); break; case SHAPE_OKHSL_CIRCLE: wheel_edit->show(); w_edit->show(); + if (uv_edit->has_focus()) { wheel_uv->grab_focus(); - } else if (wheel_margin->has_focus()) { + } else if (wheel_h_focus_display->has_focus()) { w_edit->grab_focus(); } + uv_edit->hide(); btn_shape->show(); wheel->set_material(circle_mat); circle_mat->set_shader(circle_ok_color_shader); - wheel_margin->set_focus_mode(FOCUS_NONE); + wheel_h_focus_display->hide(); break; case SHAPE_NONE: - wheel_margin->set_focus_mode(FOCUS_NONE); + wheel_h_focus_display->hide(); wheel_edit->hide(); w_edit->hide(); uv_edit->hide(); @@ -1338,10 +1346,6 @@ void ColorPicker::_hsv_draw(int p_which, Control *c) { if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { circle_mat->set_shader_parameter("v", v); } - } else if (p_which == 3) { - int margin = c->get_theme_constant("margin_bottom"); - focus_rect.set_position(Vector2(-margin, -margin)); - focus_rect.set_size(Vector2(focus_rect.get_size().x+margin*2, focus_rect.get_size().y+margin)); } if (c->has_focus()) { @@ -1382,7 +1386,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { bev->get_position().y < corner_y || bev->get_position().y > c->get_size().y - corner_y) { { real_t dist = center.distance_to(bev->get_position()); - wheel_margin->grab_focus(); + wheel_h_focus_display->grab_focus(); if (dist >= center.x * 0.84 && dist <= center.x) { real_t rad = center.angle_to_point(bev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; @@ -1513,7 +1517,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { if (c == wheel_uv) { s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); - } else if (c == wheel_margin) { + } else if (c == wheel_h_focus_display) { int h_change = 0; if (Math::is_equal_approx(h, 0) || Math::is_equal_approx(h, 0.5f) || Math::is_equal_approx(h, 1)) { @@ -2146,15 +2150,20 @@ ColorPicker::ColorPicker() { wheel_margin = memnew(MarginContainer); wheel_margin->add_theme_constant_override("margin_bottom", 8); wheel_edit->add_child(wheel_margin); - // TODO: Hack - I cannot draw focus stylebox on wheel itself, as it's drawing based on shader - wheel_margin->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(3, wheel_margin)); - wheel_margin->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(wheel_margin)); wheel = memnew(Control); wheel_margin->add_child(wheel); wheel->set_mouse_filter(MOUSE_FILTER_PASS); wheel->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(2, wheel)); + // TODO: Hack - I cannot draw focus stylebox on wheel itself, as it's drawing based on shader + wheel_h_focus_display = memnew(Control); + wheel_margin->add_child(wheel_h_focus_display); + wheel_h_focus_display->set_mouse_filter(MOUSE_FILTER_PASS); + wheel_h_focus_display->set_focus_mode(FOCUS_ALL); + wheel_h_focus_display->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(3, wheel_h_focus_display)); + wheel_h_focus_display->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(wheel_h_focus_display)); + wheel_uv = memnew(Control); wheel_uv->set_name("WHEEL UV"); wheel_margin->add_child(wheel_uv); diff --git a/scene/gui/color_picker.h b/scene/gui/color_picker.h index b9cce332b150..05803d4969ee 100644 --- a/scene/gui/color_picker.h +++ b/scene/gui/color_picker.h @@ -145,6 +145,7 @@ class ColorPicker : public VBoxContainer { Ref circle_mat; Control *wheel = nullptr; Control *wheel_uv = nullptr; + Control *wheel_h_focus_display = nullptr; TextureRect *sample = nullptr; GridContainer *preset_container = nullptr; HBoxContainer *recent_preset_hbc = nullptr; From 1fafafb6cf58359a33b736abefb2fd79855e5fd0 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 22:49:50 +0100 Subject: [PATCH 14/20] Cleanup --- scene/gui/color_picker.cpp | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 8f63af8bd7da..5d0c0233264c 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1474,9 +1474,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { Ref jmev = p_event; if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { - // TODO: It should be done in process instead of input to handle joypads better // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit - // Vector2 color_change_vector = Input::get_singleton()->get_vector("ui_left", "ui_right", "ui_up", "ui_down"); Vector2 color_change_vector = Vector2(); if (p_event->is_action_pressed("ui_left", true)) { color_change_vector.x -= 1; @@ -1491,8 +1489,8 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { if (!Math::is_zero_approx(color_change_vector.length())) { if (actual_shape == SHAPE_HSV_RECTANGLE) { - s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); - v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); + s = CLAMP(s + color_change_vector.x / 100.0, 0, 1); + v = CLAMP(v - color_change_vector.y / 100.0, 0, 1); } else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { Vector2 center = c->get_size() / 2.0; @@ -1515,8 +1513,8 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { } else if (actual_shape == SHAPE_HSV_WHEEL) { if (c == wheel_uv) { - s = CLAMP(s + color_change_vector.x / modes[MODE_HSV]->get_slider_max(1), 0, 1); - v = CLAMP(v - color_change_vector.y / modes[MODE_HSV]->get_slider_max(2), 0, 1); + s = CLAMP(s + color_change_vector.x / 100.0, 0, 1); + v = CLAMP(v - color_change_vector.y / 100.0, 0, 1); } else if (c == wheel_h_focus_display) { int h_change = 0; @@ -1539,9 +1537,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { } h_change = CLAMP(h_change, -1, 1); - // int tmp = ((h * 360) + h_change) % 360; - // h = tmp / 360.0; - h = Math::wrapf(h + h_change / modes[MODE_HSV]->get_slider_max(0), 0, 1); + h = Math::wrapf(h + h_change / 360.0, 0, 1); } } @@ -1618,10 +1614,10 @@ void ColorPicker::_w_input(const Ref &p_event) { float color_change = Input::get_singleton()->get_axis("ui_up", "ui_down"); if (!Math::is_zero_approx(color_change)) { if (actual_shape == SHAPE_HSV_RECTANGLE) { - h = CLAMP(h + color_change / modes[MODE_HSV]->get_slider_max(0), 0, 1); + h = CLAMP(h + color_change / 360.0, 0, 1); } else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { - v = CLAMP(v - color_change / modes[MODE_HSV]->get_slider_max(2), 0, 1); + v = CLAMP(v - color_change / 100.0, 0, 1); } accept_event(); @@ -2013,7 +2009,6 @@ ColorPicker::ColorPicker() { hb_edit->set_v_size_flags(SIZE_SHRINK_BEGIN); uv_edit = memnew(Control); - uv_edit->set_name("UV EDIT"); hb_edit->add_child(uv_edit); uv_edit->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(uv_edit)); uv_edit->set_mouse_filter(MOUSE_FILTER_PASS); @@ -2165,7 +2160,6 @@ ColorPicker::ColorPicker() { wheel_h_focus_display->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(wheel_h_focus_display)); wheel_uv = memnew(Control); - wheel_uv->set_name("WHEEL UV"); wheel_margin->add_child(wheel_uv); wheel_uv->set_focus_mode(FOCUS_ALL); wheel_uv->connect(SceneStringName(gui_input), callable_mp(this, &ColorPicker::_uv_input).bind(wheel_uv)); From 4bd57efaffc0fe230d54e422c25483f7609a78e8 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 23:02:20 +0100 Subject: [PATCH 15/20] Add documentation for ColorPicker's picker_focus theme item --- doc/classes/ColorPicker.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/classes/ColorPicker.xml b/doc/classes/ColorPicker.xml index cf26f917e13e..2413b8770802 100644 --- a/doc/classes/ColorPicker.xml +++ b/doc/classes/ColorPicker.xml @@ -141,6 +141,9 @@ + + [StyleBox] used when the picker is focused. The [theme_item picker_focus] [StyleBox] is displayed [i]over[/i] the picker shape, so a partially transparent [StyleBox] should be used to ensure the picker shape remains visible. A [StyleBox] that represents an outline or an underline works well for this purpose. To disable the focus visual effect, assign a [StyleBoxEmpty] resource. Note that disabling the focus visual effect will harm keyboard/controller navigation usability, so this is not recommended for accessibility reasons. + Overrides the [theme_item Slider.center_grabber] theme property of the sliders. From 8143904d5d6790a25c976a48a344e7837ed0229f Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Sun, 17 Nov 2024 23:32:36 +0100 Subject: [PATCH 16/20] Cleanup --- scene/gui/color_picker.cpp | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 5d0c0233264c..0fe642879e92 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -251,17 +251,15 @@ void ColorPicker::set_focus_on_line_edit() { void ColorPicker::set_focus_on_picker_shape() { switch (_get_actual_shape()) { case SHAPE_HSV_RECTANGLE: - callable_mp(uv_edit, &Control::grab_focus).call_deferred(); - break; + callable_mp(w_edit, &Control::grab_focus).call_deferred(); + break; case SHAPE_HSV_WHEEL: - callable_mp(wheel_uv, &Control::grab_focus).call_deferred(); - break; + callable_mp(wheel_h_focus_display, &Control::grab_focus).call_deferred(); + break; case SHAPE_VHS_CIRCLE: - callable_mp(wheel_uv, &Control::grab_focus).call_deferred(); - break; case SHAPE_OKHSL_CIRCLE: callable_mp(wheel_uv, &Control::grab_focus).call_deferred(); - break; + break; default: { } } @@ -444,7 +442,7 @@ void ColorPicker::_slider_value_changed() { s = sliders[1]->get_value() / 100.0; v = sliders[2]->get_value() / 100.0; - hsv_keyboard_picker_cursor_position = Vector2i(0,0); + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); last_color = color; } @@ -1373,7 +1371,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t rad = center.angle_to_point(bev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; s = CLAMP(dist / center.x, 0, 1); - hsv_keyboard_picker_cursor_position = Vector2i(0,0); + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); } else { return; } @@ -1441,7 +1439,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t rad = center.angle_to_point(mev->get_position()); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; s = CLAMP(dist / center.x, 0, 1); - hsv_keyboard_picker_cursor_position = Vector2i(0,0); + hsv_keyboard_picker_cursor_position = Vector2i(0, 0); } else { if (spinning) { real_t rad = center.angle_to_point(mev->get_position()); @@ -1486,13 +1484,11 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { color_change_vector.y += 1; } - if (!Math::is_zero_approx(color_change_vector.length())) { if (actual_shape == SHAPE_HSV_RECTANGLE) { s = CLAMP(s + color_change_vector.x / 100.0, 0, 1); v = CLAMP(v - color_change_vector.y / 100.0, 0, 1); - } - else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { + } else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { Vector2 center = c->get_size() / 2.0; // TODO: It's a hack, as it messes up if I calculate it this way always @@ -1501,7 +1497,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { hsv_keyboard_picker_cursor_position.y = center.y + (center.y * Math::sin(h * Math_TAU) * s); } - real_t potential_new_cursor_distance = center.distance_to(hsv_keyboard_picker_cursor_position+color_change_vector); + real_t potential_new_cursor_distance = center.distance_to(hsv_keyboard_picker_cursor_position + color_change_vector); if (potential_new_cursor_distance <= center.x) { hsv_keyboard_picker_cursor_position += color_change_vector; } @@ -1510,8 +1506,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { real_t rad = center.angle_to_point(hsv_keyboard_picker_cursor_position); h = ((rad >= 0) ? rad : (Math_TAU + rad)) / Math_TAU; s = CLAMP(dist / center.x, 0, 1); - } - else if (actual_shape == SHAPE_HSV_WHEEL) { + } else if (actual_shape == SHAPE_HSV_WHEEL) { if (c == wheel_uv) { s = CLAMP(s + color_change_vector.x / 100.0, 0, 1); v = CLAMP(v - color_change_vector.y / 100.0, 0, 1); @@ -1546,7 +1541,6 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { last_color = color; set_pick_color(color); - // TODO: Consider handling deffered mode emit_signal(SNAME("color_changed"), color); } } @@ -1615,8 +1609,7 @@ void ColorPicker::_w_input(const Ref &p_event) { if (!Math::is_zero_approx(color_change)) { if (actual_shape == SHAPE_HSV_RECTANGLE) { h = CLAMP(h + color_change / 360.0, 0, 1); - } - else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { + } else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { v = CLAMP(v - color_change / 100.0, 0, 1); } @@ -1625,7 +1618,6 @@ void ColorPicker::_w_input(const Ref &p_event) { last_color = color; set_pick_color(color); - // TODO: Consider handling deffered mode emit_signal(SNAME("color_changed"), color); } } From 95a22d063810c2898c8d964a15c49f2a7ad5ef8e Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Mon, 18 Nov 2024 08:19:24 +0100 Subject: [PATCH 17/20] Fix documentation --- doc/classes/ColorPicker.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/classes/ColorPicker.xml b/doc/classes/ColorPicker.xml index 2413b8770802..aac3686e00d7 100644 --- a/doc/classes/ColorPicker.xml +++ b/doc/classes/ColorPicker.xml @@ -141,9 +141,6 @@ - - [StyleBox] used when the picker is focused. The [theme_item picker_focus] [StyleBox] is displayed [i]over[/i] the picker shape, so a partially transparent [StyleBox] should be used to ensure the picker shape remains visible. A [StyleBox] that represents an outline or an underline works well for this purpose. To disable the focus visual effect, assign a [StyleBoxEmpty] resource. Note that disabling the focus visual effect will harm keyboard/controller navigation usability, so this is not recommended for accessibility reasons. - Overrides the [theme_item Slider.center_grabber] theme property of the sliders. @@ -204,5 +201,8 @@ The icon for rectangular wheel picker shapes. + + [StyleBox] used when the picker is focused. The [theme_item picker_focus] [StyleBox] is displayed [i]over[/i] the picker shape, so a partially transparent [StyleBox] should be used to ensure the picker shape remains visible. A [StyleBox] that represents an outline or an underline works well for this purpose. To disable the focus visual effect, assign a [StyleBoxEmpty] resource. Note that disabling the focus visual effect will harm keyboard/controller navigation usability, so this is not recommended for accessibility reasons. + From c7fddb2d19f8f6af225e2f10219fc4e39fdd445a Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Wed, 20 Nov 2024 17:35:03 +0100 Subject: [PATCH 18/20] Modify comments and simplify code according to review suggestions. --- scene/gui/color_picker.cpp | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 0fe642879e92..582805648e5d 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1472,8 +1472,8 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { Ref jmev = p_event; if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { - // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit - Vector2 color_change_vector = Vector2(); + // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit. + Vector2 color_change_vector; if (p_event->is_action_pressed("ui_left", true)) { color_change_vector.x -= 1; } else if (p_event->is_action_pressed("ui_right", true)) { @@ -1484,15 +1484,15 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { color_change_vector.y += 1; } - if (!Math::is_zero_approx(color_change_vector.length())) { + if (!Math::is_zero_approx(color_change_vector.length_squared())) { if (actual_shape == SHAPE_HSV_RECTANGLE) { s = CLAMP(s + color_change_vector.x / 100.0, 0, 1); v = CLAMP(v - color_change_vector.y / 100.0, 0, 1); } else if (actual_shape == SHAPE_VHS_CIRCLE || actual_shape == SHAPE_OKHSL_CIRCLE) { Vector2 center = c->get_size() / 2.0; - // TODO: It's a hack, as it messes up if I calculate it this way always - if (hsv_keyboard_picker_cursor_position.x == 0 && hsv_keyboard_picker_cursor_position.y == 0) { + // HACK: It's a hack, as it messes up if I calculate it this way always. + if (hsv_keyboard_picker_cursor_position == Vector2i()) { hsv_keyboard_picker_cursor_position.x = center.x + (center.x * Math::cos(h * Math_TAU) * s); hsv_keyboard_picker_cursor_position.y = center.y + (center.y * Math::sin(h * Math_TAU) * s); } @@ -1603,8 +1603,7 @@ void ColorPicker::_w_input(const Ref &p_event) { Ref jmev = p_event; if (kev.is_valid() || jbev.is_valid() || jmev.is_valid()) { - // TODO: It should be done in process instead of input to handle joypads better - // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit + // TODO: Consider adding new ui actions specific to ColorPicker, like the ones used for LineEdit. float color_change = Input::get_singleton()->get_axis("ui_up", "ui_down"); if (!Math::is_zero_approx(color_change)) { if (actual_shape == SHAPE_HSV_RECTANGLE) { @@ -2143,7 +2142,7 @@ ColorPicker::ColorPicker() { wheel->set_mouse_filter(MOUSE_FILTER_PASS); wheel->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(2, wheel)); - // TODO: Hack - I cannot draw focus stylebox on wheel itself, as it's drawing based on shader + // HACK: I cannot draw focus stylebox on wheel itself, as it's drawing based on shader. wheel_h_focus_display = memnew(Control); wheel_margin->add_child(wheel_h_focus_display); wheel_h_focus_display->set_mouse_filter(MOUSE_FILTER_PASS); From 39526a63093f919dbf7d0e143632637ef57a4dc5 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Wed, 20 Nov 2024 23:02:21 +0100 Subject: [PATCH 19/20] Simplify checking if color change vector is zero --- scene/gui/color_picker.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index 582805648e5d..d22d75ac32c7 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1484,7 +1484,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { color_change_vector.y += 1; } - if (!Math::is_zero_approx(color_change_vector.length_squared())) { + if (!color_change_vector.is_zero_approx()) { if (actual_shape == SHAPE_HSV_RECTANGLE) { s = CLAMP(s + color_change_vector.x / 100.0, 0, 1); v = CLAMP(v - color_change_vector.y / 100.0, 0, 1); From b2fef7a0fd35510903d640e753660fb0c0b8faf3 Mon Sep 17 00:00:00 2001 From: Konrad Gadzina Date: Thu, 21 Nov 2024 13:01:03 +0100 Subject: [PATCH 20/20] Apply suggestions from code review Co-authored-by: A Thousand Ships <96648715+AThousandShips@users.noreply.github.com> --- doc/classes/ColorPicker.xml | 2 +- scene/gui/color_picker.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/classes/ColorPicker.xml b/doc/classes/ColorPicker.xml index aac3686e00d7..3c3d78fe59a3 100644 --- a/doc/classes/ColorPicker.xml +++ b/doc/classes/ColorPicker.xml @@ -202,7 +202,7 @@ The icon for rectangular wheel picker shapes. - [StyleBox] used when the picker is focused. The [theme_item picker_focus] [StyleBox] is displayed [i]over[/i] the picker shape, so a partially transparent [StyleBox] should be used to ensure the picker shape remains visible. A [StyleBox] that represents an outline or an underline works well for this purpose. To disable the focus visual effect, assign a [StyleBoxEmpty] resource. Note that disabling the focus visual effect will harm keyboard/controller navigation usability, so this is not recommended for accessibility reasons. + The [StyleBox] used when the picker is focused. Displayed [i]over[/i] the picker shape, so a partially transparent [StyleBox] should be used to ensure the picker shape remains visible. A [StyleBox] that represents an outline or an underline works well for this purpose. To disable the focus visual effect, assign a [StyleBoxEmpty] resource. Note that disabling the focus visual effect will harm keyboard/controller navigation usability, so this is not recommended for accessibility reasons. diff --git a/scene/gui/color_picker.cpp b/scene/gui/color_picker.cpp index d22d75ac32c7..06e6b92ed8e9 100644 --- a/scene/gui/color_picker.cpp +++ b/scene/gui/color_picker.cpp @@ -1513,7 +1513,7 @@ void ColorPicker::_uv_input(const Ref &p_event, Control *c) { } else if (c == wheel_h_focus_display) { int h_change = 0; - if (Math::is_equal_approx(h, 0) || Math::is_equal_approx(h, 0.5f) || Math::is_equal_approx(h, 1)) { + if (Math::is_zero_approx(h) || Math::is_equal_approx(h, 0.5f) || Math::is_equal_approx(h, 1)) { color_change_vector.x = 0; } else if (Math::is_equal_approx(h, 0.25f) || Math::is_equal_approx(h, 0.75f)) { color_change_vector.y = 0; @@ -2142,7 +2142,7 @@ ColorPicker::ColorPicker() { wheel->set_mouse_filter(MOUSE_FILTER_PASS); wheel->connect(SceneStringName(draw), callable_mp(this, &ColorPicker::_hsv_draw).bind(2, wheel)); - // HACK: I cannot draw focus stylebox on wheel itself, as it's drawing based on shader. + // HACK: I cannot draw focus stylebox on the wheel itself, as it's drawing based on shader. wheel_h_focus_display = memnew(Control); wheel_margin->add_child(wheel_h_focus_display); wheel_h_focus_display->set_mouse_filter(MOUSE_FILTER_PASS);