Fix TextServer::shaped_text_get_grapheme_bounds() returning the bounds of the wrong grapheme

[bs-mwoerner]

Fixes #108269.

A text of "abc" produces three glyphs with start/end pairs of (0, 1), (1, 2), and (2, 3), so apparently end is exclusive. This means that this condition for when the correct position is reached leads to an off-by-one error:

godot/servers/text_server.cpp

Line 1635 in 9b22b41

if (glyphs[i].start <= p_pos && glyphs[i].end >= p_pos) {

If p_pos == 1 (we want the bounds of character index 1) and glyph[0].start == 0 and glyph[0].end == 1, then this will already be true for i == 0 and return the position of the first glyph instead of the second.

This is a rather fundamental change for something that's been like this for four years, so maybe there's quite a bit of code out there that compensates for this behavior and would produce incorrect results if we fix it? Then again, just having it return incorrect values forever doesn't feel quite right either.

Edit: Oh, wow, this change fails all the test cases. 😳 Maybe it was actually inteded that way and the better route would be to document that this function and functions that use it use 1-based indexing?

			SEND_GUI_MOUSE_BUTTON_EVENT(text_edit->get_rect_at_line_column(1, 0).get_center(), MouseButton::LEFT, MouseButtonMask::LEFT, Key::NONE);
			// Add (2,0) to bring it past the center point of the grapheme and account for integer division flooring.
			SEND_GUI_MOUSE_MOTION_EVENT(text_edit->get_rect_at_line_column(1, 5).get_center() + Point2i(2, 0), MouseButtonMask::LEFT, Key::NONE);
			CHECK(text_edit->has_selection());
			CHECK(text_edit->get_selected_text() == "for s");

This test case clicks at the center of character 0, drags a bit past the center of character 5, and expects to then have 5 characters selected. Is that what we want? I'm confused.

012345
for se

[kitbdev]

Edit: Oh, wow, this change fails all the test cases. 😳 Maybe it was actually inteded that way and the better route would be to document that this function and functions that use it use 1-based indexing?

Yes, there is compatibility concerns with changing the behavior, so I think it should just be a documentation update.

[bs-mwoerner]

Ah, right, this entire pull request is a duplicate of #82355. Looks like this has been discussed before, so I'll go ahead and close this.