Opened 13 years ago

Closed 11 years ago

Last modified 8 years ago

#723 closed defect (fixed)

[PATCH] Multiline centered text

Reported by: fcxSanya Owned by: kingadami
Priority: Should Have Milestone: Alpha 13
Component: Core engine Keywords: patch
Cc: Patch:

Description

Currently text centered by the first line position, so if there is multiple lines, each line will have the same indention as the first one instead of be centered individually.

Attachments (3)

Multiline_centered_text.diff (16.6 KB ) - added by fcxSanya 13 years ago.
multiline-center-2.patch (1.9 KB ) - added by Philip Taylor 13 years ago.
alternative approach (incomplete)
0adTicket_723.patch (5.4 KB ) - added by kingadami 11 years ago.
Finished Philips patch by adding support for left and right aligned text.

Download all attachments as: .zip

Change History (14)

by fcxSanya, 13 years ago

comment:1 by fcxSanya, 13 years ago

Keywords: review added

Attached patch should fix this issue and should allow to properly display multiline centered text.

comment:2 by Philip Taylor, 13 years ago

I get some minor compiler errors:

../../../source/gui/IGUITextOwner.cpp:123: error: ‘>>’ should be ‘> >’ within a nested template argument list
../../../source/gui/CGUI.cpp:977: error: no matching function for call to ‘min(unsigned int&, size_t)’

On the main menu, if I click the WFG logo in the bottom-left corner it pops up an "About 0 A.D." box. The "0" icon in the centered title bar is incorrectly aligned - presumably something is wrong with the sprite code but I can't tell what...

This seems like a pretty large change to the code - is it really all necessary just to get centered text? I've tried hacking up a much simpler patch to CGUI::DrawText to center all text, which looks like it basically works. Am I missing problems with this approach? (That patch needs some more work to support left-aligned and right-aligned text but that doesn't look hard.)

by Philip Taylor, 13 years ago

Attachment: multiline-center-2.patch added

alternative approach (incomplete)

comment:3 by historic_bruno, 13 years ago

Keywords: review removed
Summary: Multiline centered text[PATCH] Multiline centered text

comment:4 by kingadami, 11 years ago

Owner: set to kingadami
Status: newassigned

comment:5 by kingadami, 11 years ago

Keywords: patch review added

comment:6 by kingadami, 11 years ago

Defect #1386 is a duplicate of this one. I like Philip's approach better than mine while I was looking into 1386 before I found this one.

My alternate approach was to just change the IGUITextOwner::CalculateTextPosition method. This method involved looping through all of the text calls again (which I didn't like) to find each line and computing the offset.

I attached patch 0adTicket_723.patch which completes Philip's patch for right and left aligned text.  The "text_align" property was added to tooltips as well since they would produce a warning because they use the GenerateText method as well.  The default is left align which makes them look like they always have.

Last edited 11 years ago by historic_bruno (previous) (diff)

comment:7 by historic_bruno, 11 years ago

Milestone: BacklogAlpha 13

I tested the patch, it seems to fix the alignment issues as described. But I noticed one bug: dropdown text is now centered instead of left-aligned as it was before.


Now what would be even nicer is if text that's too long to fit on a single line (without wrap points) would automatically wrap to the next line instead of disappearing out of the control. Here's an example to try:

messageBox(500, 300,
            "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 abcdefghijklmnopqrstuvwxyz0123456789",
            "Test", 2);

The first chunk is very long, doesn't wrap and will extend beyond the bounds of the text box. It seems hyphens and spaces can trigger a wraparound but nothing else, so e.g. long paths without those characters will probably not fit in a text box. This is also what #1386 was describing.

It's not always practical to trim the text before display either, since the script logic doesn't know the font being used, how wide the characters will be when rendered, etc.

Last edited 11 years ago by historic_bruno (previous) (diff)

in reply to:  7 comment:8 by kingadami, 11 years ago

Replying to historic_bruno:

I tested the patch, it seems to fix the alignment issues as described. But I noticed one bug: dropdown text is now centered instead of left-aligned as it was before.


Now what would be even nicer is if text that's too long to fit on a single line (without wrap points) would automatically wrap to the next line instead of disappearing out of the control. Here's an example to try:

messageBox(500, 300,
            "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789 abcdefghijklmnopqrstuvwxyz0123456789",
            "Test", 2);

The first chunk is very long, doesn't wrap and will extend beyond the bounds of the text box. It seems hyphens and spaces can trigger a wraparound but nothing else, so e.g. long paths without those characters will probably not fit in a text box. This is also what #1386 was describing.

It's not always practical to trim the text before display either, since the script logic doesn't know the font being used, how wide the characters will be when rendered, etc.

I looked into why the dropdown text is centered now, and it is because the style it uses StoneDropDown has a text_align property set to "center". Look in the file binaries/data/mods/pub/gui/common/common_styles.xml:247. I guess there must have been some kind of existing bug causing it to not center before. Let me know if you want to changed the style to left align.

I will work with your second example and add logic to wrap really long strings without hyphens or spaces as well.

Thanks for the feedback.

by kingadami, 11 years ago

Attachment: 0adTicket_723.patch added

Finished Philips patch by adding support for left and right aligned text.

comment:9 by kingadami, 11 years ago

Updated the patch after talking with historic_bruno on IRC

comment:10 by ben, 11 years ago

Resolution: fixed
Status: assignedclosed

In 13051:

Fixes multiline text alignment, fixes #723. Patch by kingadami.
Adds text alignment to tooltips, fixes text alignment for dropdowns.

comment:11 by sanderd17, 8 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.