Align baseline to pixel boundary in examples and tests. #343
Conversation
examples/swash_render/src/main.rs
Outdated
| // Round the baseline to pixel boundary. | ||
| // An even better renderer would account for fractional DPI scale. | ||
| // To get the baseline we need to calculate the bottom edge of the box. |
There was a problem hiding this comment.
Aren't these metrics already in physical pixels?
There was a problem hiding this comment.
They are, but in f32 with fractional bits. So it'll get spread across multiple device pixels unless we round to where the device pixel boundary is.
| /// ```ignore | ||
| /// y = (y + height).round_to_pixel_boundary() - height | ||
| /// ``` |
There was a problem hiding this comment.
Could be we be explicit here that this is because y is the baseline but it is the top of the box that should be pixel-aligned to achieve crisp rendering?
(we may will need to change the semantics of y when we introduce other (non-baseline) alignment modes)
There was a problem hiding this comment.
The goal is to align it to the baseline of the line, not to align the top of the box. So imagine a box with a fractional height. We want the smudge to happen at the top, not at baseline.
This whole dance isn't great, I agree. I think an API change is probably worth it, but not really what I wanted to get into right now as that is a larger task and I want a release out within a week or so.
There was a problem hiding this comment.
Ok, so I actually had it backwards in my previous comment? y here is the top of the box and we're trying to snap the baseline?
Thinking about this, I'm not entirely convinced this is the right approach for boxes. We probably actually need to round both edges independently? But seeing as this is example code it doesn't matter too much.
There was a problem hiding this comment.
Yeah the PositionedInlineBox has x-y as top-left. So you need to actually reverse the calculation that was made to get y in order to get the original baseline, then round it, and then calculate the new y. Not ergonomic in the least.
I'm also not super convinced that Parley shouldn't do all this rounding automatically internally. I know #297 argued otherwise and it was accepted, but I'm still trying to fully understand the motivation. Even if it's valid, an optional way to get non-rounded results would probably be more ergonomic for most.
There was a problem hiding this comment.
Yeah the PositionedInlineBox has x-y as top-left.
I ought to have known that, because I think it was me who made it that way!
I'm also not super convinced that Parley shouldn't do all this rounding automatically internally. I know #297 argued otherwise and it was accepted, but I'm still trying to fully understand the motivation. Even if it's valid, an optional way to get non-rounded results would probably be more ergonomic for most.
Taffy has rounding as a config option which may be the way to go here.
One thing which might make it complicated for Parley to do the rounding is that the parley Layout might itself be positioned at a fractional offset. If so, then Parley wouldn't have sufficient information to round to exact pixels (rounding within the parley coordinate space would be wrong).
There was a problem hiding this comment.
Yeah but we could document that Parley assumes that its 0,0 is actually aligned - and potentially get even fancier and provide an optional fractional offset option to inform Parley of the alignment.
|
Regarding fractional DPI scaling, I've mostly punted on thinking about it, but let me spend a bit of time looking into it and see if I can improve some of the messaging around it. |
|
I removed the messaging around fractional DPI scaling. This was originally there because one of the key motivations of #297 was that scaling would be done outside of Parley. However I think we can consider that an advanced tactic and so the officially recommended Parley approach should continue to be to pass Parley the scale factor and let Parley do it. In which case doing a simple |
|
I'll let this sit and not merge just yet. I'll play a bit with an alternative API approach that might allow us to avoid a lot of this dance. |
There was a problem hiding this comment.
My main question is how this interacts with #326. My understanding was that the solution you want most of the time is to make sure the first baseline is on a pixel boundary, then always make following baselines be integers. It does mean that your "error" from the "ideal" line height is always increasing, but since text will be read locally, that error doesn't matter.
See e.g. the discussion in #297 (comment)
That is to say, if this is implemented correctly (which I've not done any validation of), then it's an improvement, but not necessarily the best final state
There was a problem hiding this comment.
I'm surprised that any change is needed in vello_editor; the hinting implementation inside Vello should be doing something similar already (the slight wrinkle there is that it's conceivable that you get some glyphs which jump up/down a pixel?). If not, that's a bug in Vello, I think.
Does this actually change rendering here?
There was a problem hiding this comment.
Yes it does change rendering. See my comment in #344 for a GIF of benefits in action. Like I said in that comment though, that's with Vello 4.0. I haven't tested with Vello main yet.
|
I'll let someone else implement this later, or do it myself if the egui work ever needs to get done and is blocked on this. Seems people have still not come to an agreement on the best API surface. |
What do you mean by "this" - #326? If so, sure, that should get implemented and I might do it myself then. Not sure why you would close this examples/tests PR here because of that though. Doesn't matter too much I guess, because like I said I think #344 is the path forward. A bit puzzling, though. |
|
Oops. Yeah, my bad. I meant #326. I'm really sorry. |
|
Oh, just a mistake. Well, nothing to worry about. Makes things much clearer. 😄 |
## Background Vertical metrics are no longer rounded since #297 and so the rounding needs to happen in the renderer. The upside is that there's potential for more control, the downside is that it's another footgun. I initially created documentation and updated examples in #343 but after going through that I came to the conclusion that asking users to do that will be a major inconvenience, and likely will lead to incorrect results. ## Solution This PR here takes a different approach. We bring back quantization but make it optional. That way advanced users who know what they're doing can still get access to the raw fractions. However, for most users we can recommend just using our quantization which will automatically take care of the blurry text issue. The examples will also use the much simpler quantization option. ## Implementation details Now as pointed out in #297 our old quantization strategy was flawed where rounding errors accumulated and weren't dealt with. I didn't want to just bring that back so instead I played around with Chrome a bunch to see how it does line height handling. This PR now contains a solution that mimics Chrome. We now only round `baseline`, `min_coord`, and `max_coord` in the exported metrics. There's more rounding internally, but only temporarily to calculate the aforementioned. So e.g. exported `ascent` will still be the raw fractional. For the line's `y` position, when the accumulated overflow reaches over half a pixel we will either overflow to the previous line or start with a gap, depending on the direction of the accumulation. ## Examples As seen in the example below (*16.0px font size, 1.3em line height (20.8px)*), we now match Chrome's behavior of overflowing the **1st, 4th, 9th, and 14th** line over the previous line. Best spotted by looking at the edges of the red/green selection boxes. Also note that despite the perceived line height fluctuating between lines due positional changes, the rendered line height does not change. Easy to see via the selection boxes, which are always the same height. (*They're colored spans in Chrome, but colored spans are the same height as selection boxes until line height is greater than ascent + descent, which is not the case in this example - making it a valid comparison.*)  Here's another example (*16.0px font size, 1.33125em line height (21.3px)*) showing a scenario where gaps are formed due to the line height rounding going in the other direction. Also the line height is greater than ascent + descent, so the gaps are actually visible with selection boxes. | Chrome | Swash | | --- | --- | |  |  | Here's a bunch more examples: | Notes | Chrome | Swash | | --- | --- | --- | | Mixing of fonts and sizes, 1.99em line height |  |  | | 16px 0.83em (13.28px) |  |  | | 19px 0.83em (13.28px) |  |  | ## Mismatches However, it's not always a perfect match. Fractional font sizes are rendered at a different size, which probably causes the layout changes seen in the 18.5px example. Also, when going to very small line heights, starting from around 0.65em Chrome has some additional logic to keep perceived line height higher. I'm not exactly sure what it is, but it accumulates by the time you get to tiny line heights like 0.25em. It's not related to the rounding of the line height, because in the 0.25em case the line height is exactly 4px. Anyway, not super important right now, but might be worth figuring out later. | Notes | Chrome | Swash | | --- | --- | --- | | 18.5px 0.83em (13.28px) |  |  | | 16px 0.65em (10.4px) |  |  | | 16px 0.25em (4px) |  |  | --- Alternative to and closes #343.
Vertical metrics are no longer rounded since #297 and so the rounding needs to happen in the renderer. The upside is that there's potential for more control, the downside is that it's another footgun. I added a bit more documentation to communicate this, plus I updated all the examples with rounding. I also updated the test renderer, which is essentially a copy of the Tiny Skia example.
The example render results are significantly improved as a result. Crisper and no more cut off text with Swash.
Swash
Tiny Skia