litiengine icon indicating copy to clipboard operation
litiengine copied to clipboard

Sometimes automatic line breaks start at the wrong x-position

Open Hades948 opened this issue 2 years ago • 3 comments

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:

  • GuiComponent width = 730
  • Text align = Align.LEFT
  • Font = https://www.fontspace.com/primitive-font-f5893
  • Font size = 28f

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 avatar Oct 15 '22 19:10 Hades948

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

nightm4re94 avatar Dec 30 '22 21:12 nightm4re94

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).

Hades948 avatar Dec 30 '22 21:12 Hades948

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.

Gamebuster19901 avatar Dec 30 '22 22:12 Gamebuster19901

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:

https://github.com/gurkenlabs/litiengine/blob/84d617a2e556b438f0a526370359c64db398382b/litiengine/src/main/java/de/gurkenlabs/litiengine/Align.java#L93-L95

https://github.com/gurkenlabs/litiengine/blob/84d617a2e556b438f0a526370359c64db398382b/litiengine/src/main/java/de/gurkenlabs/litiengine/Valign.java#L92-L94

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:

https://github.com/gurkenlabs/litiengine/blob/84d617a2e556b438f0a526370359c64db398382b/litiengine/src/test/java/de/gurkenlabs/litiengine/AlignTests.java#L25-L41

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.

hedfol avatar Mar 12 '24 14:03 hedfol

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.

nightm4re94 avatar Mar 16 '24 20:03 nightm4re94

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

hedfol avatar Mar 23 '24 20:03 hedfol