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

Parley rendering beyond measured bounds #165

Open
jamesthurley opened this issue Nov 9, 2024 · 6 comments
Open

Parley rendering beyond measured bounds #165

jamesthurley opened this issue Nov 9, 2024 · 6 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@jamesthurley
Copy link

I'm using Parley with Taffy to render text within flexbox layouts. However Parley is rendering it's text beyond the bounds given by its computed layout.

This seems like it might be slightly related to issue 119, although I'm already using a larger line height of 1.3 as suggested.

I've cobbled together a simple project that shows the issue by adapting various examples from Parley and Taffy (here, here and here):

The project is here:
https://github.com/jamesthurley/parley-test

First run download_fonts.sh which will fetch the OpenSans font from google's github repo, to ensure we're all looking at exactly the same output, and then you can do cargo run and it will create a file called output.png.

Or, if you just want to use system fonts you can comment out lines 66-71 and line 233.

The example uses Taffy to create a layout with two vertically oriented boxes. The top box will take as much space as required by the text within (using Parley to compute the required size, and to render the text), the second box takes up the remaining space. There is a 10 pixel border to make things clearer.

This is the output image I get:
image

Looking more closely, there seems to be perhaps three visible issues.

First, the text is offset vertically downwards by about 4 pixels (overlapping the red line is fine, going beyond it isn't):

image

Second, the measured pixel height returned by Parley is too short. You see here the rendered text extends down 8 pixels beyond the red box:

image

Third, also visible in the above image, the j extends one pixel too far to the left.

I am certain I have rendered the red box with the correct position and height, as shown by the console output:

Width constraint is: Some(480.0)
Measured text block: width: 481.375, height: 189
TREE
└──  FLEX COL [x: 0    y: 0    w: 500  h: 300  content_w: 491  content_h: 290  border: l:0 r:0 t:0 b:0, padding: l:10 r:10 t:10 b:10] (NodeId(4294967299))
    ├──  LEAF [x: 10   y: 10   w: 480  h: 189  content_w: 481  content_h: 189  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967297))
    └──  LEAF [x: 10   y: 199  w: 480  h: 91   content_w: 0    content_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967298))
Drawing text box at 10,10 with size 480x189
Drawing other box at 10,199 with size 480x91

It's very possible I've made a mistake with how I'm using Parley. Any pointers would be appreciated.

@xorgy
Copy link
Member

xorgy commented Nov 9, 2024

Seems like it may be a matter of adding functions to measure real layout overflow, rather than the ‘layout size’ (total inline advance of the longest line by total of line heights). There might also be use for a layout mode that limits the real overflow bounds, because there is not a simple or efficient way to implement this on top of the current functionality.

And of course, this calls for better documentation, and possibly more descriptive naming, on the existing functions.

@xorgy xorgy added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 9, 2024
@dfrg
Copy link
Collaborator

dfrg commented Nov 10, 2024

Parley probably doesn’t want to be in the business of loading/scaling outlines so computing the full ink rectangle is likely out of scope. I can be convinced otherwise but my current thinking is that the information for doing this is likely already available in the application’s glyph cache and we don’t want to do redundant work.

The overflow of the last line is more concerning because we’re apparently not even including the baseline in the computed height. That said, the roughly 8 pixel overflow is very close to the applied padding of 10px so I wonder if something funky is going on there.

@jamesthurley
Copy link
Author

Thanks for the responses. @dfrg That makes sense that it is out of scope. Based on that I've implemented my own code which measures the full pixel width and height, and also the pixel offset, and this now renders nicely in the box:

image

Interestingly the width constraint seems to have been hit exactly when I measure it manually:

Width constraint is: Some(480.0)
Parley's measured text size: 481.375x189
Manually measured bounds: Some(Rect { left: -2, right: 477, top: 4, bottom: 196 })
Manually measured text size: 480x193
Manually measured text offset: -2x4
TREE
└──  FLEX COL [x: 0    y: 0    w: 500  h: 300  content_w: 490  content_h: 290  border: l:0 r:0 t:0 b:0, padding: l:10 r:10 t:10 b:10] (NodeId(4294967299))
    ├──  LEAF [x: 10   y: 10   w: 480  h: 193  content_w: 480  content_h: 193  border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967297))
    └──  LEAF [x: 10   y: 203  w: 480  h: 87   content_w: 0    content_h: 0    border: l:0 r:0 t:0 b:0, padding: l:0 r:0 t:0 b:0] (NodeId(4294967298))
Drawing text box at 10,10 with size 480x193
Drawing other box at 10,203 with size 480x87

Regarding the overflow of the last line using the built in layout.height(), it persists even if I remove the 10 pixel padding:
image

However, as I am now using my own measuring this isn't a blocker for me. Feel free to close this issue if you think it makes sense.

@nicoburns
Copy link
Contributor

nicoburns commented Nov 11, 2024

I think this bug ought to be left open.

I believe we ought to be able to compute reasonable bounds just using the font metrics, and the fact we are not is simply a bug, quite possibly introduced in #84 which switched us from using font metrics to using font_size * line_height for the line height. It's possible a font-metric based approach won't cover all cases, but it should be a lot better than this.

I'm glad you have found a workaround for the time being.

@dfrg
Copy link
Collaborator

dfrg commented Nov 11, 2024

Agreed with keeping this open. We don’t guarantee ink bounds but we should provide bounds that fully enclose the text based on metrics.

@xorgy
Copy link
Member

xorgy commented Nov 11, 2024

Ink bounds can be important for when you want to put some text in a clipped box or a texture, but the most common use case for this is single-line text in a field, where Parley consumers can generally figure that out (since they are dealing with positioning and horizontal scrolling themselves). After looking at it, yeah it looks like it would be very invasive to do real ink-bounded layouts in Parley, even as a feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants