go-text / typesetting

High quality text shaping in pure Go.
Other
88 stars 11 forks source link

Better support for vertical text #124

Closed benoitkugler closed 6 months ago

benoitkugler commented 6 months ago

This PR aims at fixing #111, by supporting "sideways" glyph orientation for vertical text.

It is roughly divided in two parts :

  1. Adding a "sideways" flag in Direction, and implementing "sideways" mode in Shape. A convenience method is provided : GlyphOutline.Sideways to be used by renderers to rotate glyph.
  2. Resolving the preferred orientation base on Unicode rules : this is done in Segmenter.Split

The only change required for users/renderers is thus to check against the Ouput.Direction.IsSideways() boolean, to properly rotate glyphs when rendering.

Happy to read your thoughts !

(@hajimehoshi Maybe you would like to test this change ? The code in ExampleShaper_Shape and drawVGlyphs may be useful to do so)

hajimehoshi commented 6 months ago

Thanks! I tested Monglian and Japanese texts. Fonts are Noto Sans Monglian and M+ 1 Regular.

SetSideways(false): image

SetSideways(true): image

This is just a quick report. I'll try to make a minimal test case if needed.

benoitkugler commented 6 months ago

@hajimehoshi Thank you for the quick tests !

Could you make sure you use Segmenter.Split to properly assign orientation (based on the script) ? If you manually specify SetSideways, you don't benefit from auto-selection.

Also, in sideways mode, it seems you do not use GlyphOutlines.Sideways when drawing ?

hajimehoshi commented 6 months ago

I'll test those tomorrow. Could you tell me what APIs were newly added?

benoitkugler commented 6 months ago

I'll test those tomorrow. Could you tell me what APIs were newly added?

So I've updated the PR description with links towards the new API : Segmenter.Split, Direction.IsSideways, GlyphOutline.Sideways. Hope it is clearer now :)

hajimehoshi commented 6 months ago

// If [yOffset] is zero, the resulting glyph will typically be displayed // under the baseline. To position the glyph over the baseline, pass // the YAdvance as [yOffset] (a positive value to lift the glyph up).

yOffset is float32 while YAdvance is fixed.Int26_6. How should I convert this appropriately?

hajimehoshi commented 6 months ago

I utilized Segment.Split and Glyph.Sideways without Direction.SetSideways, and the result seems much better.

image

The Monglians seems fine! I found that mixing Japanese characters and English characters doesn't work well. In the above figure "あHello, World.あ"'s Y offsets does match. The last あ overlaps the adjacent English sentence. Should I adjust this with HAscent for each outputs?

Thanks!

EDIT: You can test this with

go run github.com/hajimehoshi/ebiten/v2/examples/texti18n@874d48f945b0d0480d2975e2a7ad69120fd949b4

EDIT2: Arabic results was also changed unexpectedly. I'll take a look...

EDIT3: Fixed the Arabic issue at 363a4f853717300361c6c17ac51dc4e513b87aef by reversing the split inputs for RTL texts.

benoitkugler commented 6 months ago

// If [yOffset] is zero, the resulting glyph will typically be displayed // under the baseline. To position the glyph over the baseline, pass // the YAdvance as [yOffset] (a positive value to lift the glyph up).

yOffset is float32 while YAdvance is fixed.Int26_6. How should I convert this appropriately?

Since YAdvance has been scaled by Input.Size (output = font_units Size / face.Upem), and Sideways expects a value in font units, you should pass YAdvance / (Size face.Upem) to convert back to fonts units. More precisely, you could use float32((-YAdvance / Size).Round()) / float32(face.Upem()) which is a bit inaccurate due to Rounding. A better way would be to use a fixed.Int26_6 to float32 conversion :

func toFloat32(a fixed.Int26_6) float32 {
    return float32(a) / 64
}
hajimehoshi commented 6 months ago

A better way would be to use a fixed.Int26_6 to float32 conversion :

OK, I actually did this:

func fixed26_6ToFloat32(x fixed.Int26_6) float32 { 
    return float32(x>>6) + float32(x&((1<<6)-1))/float32(1<<6)
}

EDIT: Your /64 is much better. This worked correctly even with negative numbers.

hajimehoshi commented 6 months ago

Since YAdvance has been scaled by Input.Size (output = font_units * Size / face.Upem), and Sideways expect a value in font units,

So, the simple conversion like YAdvance / 64 doesn't sound correct in terms of unit conversion, but is this actually better...?

hajimehoshi commented 6 months ago

I think I was misunderstanding. Your intention is like this?

fixed26_6ToFloat32(-YAdvance) / Size / float32(face.Upem())
hajimehoshi commented 6 months ago

I'm sorry for spamming, but

Since YAdvance has been scaled by Input.Size (output = font_units Size / face.Upem), and Sideways expect a value in font units, you should pass YAdvance / (Size face.Upem) to convert back to fonts units.

YAdvance is a value in the scaled units, while Sideways expects a value in the font units. So

-YAdvance / Size * face.Upem

seems correct, but am I missing something? With multiplying Upem, the rendering result is broken, but I still don't understand why -YAdvance / Size / face.Upem works (though there is still the overlapping issue). The below result is with multiplying Upem.

image

benoitkugler commented 6 months ago

YAdvance is a value in the scaled units, while Sideways expects a value in the font units. So

-YAdvance / Size * face.Upem

seems correct, but am I missing something?

I agree with this formula. Hum... I'll try to render the glyphs from your example and investigate tomorrow.

benoitkugler commented 6 months ago

@hajimehoshi I've rendered the Mongolian and (a sample of) Japanese texts, and have noticed none of the visual issues you have found.

See the outputs I get :

shape_verts

Could you try and find where your pipeline differs from this one ?

hajimehoshi commented 6 months ago

I'll check this later, but

glyphData.Sideways(float32(-g.YAdvance.Round()) / sizeFactor)

Isn't the value almost 0?

EDIT: NVM, I missed that sizeFactor := float32(out.Size.Round()) / float32(out.Face.Upem())

hajimehoshi commented 6 months ago

image

By NOT inverting Y in segs, I got a mirrored result to the correct result. I think there is likely an issue in my implementation. I'll take a look further.

hajimehoshi commented 6 months ago

Could you try and find where your pipeline differs from this one ?

This test code doesn't use Segmenter's Split, right? If so, I am not sure why my results is different... I am using Segmenter's Split, and without this, all the glyphs are upright.

EDIT: NVM, this used Segmenter. Why did I miss that...

hajimehoshi commented 6 months ago

image

Here is the current result. Thanks to your test code, I could fix some issues in my code. The last thing is Japanese texts are unnaturally shifted due to Glyph.YOffset.

go run github.com/hajimehoshi/ebiten/v2/example/texti18n@a1358c154a6394dbfbb3891b480d8db8cbd8f708
benoitkugler commented 6 months ago

The YOffsets are indeed incorrect: this is actually a bug in harfbuzz (#126). Since harfbuzz already position the glyphs under the origin, this will also change the implementation of "sideways" mode. I'll correct it and ping when its done.

benoitkugler commented 6 months ago

Alright, "sideways" mode should work as well now. I've also added some helper functions Output.{ToFontUnit, FromFontUnit} to make drawing outlines easier.

There is one issue remaining when laying out mixed "sideways" and "upright" : the baseline are not nicely aligned. I think this is in the scope of go-text to properly adjust it, but I'm not sure how hard it is. Is it enough to use the text orientation or do we need more information ? There is a dedicated table in Opentype fonts : https://learn.microsoft.com/en-us/typography/opentype/spec/base which should be helpful. Also, this concerns horizontal text as well, so perhaps we should postpone this task to another PR ?

hajimehoshi commented 6 months ago

image

Now everything seems fine except for the baseline. Thank you very much!

Also, this concerns horizontal text as well, so perhaps we should postpone this task to another PR ?

I agree with you.

benoitkugler commented 6 months ago

Now everything seems fine except for the baseline. Thank you very much!

Also, this concerns horizontal text as well, so perhaps we should postpone this task to another PR ?

I agree with you.

@hajimehoshi Well, thank you very much for all the testing/debugging, complicated by the offset bug..

So @andydotxyz, @whereswaldon I think this PR is ready for review now :)

benoitkugler commented 6 months ago

There is one issue remaining when laying out mixed "sideways" and "upright" : the baseline are not nicely aligned.

So, I've explored a little bit the Opentype BASE table and thought about baseline. I think there are two different concepts :

Implementing the first one is probably desirable, but postponed to another PR.

Implementing the second one is visually more important, and may be done quite simply, as in 582c627. Now, calling Line.AdjustBaseline just before rendering will make 'sideways' text follow the same convention than 'upright', that is roughly placing the baseline in the middle of each glyph.

We may want to fine tune the implementation of Line.AdjustBaseline latter, but it seems to be a good enough start.

@hajimehoshi Could you confirm that using Line.AdjustBaseline visually improves the rendering ?

hajimehoshi commented 6 months ago

@benoitkugler image

Now everything looks fine. Thank you very much!

What I did is to add these lines to expect the side effect of AdjustBaseline. I hope this is correct.

line := shaping.Line{out}
line.AdjustBaseline()

// After that, `out` is directly used and `line` is no longer used.
hajimehoshi commented 6 months ago

image

I found an interesting thing: the baseline varies for an English sentence. In the above figure, the existence of g matters. Is this intended?

hajimehoshi commented 6 months ago

I found an interesting thing: the baseline varies for an English sentence. In the above figure, the existence of g matters. Is this intended?

image

Unfortunately this is an issue as the baseline are not alinged with English texts. In the above figure, I rendered "Hello\nEbitengine\nWorld" with the same line spaces, but the baseline are not alinged as expected.

whereswaldon commented 6 months ago

Perhaps we should align the text not by the ascent/descent of the particular glyphs within it, but by the face's advertised ascent/descent? That should be independent of specific glyph contents, and it should look decent.

whereswaldon commented 6 months ago

I found an interesting thing: the baseline varies for an English sentence. In the above figure, the existence of g matters. Is this intended?

image

Unfortunately this is an issue as the baseline are not alinged with English texts. In the above figure, I rendered "Hello\nEbitengine\nWorld" with the same line spaces, but the baseline are not alinged as expected.

This particular issue is more of a line-spacing/line-height problem. The typesetting repo doesn't currently implement this part of the text display process internally. It's up to applications to decide where to start the baseline of each line of text. See the discussion here for some examples of the complexities inherent in this space: https://github.com/gioui/gio/pull/123

hajimehoshi commented 6 months ago

Perhaps we should align the text not by the ascent/descent of the particular glyphs within it, but by the face's advertised ascent/descent? That should be independent of specific glyph contents, and it should look decent.

I agree with you. A face's metrics should be used rather than each glyph's metrics.

It's up to applications to decide where to start the baseline of each line of text.

OK, but if an application moves the dot position for each line with a constant interval, each line should be aligned, right?

benoitkugler commented 6 months ago

OK, but if an application moves the dot position for each line with a constant interval, each line should be aligned, right?

Well, in theory, the shaper (harfbuzz) could decide to move glyphs up (for instance if the font provides a positioning table), so I'm not sure this promise holds. Also, consider lines with different text sizes, or even with different fonts.

benoitkugler commented 6 months ago

This is what I get using the font line extents (left) vs glyph extents (right). It seems to me the left version still feels a bit akward..

shape_vert_mixed

hajimehoshi commented 6 months ago

Also, consider lines with different text sizes, or even with different fonts.

Yeah, but in my example, the same size and face were used. I'm OK even if lines are not aligned when multiple faces or sizes are used.

hajimehoshi commented 6 months ago

It seems to me the left version still feels a bit akward..

In Chrome browsers, the baseline seems the same as the right. 'g' is put so that the glyph is extended to the left side. I think this makes sense.

image

whereswaldon commented 6 months ago

OK, but if an application moves the dot position for each line with a constant interval, each line should be aligned, right?

Well, in theory, the shaper (harfbuzz) could decide to move glyphs up (for instance if the font provides a positioning table), so I'm not sure this promise holds. Also, consider lines with different text sizes, or even with different fonts.

I think conceptually it's correct to shift the gap between dots by a fixed value. It's true that situationally the glyphs may look misaligned if the shaper did something fancy, but that would be a feature, not a bug.

whereswaldon commented 6 months ago

It seems to me the left version still feels a bit akward..

In Chrome browsers, the baseline seems the same as the right. 'g' is put so that the glyph is extended to the left side. I think this makes sense.

image

To achieve this, would we shift the glyphs by half the distance from the face's baseline to its ascent? I'd expect that to visually center the above-the-baseline portion of the text. I don't know for sure if this would be correct for all scripts, but it certainly would be for Latin and CJK.

benoitkugler commented 6 months ago

To achieve this, would we shift the glyphs by half the distance from the face's baseline to its ascent? I'd expect that to visually center the above-the-baseline portion of the text. I don't know for sure if this would be correct for all scripts, but it certainly would be for Latin and CJK.

Hum, I've just tried that and it seems to move the glyphs a bit too much down. I think the proper way to go would be to align the baseline : ideographic and roman. So we would detect what baseline to use based on the script and align them.

However, it seems to me this does not solve the "only english" example. Maybe, in this case we could shift by the font advance as @whereswaldon suggested earlier.

benoitkugler commented 6 months ago

Well, what do you think of handling the baseline issue in a separate PR ? This one is already large and it seems we need more discussion on how to do it anyway.

whereswaldon commented 6 months ago

Well, what do you think of handling the baseline issue in a separate PR ? This one is already large and it seems we need more discussion on how to do it anyway.

Fine by me

hajimehoshi commented 6 months ago

Well, what do you think of handling the baseline issue in a separate PR ? This one is already large and it seems we need more discussion on how to do it anyway.

+1. I'm fine with this issue addressed in another PR. I really appreciate your team's hard work!

hajimehoshi commented 6 months ago

@andydotxyz friendly ping