-
-
Notifications
You must be signed in to change notification settings - Fork 20.9k
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
Fix most of TextEdit
visual issues.
#97301
base: master
Are you sure you want to change the base?
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
0840a25
to
7235e38
Compare
I shouldn't have touched this mine field! 😆 Calculations for width are alright but for height are alwrong. |
This comment was marked as resolved.
This comment was marked as resolved.
af2a9ea
to
c517c72
Compare
For this pull request it's enough to respect the scroll bars when they appear and the content margins, also to not draw the readonly style over the normal style. I will try to fix some minimap issues appears on long scripts in another pull request, then at last i will try to do something for RTL issues, I couldn't find any solutions for fixing text clipping because drawing to another child control may break compatibility and will result in a lot of changes for |
a2e2caa
to
8071071
Compare
For the text clipping, see:
From that LineEdit PR, it seems the solution is to have a separate RID to use for the text. The RenderingServer has methods like Also can you update the description? Since it looks like the RTL issues are going to be later. Does this close the proposal too? |
This is what i was searching for, Thank You!
Yes, since it's now drawing only the readonly stylebox when it's not editable. |
c69badd
to
8671283
Compare
8671283
to
950add1
Compare
950add1
to
1f56add
Compare
@MewPurPur Those changes may affect
Note: I have removed any uncontrolled margins to be replaced later with theme I'd like to know if there are any issues that we can consider to make the |
Cool! Thanks for checking. I do have some other grievances with TextEdit but they are unrelated. |
1f56add
to
1cd4eca
Compare
I really hate those unit tests. Just fixed the first/last line scroll offsets to make them fully visible when adjusting the viewport to them and now i have to deal with the tests again :))))))) |
1cd4eca
to
73f55a4
Compare
I just have used floor(text_edit->get_v_scroll()) instead since my tests are broken in my hard fork due to removing a lot of classes.
This is the second fix that have made the tests fails, when scrolling to the first or last line, i have insured that they become fully visible (if they aren't), similar to other text editors. scroll.adjust.mp4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tests/scene/test_text_edit.h
Outdated
@@ -7445,7 +7445,7 @@ TEST_CASE("[SceneTree][TextEdit] viewport") { | |||
text_edit->set_line_as_last_visible(visible_lines * 2); | |||
MessageQueue::get_singleton()->flush(); | |||
CHECK(text_edit->get_first_visible_line() == visible_lines); | |||
CHECK(text_edit->get_v_scroll() == visible_lines); | |||
CHECK(floor(text_edit->get_v_scroll()) == visible_lines); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For most of these you can add the fractional part to visible_lines
. It should be _get_visible_lines_offset()
, but since it isn't exposed, something like:
const float v_offset = 1.0 - ((text_edit->get_size().y - text_edit->get_minimum_size().y) / text_edit->get_line_height() - visible_lines);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have found that (text_edit->get_v_scroll_bar()->get_max() - floor(text_edit->get_v_scroll_bar()->get_max()))
fixes it, also it's similar to the change that have caused the issue set_v_scroll(get_scroll_pos_for_line(first_line, next_line.y) + (v_scroll->get_max() - floor(v_scroll->get_max())));
in void TextEdit::set_line_as_last_visible(int p_line, int p_wrap_index)
.
Fixed. |
Additonally:
Fixes was applied one by one to fix each issue separetly, with a test after each change to make sure the selected issue was fixed before moving to the next one.
Please add this to the 4.4 milestone, since most of the fixes are applied to the DRAW notification.
For testing, also check
ShaderEditor
,ScriptEditor
andCodeEdit
.Thanks in advance.