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

Fix Label's shadow and outline overlapping #87914

Closed
wants to merge 1 commit into from

Conversation

WhalesState
Copy link
Contributor

@WhalesState WhalesState commented Feb 3, 2024

Fixes: #35961

Before.
image

After.
image

@WhalesState WhalesState requested a review from a team as a code owner February 3, 2024 20:30
@AThousandShips AThousandShips changed the title fix Label's shadow and outline overlapping Fix Label's shadow and outline overlapping Feb 3, 2024
@AThousandShips AThousandShips added this to the 4.x milestone Feb 3, 2024
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.

Instead of having 3 nearly identical loops, it probably worth moving all the code (ellipsis and draw parts) to the helper function and calling it for the shadow, outline, and text.

scene/gui/label.cpp Show resolved Hide resolved
@WhalesState
Copy link
Contributor Author

WhalesState commented Feb 3, 2024

Instead of having 3 nearly identical loops, it probably worth moving all the code (ellipsis and draw parts) to the helper function and calling it for the shadow, outline, and text.

I'm sorry if it's not the best, moving them to a helper function will be complicated for me, I may need some help.

The three of them are calling three different functions and running under different conditions and also changing three different variables and calling x advance at the end for 2 different variables, a helper function will require more checks and will take so many arguments and will run much more conditions for each loop to do the same.

The only code which is identical is this :

        if (trim_pos >= 0) {
	        if (rtl) {
		        if (j < trim_pos) {
			        continue;
		        }
	        } else {
		        if (j >= trim_pos) {
			        break;
		        }
	        }
        }

The only way to prevent repeating is to loop once without drawing and to save the glyphs that should draw to an array, then looping over it three times in order. [shadow] -> [outline] -> [main text], this depends on the cost of constructing a new Glyph in every loop inside draw function or creating a Vector to store each glyph separately in an array with it's own structure. we better draw them after looping over all lines, to insure that a line's shadow never overlaps over previous lines.

@WhalesState
Copy link
Contributor Author

This was already fixed

@WhalesState WhalesState deleted the gui-label branch September 12, 2024 04:03
@YeldhamDev YeldhamDev removed this from the 4.x milestone Sep 12, 2024
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.

Outlines go around shadows on (non-RichText) Labels
4 participants