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

Sometimes automatic line breaks start at the wrong x-position #474

Closed
Hades948 opened this issue Oct 15, 2022 · 6 comments
Closed

Sometimes automatic line breaks start at the wrong x-position #474

Hades948 opened this issue Oct 15, 2022 · 6 comments

Comments

@Hades948
Copy link
Contributor

Describe the bug
Sometimes automatic line breaks start at the wrong x-position. For example, if I set the text of a GuiComponent to be "This is a much longer test to test text box wrapping for the NPC text boxes." it looks like this:
image

Clearly not what it's supposed to look like. However, if I change nothing other than the string, it works. For example, if I replace the 'g' in "wrapping" to a period, it looks fine:
image

So, I'm guessing that first line just happened to be juuuust the right width to break something.

In Align.getLocation, the objectWidth > width for the bad string, but < width for the good string.

  • With = 730
  • Good objectWidth = 726
  • Bad objectWidth = 735.5

Because of this, location is negative but doesn't get clamped. I'm guessing that's the issue. Or at least roughly in that area.

To Reproduce
Steps to reproduce the behavior:

Your System:

  • OS: Windows 11
  • LITIENGINE version: 0.5.2
  • Java JDK/JRE version: jdk-18.0.2.1
  • Screen resolution: 3,840 x 2,160
@Hades948 Hades948 added the bug label Oct 15, 2022
@nightm4re94
Copy link
Member

nightm4re94 commented Dec 30, 2022

I can reproduce this, but couldn't pinpoint the source of this issue.
What I can say for sure is that it depends on the font used due to the differences in glyph width:
image

@Hades948
Copy link
Contributor Author

I recently added UI resizing and I've noticed that I can get it to reliably happen for all of my text boxes, just at different sizes. I think the textbox size that it happens at changes for different fonts/strings. I took this GIF with just whatever the default Java font is when you do new Font(null, Font.PLAIN, 28):
litiengine_issue_474
I also recreated this issue with the "Monospaced" font (Using "Monospaced" instead of null in the Font constructor).

@Gamebuster19901
Copy link
Contributor

I remember also having issues with text rendering in java in one of my own projects.

I had to render the text using a GlyphVector to calculate bounds and stuff because it would just be wrong very often, however I've never seen it wrong by that much.

Just from a visual glance, it looks like the text's x-coordinate is being set to -(w*0.5) where w is the width of the line of text being rendered.

@hedfol
Copy link
Contributor

hedfol commented Mar 12, 2024

Hi LITIENGINE community,

Can I make my first contribution by working on this issue?

I discovered the same cause that was already mentioned in a bug description: negative x position is produced by Align.getLocation(width, objectWidth) when a string line width (objectWidth) exceeds available space (width). This affects not just the LEFT but all values of Align. Similar issues occur with Valign enum:

gui-comp-text-overflow png

If these lines are removed:

if (objectWidth > width) {
return location;
}

if (objectHeight > height) {
return location;
}

position gets clamped:

gui-comp-text-overflow-clamp

However, two tests fail:

AlignTests > getLocation_InPoint(): expected: <-1.5> but was: <0.0>
AlignTests > getLocation_OffPoint(): expected: <0.45> but was: <0.0>
Test code:

@Test
void getLocation_OffPoint() {
// act
double offPointLocation = alignObject.getLocation(1.0, 1.1);
// assert
assertEquals(0.45, offPointLocation, 0.001); // 1.0 - 1.1 / 2.0
}
@Test
void getLocation_InPoint() {
// act
double inPointLocation = alignObject.getLocation(1.0, 5.0);
// assert
assertEquals(-1.5, inPointLocation, 0.001); // 1.0 - 5.0 / 2.0
}

Both alignment enums are used not only in GuiComponent.renderText() (related to this issue) but also in PhysicsEngine, CollisionEntity, and direct calls of TextRenderer methods. Is it necessary to separate text and entity alignments? For example: create a dedicated TextAlign enum, or just use a common Align but rename getLocation() to getTextLocation() and add getEntityLocation().

Either way an expected location behavior should be determined for all alignment values.

@nightm4re94
Copy link
Member

Hi @hedfol , thank you for your detailed assessment! We welcome your contribution in this regard. I would neither create separate classes nor methods for this, but an overload to the getLocation method with an additional boolean parameter that lets the user decide which behaviour to choose. Since the current logic seems to be assuming the entity case, we should leave the uses of getLocation in entities untouched and only replace the uses in text scenarios with the new overload.

hedfol added a commit to hedfol/litiengine that referenced this issue Mar 23, 2024
Add getLocation method overload to Align and Valign enums.
Implement an option to prevent the values that are out of bounds.
hedfol added a commit to hedfol/litiengine that referenced this issue Mar 23, 2024
Make getLocation calls in text scenarios use preventOverflow option.
Do not alter the default uses of getLocation for entities.
hedfol added a commit to hedfol/litiengine that referenced this issue Mar 23, 2024
Add basic tests for Align.getLocation with preventOverflow:true.
Create similar tests for Valign.getLocation: default and no-overflow.
@hedfol
Copy link
Contributor

hedfol commented Mar 23, 2024

Thanks @nightm4re94 , that's an easy solution! I've created a PR with suggested changes.

nightm4re94 pushed a commit that referenced this issue Mar 25, 2024
Make getLocation calls in text scenarios use preventOverflow option.
Do not alter the default uses of getLocation for entities.
nightm4re94 pushed a commit that referenced this issue Mar 25, 2024
Add basic tests for Align.getLocation with preventOverflow:true.
Create similar tests for Valign.getLocation: default and no-overflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants