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

GDI-compatible rotation handling #702

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

rcombs
Copy link
Member

@rcombs rcombs commented Aug 6, 2023

I believe this should fix #701, as well as waddou/libass#95 more broadly (or at least improve the situation there).

Caveats:

  • As far as I can tell, GDI only applies VERT, not VKNA. I'm leaving this be for now until we can test to see what the impact of changing would be.
  • GDI only applies VERT on characters that it deems to be fullwidth; we're applying it to all runs in @fonts. This is probably fine, since it should be quite rare to have a VERT substitution for a halfwidth character as detected by this code.
  • GDI prefers to use the mort table for vertical substitutions over gsub; it looks like harfbuzz has some support for that, but I'm not entirely clear on whether it would apply to our current usage. This seems to only matter for a few very old fonts.
  • GDI makes use of the system code page in a couple places, which we don't want to do here; I attempt to guess the font's encoding instead.
  • This definitely is going to need a decent amount of testing and comparison with vsfilter behavior.

@rcombs rcombs requested a review from astiob August 6, 2023 02:07
@astiob
Copy link
Member

astiob commented Aug 6, 2023

Haven’t had a look at the code yet, but just a thought that popped in my mind after reading the description: GDI’s behaviour may also differ in runs where it detects that complex shaping is necessary and delegates to Uniscribe. (Those situations are probably uncommon in text set in @fonts, though.)

@rcombs rcombs force-pushed the gdi-rotation branch 2 times, most recently from 79a1c7b to b2576b6 Compare August 6, 2023 04:54
libass/ass_shaper.c Outdated Show resolved Hide resolved
This will be used to determine if a glyph should be rotated in an @font
@rcombs
Copy link
Member Author

rcombs commented Aug 6, 2023

Rebased with changes to match the behavior of WinXP instead of NT4; this mainly involves adding more Unicode ranges that are treated as wide. This now fully resolves all of the rotation mismatches in #600, though it still does nothing to address the positioning/metrics errors.

@khaledhosny
Copy link
Contributor

GDI prefers to use the mort table for vertical substitutions over gsub;

Are you sure about that? I’m surprised to learn that GDI has any support for mort table, or is this strictly for vertical rotation only?

@rcombs
Copy link
Member Author

rcombs commented Aug 6, 2023

It's exclusively for rotation, yeah.

@rcombs
Copy link
Member Author

rcombs commented Aug 6, 2023

…this no longer fixes #701, and now I'm not entirely sure how that works in GDI on XP+ at all (as far as I can tell, it treats — as fullwidth, and thus should rotate it?). It improves some metrics stuff from #600 further, though!

I do wonder if there are behavior variances in versions of windows that are newer than what I've analyzed, though.

@astiob
Copy link
Member

astiob commented Aug 6, 2023

as far as I can tell, it treats — as fullwidth, and thus should rotate it?

Er, I’m not sure I’m following you correctly, but — in vertical writing is supposed to be vertical. So, to counteract the expected 90° rotation of the whole text, GDI displays it horizontally in an @-font run. It’s the same thing that happens to other punctuation where it shifts position: the glyph itself is substituted. The font used in #701 is sans-serif, so you don’t see it, but in a serif font, the @-font’s — would actually have different serifs from the normal font’s, but both glyphs would be horizontal.

@astiob
Copy link
Member

astiob commented Aug 6, 2023

Oh, my bad, the symbol in #701 is actually the em dash! I was the one thinking it was the chouon this time.

Still, probably substitutions? The font in question has this glyph in its vert table. And by contrast, MS Gothic (which displays it differently rotated) doesn’t.

@rcombs
Copy link
Member Author

rcombs commented Dec 27, 2023

Okay, I think we're potentially running up against a harfbuzz limitation. The file in #701 has lines entirely composed of em dashes, which have HB_SCRIPT_COMMON. The font's gsub contains this:

    <ScriptList>
      <!-- ScriptCount=1 -->
      <ScriptRecord index="0">
        <ScriptTag value="hani"/>
        <Script>
          <DefaultLangSys>
            <ReqFeatureIndex value="65535"/>
            <!-- FeatureCount=1 -->
            <FeatureIndex index="0" value="0"/>
          </DefaultLangSys>
          <!-- LangSysCount=1 -->
          <LangSysRecord index="0">
            <LangSysTag value="CHN "/>
            <LangSys>
              <ReqFeatureIndex value="65535"/>
              <!-- FeatureCount=1 -->
              <FeatureIndex index="0" value="0"/>
            </LangSys>
          </LangSysRecord>
        </Script>
      </ScriptRecord>
    </ScriptList>
    <FeatureList>
      <!-- FeatureCount=1 -->
      <FeatureRecord index="0">
        <FeatureTag value="vert"/>
        <Feature>
          <!-- LookupCount=1 -->
          <LookupListIndex index="0" value="0"/>
        </Feature>
      </FeatureRecord>
    </FeatureList>

GDI sees this in an @font case, goes "yup, that's a vert in FeatureList", and applies it; the ScriptList is completely ignored. Harfbuzz, however, will only apply the vert when the buffer script is set to hani (which it isn't here, since the line is entirely composed of em dashes).

Telling harfbuzz that our text direction is TTB instead of LTR forces vert to always be on regardless of script, but also seems to break a lot of other stuff in ways that I don't see an obvious way to resolve. Maybe we should be doing that, but maybe harfbuzz should provide a way to set this at the API level? Anyone have any thoughts?

@astiob
Copy link
Member

astiob commented Dec 27, 2023

Telling harfbuzz that our text direction is TTB […] seems to break a lot of other stuff in ways that I don't see an obvious way to resolve.

For reference, could you list some examples? HarfBuzz’s native TTB mode was what I thought was the way forward before this PR.

@rcombs
Copy link
Member Author

rcombs commented Dec 28, 2023

Harfbuzz's TTB mode may well be the answer here, I'm just definitely using it wrong. Here's my current WIP patch: https://gist.github.com/rcombs/69e9e7ec9cea11859855ef255dfe5fb2

This is definitely doing some stuff wrong; it falls apart badly on parts of the sample from #701:
dmg

Here's output with this trivial test patch to use HB_SCRIPT_HAN:

diff --git a/libass/ass_shaper.c b/libass/ass_shaper.c
index f4e7367..74e8e31 100644
--- a/libass/ass_shaper.c
+++ b/libass/ass_shaper.c
@@ -727,7 +727,7 @@ static bool shape_harfbuzz(ASS_Shaper *shaper, GlyphInfo *glyphs, size_t len)
 
         props.direction = FRIBIDI_LEVEL_IS_RTL(level) ?
             HB_DIRECTION_RTL : HB_DIRECTION_LTR;
-        props.script = glyphs[offset].script;
+        props.script = HB_SCRIPT_HAN;
         props.language  = hb_shaper_get_run_language(shaper, props.script);
         hb_buffer_set_segment_properties(buf, &props);
 

dmg

Notably, the em-dash lines show as contiguous lines instead of dashed as in vsfilter; not sure what's going on there. Some of the text might also be rendering wrong since only the font used for the em-dashes was provided. I also had to change the font name to FZBaoSong-Z04, as the Chinese name (方正报宋_GBK) doesn't match in CoreText's search (which is an issue unto itself that we should look into).

@astiob
Copy link
Member

astiob commented Dec 28, 2023

The picture looks like you may just need to move the negation (within the macro) to a different place.

@rcombs
Copy link
Member Author

rcombs commented Dec 28, 2023

Well, here's some new and interesting screwiness: https://gist.github.com/rcombs/c428c575a1f82f6055e3c1e9e8f9398e

dmg

I'm mostly just throwing spaghetti at the wall here, and I'm not sure what to do about these offsets (maybe something about the bearing calculation?). Someone who understands this better than I do should probably take a stab at this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rendering: wrong direction of lines
5 participants