Skip to content

Slider: Add bottom and top ticks and tick offset #103907

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

Merged
merged 1 commit into from
Jun 11, 2025

Conversation

beicause
Copy link
Contributor

@beicause beicause commented Mar 10, 2025

Fixes #103839.

Before:

Edit: This PR adds tick_position and tick_offset to allows the ticks to be placed at the top, bottom, or center with custom offset.

@Calinou
Copy link
Member

Calinou commented Mar 10, 2025

This will affect existing projects (albeit not in major ways), so it would be best to make it configurable with a Tick Offset theme constant.

@beicause beicause force-pushed the slider-draw-tick-center branch from 5cd050b to 3e35d83 Compare March 10, 2025 18:40
@beicause beicause requested a review from a team as a code owner March 10, 2025 18:40
@AThousandShips
Copy link
Member

The offset default should be such that it preserves the original behaviour, and allow you to change it to compensate, or this will still break compatibility

@beicause
Copy link
Contributor Author

If we accept tick is drawn at the center, the original behavior still can't be guaranteed by the default value of tick offset, because the widget_width (the size of the slider stylebox) may be modified by users.

@bruvzg
Copy link
Member

bruvzg commented Mar 11, 2025

Both before and after are pretty strange UI choices, sliders usually have either no ticks and round grabber, or offsets ticks on the one or both sides and arrow/rectangular grabber.

It might be worth adding separate top/bottom ticks and offset and length theme constants for both to allow full customization.

@beicause
Copy link
Contributor Author

I don't think top bottom length are necessary. To customize the size&color of ticks, the simplest way is to use GradientTexture2D as the icon.

@KoBeWi
Copy link
Member

KoBeWi commented Mar 18, 2025

For reference, here's how ticks appear in Windows dialogs:
image

And in Chromium:
image

If we accept tick is drawn at the center, the original behavior still can't be guaranteed by the default value of tick offset, because the widget_width (the size of the slider stylebox) may be modified by users.

You can at least guarantee it for default theme.

@beicause beicause force-pushed the slider-draw-tick-center branch from 3e35d83 to d06a1ee Compare March 18, 2025 15:33
@beicause beicause requested a review from a team as a code owner March 18, 2025 15:33
@beicause
Copy link
Contributor Author

After consideration, I think adding top and bottom ticks makes sense, but the alignment should be changed to the upper parts of slider for top tick and lower parts for bottom tick.
(now maybe I should change the title of the pr :)
屏幕截图_20250318_233223

@beicause beicause changed the title Slider: Draw ticks in the center Slider: Add bottom and top ticks and tick offset Mar 18, 2025
@beicause beicause force-pushed the slider-draw-tick-center branch 2 times, most recently from 03f8759 to 3c89baf Compare March 18, 2025 16:39
@KoBeWi
Copy link
Member

KoBeWi commented Mar 18, 2025

Is there any use in having style for both top and bottom tick? All examples of double ticks I found are symmetrical, so you can just repeat the tick icon (at most it could be mirrored maybe?). Likewise, the offset can be single value for both sides. There could be a property that controls whether bottom, top or both ticks appear.

@beicause beicause force-pushed the slider-draw-tick-center branch from 3c89baf to 5bed503 Compare March 18, 2025 17:47
@Charpurrr
Copy link

personally id still really like a way to have them run through the center, just for the customizability (and stylisation) option. could integrating the original pr as a toggle or similar be considered? :3

@beicause
Copy link
Contributor Author

It's OK to add a "center" option. And we can just keep one tick_offset instead of having a separate tick_offset for "top", "bottom" and "center" then.

@beicause beicause force-pushed the slider-draw-tick-center branch from 5bed503 to 4b0872c Compare March 19, 2025 04:45
Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

Looks good. Compatibility breakage could be avoided with proper defaults, but not sure if it's worth it; it would require some awkward values. The new ticks look better IMO and they are similar enough.

@beicause beicause force-pushed the slider-draw-tick-center branch from 4b0872c to 3366682 Compare March 24, 2025 05:56
@beicause beicause force-pushed the slider-draw-tick-center branch from 3366682 to e5dcf1a Compare March 24, 2025 10:53
@beicause
Copy link
Contributor Author

For the top or left ticks, I used  draw_polygon  instead to make them symmetrical with the other side.

@KoBeWi
Copy link
Member

KoBeWi commented Mar 24, 2025

You can use draw_rect() instead and provide rectangle with negative size. It mirrors the texture.

@beicause beicause force-pushed the slider-draw-tick-center branch 2 times, most recently from 24ae709 to 299e1de Compare March 24, 2025 13:16
@beicause beicause force-pushed the slider-draw-tick-center branch from 299e1de to 808aa5a Compare March 31, 2025 12:07
@KoBeWi KoBeWi removed the discussion label Mar 31, 2025
@beicause beicause force-pushed the slider-draw-tick-center branch from 808aa5a to 6220074 Compare April 3, 2025 16:41
@beicause

This comment was marked as resolved.

Allow ticks to be placed at the top, bottom, or center with custom offset.
@beicause beicause force-pushed the slider-draw-tick-center branch 2 times, most recently from 05e79c3 to 257c6eb Compare June 9, 2025 04:36
@beicause
Copy link
Contributor Author

beicause commented Jun 9, 2025

Added TICK_POSITION_ prefix to the enum to follow naming style.

@akien-mga akien-mga requested review from KoBeWi and bruvzg June 10, 2025 11:56
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Seems to be working as expected.

@KoBeWi KoBeWi modified the milestones: 4.x, 4.5 Jun 11, 2025
@akien-mga akien-mga merged commit 986cc40 into godotengine:master Jun 11, 2025
20 checks passed
@akien-mga
Copy link
Member

Thanks!

@beicause beicause deleted the slider-draw-tick-center branch June 12, 2025 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HSlider ticks offset strangely
7 participants