Neos-Metaverse / NeosPublic

A public issue/wiki only repository for the NeosVR project
197 stars 9 forks source link

Particular Japanese character not rendering properly with default font #621

Open AMonsoonofBabies opened 4 years ago

AMonsoonofBabies commented 4 years ago

When using a text component to render text with the default typeface/font, the う (u) character is rendered improperly.

Steps to replicate:

  1. With Neos open, create a new world
  2. Using a developer tooltip, create a basic or outline text object
  3. Replace the text of the object with any string that includes the う character (typically by copy-paste)

Replacing the standard font with a different one fixes this issue, and the character is rendered correctly as per the new font. I checked all the other hiragana and katakana characters (the two basic alphabets) for artifacts of this kind and found none, so I believe it is just this one. 1

shiftyscales commented 4 years ago

It appears to be an issue in the text renderer. Turning up the FaceSoftness helps better visualize it. 2020-05-30 20 45 32 Adjusting various properties like the GlyphEmSize, PixelRange, Padding, etc. did not resolve it.

Frooxius commented 4 years ago

Hmm looks like some edge case in the MSDF edge coloring mechanism. I'd probably have to update the msdfgen to the latest version, since the library recently received updates, but because I have to merge them with our modifications and wrapper it's not as trivial. https://github.com/Chlumsky/msdfgen

Is this causing any significant problems right now? If not I'll have to set this as low priority.

AMonsoonofBabies commented 4 years ago

It's a relatively common character in Japanese, one of the base vowels, and is more prominent in informal speech and specific grammatical structures. I imagine it would be similar to having G or some other relatively common letter having a rendering issue. With that said, obviously Japanese and English have different ways of writing; I was initially checking to see if this issue was present in currently published Japanese worlds, but I could not find any cases, probably because they simply avoid using the character by using kanji where applicable, or some other workaround. It's a strange situation, and unfortunately it would best be answered by someone who regularly uses the written language, as I do not. At least if it's necessary, we can always use a different font, which is honestly a great way to get around it. I'll say it's up to you.

shiftyscales commented 3 years ago

Hmm looks like some edge case in the MSDF edge coloring mechanism. I'd probably have to update the msdfgen to the latest version, since the library recently received updates, but because I have to merge them with our modifications and wrapper it's not as trivial. https://github.com/Chlumsky/msdfgen

Is this causing any significant problems right now? If not I'll have to set this as low priority.

Hey @Frooxius - Rabbuttz(あむ)# 9966 had given me more context about this issue, in line with what @AMonsoonofBabies reported above. They noted it is a very commonly used character in Japanese, and that reportedly a significant portion of the Japanese community would like to see it fixed.

Psychpsyo commented 3 years ago

I just noticed that ゔ, which is exactly the same character with added ゛, renders properly in Neos. Might be helpful to narrow down what is happening as the problem area in both of these should be identical in the font.

Edit: The small う (ぅ) also works fine, being exactly the same character, just scaled down.

Here's an image: uvu

rabbuttz commented 2 years ago

This issue was reported over a year ago. I think it is time for this issue to be resolved. This issue is not fatal, but it is a very big problem for Japanese. This letter is used very often, it's like the letter A in the alphabet. This is like labeling the letter A as ∀. A lot of Japanese people really want this problem to be fixed.

Earthmark commented 2 years ago

After investigating I think I found some leads on the problematic behavior:

Example Code

Visual artifact found when rendering the problem character using CodeX.FontX.RenderGlyphMSDF using the following code, anyone should be able to run this code.

static void Main()
{
  var font = CodeX.FontX.Load(@"<A path pointing to a download of https://assets.neos.com/assets/bcda0bcc22bab28ea4fedae800bfbf9ec76d71cc3b9f851779a35b7e438a839d with .otf as the extension>");
  var id = font.CharToGlyphId('う'); // Symbol 0x3046
  var bitmap = new CodeX.Bitmap2D(128, 128, CodeX.TextureFormat.RGBA32, false);
  font.RenderGlyphMSDF(id, bitmap, new BaseX.Rect(0, 0, 128, 128), 2, false);
  bitmap.Save("texture.png");
}

MSDF Generation seems to have an issue

When run with the current neos font chain font, this MSDF is generated: texture

However, when a clean version of noto japanese is used, this MSDF is generated: texture

I have not yet confirmed that the line artifacts are the culprit, however they do not appear in symbol 0x3045 (ぅ), only in 0x3046 (う), here is 0x3045 under the same conditions: texture

The neos fork of msdfgen does not seem to exhibit this issue when the library is compiled as an executable as per the library instructions, this is using the neos font chain font with the neos fork of msdfgen.exe, this was compiled in release mode, x86: output

I am unsure if this is the root cause, however the malformed data in the MSDF image correlates well to where the image corruption is. I can see it being possible that the MSDF is an input to a downstream process where the visual issue would arise. If that's the case or not, I think it is likely that both the MSDF and ingame text have the same root cause that is expressed here.

This issue persists even with new compilations of msdfgen.dll, I am working on an updated version of the library.

OTF issue not likely

The glyph data seems fine, both 0x3045 and 0x3046 have similar segment and contour structures, it does not appear this is an issue with the otf file.

0x3046
0 0 Cubic [0.344; 0.026] [0.683; 0.121] [0.546; 0.051]
0 1 Line [0.395; -0.062]
0 2 Cubic [0.781; 0.256] [0.6390001; -0.025] [0.781; 0.08800001]
0 3 Cubic [0.562; 0.451] [0.781; 0.381] [0.692; 0.451]
0 4 Cubic [0.309; 0.411] [0.474; 0.451] [0.365; 0.422]
0 5 Cubic [0.232; 0.399] [0.287; 0.406] [0.257; 0.401]
0 6 Line [0.261; 0.297]
0 7 Cubic [0.33; 0.323] [0.277; 0.304] [0.307; 0.316]
0 8 Cubic [0.553; 0.366] [0.381; 0.339] [0.473; 0.366]
0 9 Cubic [0.683; 0.257] [0.629; 0.366] [0.683; 0.325]
1 0 Line [0.334; 0.56]
1 1 Cubic [0.6990001; 0.5190001] [0.422; 0.543] [0.612; 0.523]
1 2 Line [0.7130001; 0.606]
1 3 Cubic [0.348; 0.6440001] [0.617; 0.608] [0.447; 0.625]

0x3045
0 0 Cubic [0.295; 0.047] [0.702; 0.165] [0.538; 0.07700001]
0 1 Line [0.353; -0.05]
0 2 Cubic [0.811; 0.328] [0.617; -0.011] [0.811; 0.115]
0 3 Cubic [0.5560001; 0.559] [0.811; 0.475] [0.7040001; 0.559]
0 4 Cubic [0.258; 0.512] [0.442; 0.559] [0.33; 0.528]
0 5 Cubic [0.16; 0.497] [0.228; 0.506] [0.19; 0.499]
0 6 Line [0.191; 0.383]
0 7 Cubic [0.28; 0.414] [0.217; 0.393] [0.25; 0.406]
0 8 Cubic [0.544; 0.464] [0.335; 0.43] [0.432; 0.464]
0 9 Cubic [0.702; 0.332] [0.642; 0.464] [0.702; 0.408]
1 0 Line [0.283; 0.696]
1 1 Cubic [0.714; 0.649] [0.396; 0.6760001] [0.6020001; 0.657]
1 2 Line [0.73; 0.7460001]
1 3 Cubic [0.298; 0.791] [0.6300001; 0.747] [0.41; 0.767]

Current believed root cause

Current hypothesis is the interop layer to adapt MSDF to .net may have caused this bug. The issue is possibly due to a float getting too large or small, resulting in an edge case like a divide by zero or boundary formatting issue causing what visually looks like data corruption.

This is believed due to:

  1. The error occurs when the .net adaption layer is used. - likely isolated to that area
  2. The error occurs only on a specific set of values, but not a similar set of values. - likely float edge case related
  3. The error is a fixed, but random appearing pattern. - not memory layout related
  4. The graphical error appears on common boundary areas, AKA: edges of a curve and along the horizontal median. - likely due to traits in the underlying glyph, numeric related.

This does not appear to be due to the changes in core/equation-solver.cpp in the neos fork of the repo, reverting those changes does not resolve the graphical artifact.

Current Resolution Attempts

I have a working fork here of the latest of https://github.com/Chlumsky/msdfgen with the dll binding changes of https://github.com/Neos-Metaverse/msdfgen (minus some azure workflow and what I think are accidently checked in files). This port is not complete, and was done by removing parts that didn't seem required, such as the second pass of error correction.

This branch generates a slightly less mangled version of the MSDF: texture

And here's what that looks like inside of neos: image Could be better, but at least it's getting there.

There are still some artifacts around the lower edge of the glyph, but at least the horizontal bar is resolved.

Of note, while removing the second pass of error correction may not be desired, removing the second pass in the current neos branch does make the horizontal bar less noticeable, which makes sense as the bar is getting smoothed out by the error correction pass. This is a MSDF generated with the current neos fork of the repo, with the last msdfErrorCorrection(msdf, edgeThreshold / *(scale*range)); commented out in dll_api.cpp: texture

As there are still visible artifacts in the image with current attempts, more attempts at resolving the corruption are ongoing.

Frooxius commented 2 years ago

@Earthmark Thank you for the detailed analysis, you've pretty thorough! However I think the issue you discovered is actually completely different from the original.

The original seems like it incorrectly flips inside and outside of the character based on the curve, so wrong portion of the character ends up being "filled in". This is what the MSDF looks like in Neos:

2021-11-02 05 46 34

I'm not quite sure why exactly is this happening, I think you have the right hunch that it's a floating point somewhere. We're working on moving a bunch of our dependencies to Azure Pipelines and we'll probably update msdfgen to latest as well since it has large number of improvements.

We're actually using quite a custom fork of it, since the original wasn't suitable for a portable library and we don't need some of its dependencies. Notably we use FreeType to extract outlines from the font Neos-side and then we feed those outlines to msdfgen. Last time I tried it doesn't happen with pure msdfgen so it's probably something wonky that goes there.

Earthmark commented 2 years ago

I think this is happening inside distanceSignCorrection method, it's not flipping some of the pixels when it should be doing a whole image flip, that's where the issue is arising causing the scattered point issue with the glyph, at least with the latest branch of msdfgen.

I have a version that builds the Neos dll variant here https://github.com/Earthmark/msdfgen/actions/runs/1411066406 I'm no longer sure if I'm handling this or if Froox is, but if I'm still handling this it would be helpful for people to find other malformed characters when using the files from this build. be sure to back up the file but the files in this build should be a drop in replacement if put in Steam\steamapps\common\NeosVR\Neos_Data\Plugins\x86_64. This particular build does not fix the う glyph, however if there are other examples of the problem glyphs would be very helpful.

I'm using a github action instead of the pipeline, as I don't have access to the pipeline. It also looked like something strange was happening with the build, this is using the direct cmake build instead of what looked like modified cmake build files? The pr filters out the FreeType dependency using the same mechanism as the existing dependency management. I have the specific scripts to opt out of freetype and such in the github actions file. I'm still working on an actual fix, but https://github.com/Earthmark/msdfgen/pull/1 has the changes.

In case you are also working on this Froox, the font wizzta https://www.dafont.com/wizzta.font has a flipped area on the u character, seems like a good test case. The fully flips the image, but the u only flips a small part of the image when rendered from Neos.

I have a project using some funky snapshot tests right now as well, should you like them they test against CodeX.FontX, and may be helpful to prevent this issue in the future as new versions upgrade.

using BaseX;
using CodeX;
using System;
using System.IO;
using System.Security.Cryptography;
using Xunit;

namespace Msdfgen.NET.Tests
{
  public class ImageTest
  {
    public class ImageSource
    {
      public byte[] Image { get; }

      public ImageSource(byte[] image)
      {
        Image = image;
      }

      public override string ToString()
      {
        return $"hash: {BitConverter.ToString(SHA256.HashData(Image)).Replace("-", "")}";
      }
    }

    // The problem images are stored in a resource file, the fonts are stored as peer-files.
    public static object[][] ProblemCharacters = new object[][]{
      new object[] { "neos_core.otf", "jp-u", 'う', new ImageSource(Resources.neos_core_otf_jp_u) },
      new object[] { "wizzta.ttf", "u", 'u', new ImageSource(Resources.wizzta_ttf_u) },
     };

    [Theory]
    [MemberData(nameof(ProblemCharacters))]
    public void TestKnownFunkyCharacters(string fontPath, string descriptive, char symbol, ImageSource expectedImage)
    {
      var font = FontX.Load(fontPath);
      var id = font.CharToGlyphId(symbol);
      var bitmap = new Bitmap2D(128, 128, TextureFormat.RGBA32, false);
      font.RenderGlyphMSDF(id, bitmap, new Rect(0, 0, 128, 128), 2, false);

      var imagePath = $"{fontPath}-{descriptive}.png";
      bitmap.Save(imagePath, 85);
      byte[] resultData = File.ReadAllBytes(imagePath);

      // If this fails, checkc out -result to see how that looks.
      Assert.Equal(expectedImage.Image, resultData);
    }
  }
}
Earthmark commented 2 years ago

This does appear to be an edgecase based float thing, but as I don't know your compiler versions I can't verify specific behaviors, I can only really test using my local compiler versions.

Earthmark commented 2 years ago

It looks like I got my test cases passing, here's my two test glyphs: wizzta ttf-u neos_core otf-jp-u

Both are rendering correctly inside my test fixture, however inside of Neos it's still mangled: image

I get a feeling this is likely due to the scale or render width and height, do you have some examples of the inputs used in the glyph image generation @Frooxius ? I'm still using the above code for my test bench, but some sample values often passed into those methods by the neos runtime would help with the investigation.

As the glyph looks exactly identical to before my changes, I may try to change my strategy, but the additional data would help.

Also, in terms of the root cause I still don't quite have the true root cause, but at least in the latest build there seems to have been issues with assigning -1 to an array of chars, then equating that -1 to the char again. It only happened in a very rare edge case, and I'm not fully confident that the edge case is present in the current neos build, but it's a fun one.

Earthmark commented 2 years ago

I'm going to change tactics a bit, I'm going to try to do some pull requests updating the neos library to the newest version, that should resolve part of the symbol, btu the rest will be fixed afterwards. The symbol will still have some artifacts as shown above, but it'll be better than the current version.

Earthmark commented 2 years ago

As a status update to this issue, I'm having some trouble making the PR merging the updates into the neos branch, but the PR will look like https://github.com/Earthmark/msdfgen/pull/6 , which is my work fork.

Once the PR to the neos fork is made and through, that should pave the way for how external users can make updates to third party libraries.

Earthmark commented 2 years ago

I finally got this to replicate on the native msdf converter, but to do it I had to use a form of the glyph as processed by Neos, not as is defined in the font itself. It looks like this error may be partially related to how neos parses font files.

Originally I ruled out the font parsing as the structures look similar, and they do appear similar. However there may be an issue lying in the numbers. It's odd, these should be the same glyphs but the Neos glyph looks scaled by a factor of just over two.

For context, the msdfgen tool loads う like this, and doesn't have the issue:

{
    10.96875, 5.1875;
        m(10.96875, 2.578125; 8.40625, 1.203125);
    4.609375, 0.734375;
        c;
    5.515625, -0.78125;
        m(9.640625, -0.171875; 12.671875, 1.796875);
    12.671875, 5.125;
        m(12.671875, 7.421875; 11, 8.734375);
    8.6875, 8.734375;
        m(6.90625, 8.734375; 5.15625, 8.25);
    4.03125, 8;
        m(3.5625, 7.90625; 2.96875, 7.796875);
    2.5, 7.765625;
        y;
    2.984375, 5.984375;
        m(3.390625, 6.140625; 3.90625, 6.34375);
    4.375, 6.46875;
        m(5.234375, 6.71875; 6.75, 7.25);
    8.5, 7.25;
        m(10.03125, 7.25; 10.96875, 6.375);
    #
}
{
    4.65625, 12.359375;
        c;
    4.421875, 10.875;
        m(6.1875, 10.5625; 9.40625, 10.265625);
    11.15625, 10.140625;
        y;
    11.40625, 11.65625;
        m(9.84375, 11.671875; 6.40625, 11.984375);
    #
}

Neos loads う like this, and does have the issue:

@invert-y
{
    22.5374565125, 10.6587400436;
        m(22.5374565125, 5.29726552963; 17.2722969055, 2.47205734253);
    9.47086811066, 1.50891804695;
        c;
    11.3329372406, -1.60523188114;
        m(19.8085632324, -0.353151023388; 26.0368614197, 3.69203329086);
    26.0368614197, 10.5303211212;
        m(26.0368614197, 15.2497034073; 22.6016654968, 17.9464931488);
    17.8501796722, 17.9464931488;
        m(14.1902503967, 17.9464931488; 10.5945310593, 16.9512481689);
    8.28299713135, 16.4375743866;
        m(7.31985759735, 16.2449474335; 6.09988164902, 16.0202140808);
    5.13674211502, 15.9560050964;
        y;
    6.1319861412, 12.2960767746;
        m(6.96670627594, 12.6171226501; 8.0261592865, 13.0344829559);
    8.9892988205, 13.2913208008;
        m(10.7550535202, 13.8049945831; 13.8692035675, 14.8965520859);
    17.4649238586, 14.8965520859;
        m(20.6111774445, 14.8965520859; 22.5374565125, 13.098692894);
    #
}
{
    9.56718254089, 25.3947677612;
        c;
    9.08561325073, 22.3448295593;
        m(12.7134370804, 21.7027359009; 19.3269939423, 21.0927467346);
    22.922712326, 20.8359107971;
        y;
    23.4363861084, 23.9500617981;
        m(20.2259235382, 23.9821643829; 13.1629018784, 24.6242580414);
    #
}

It appears the error occurs due to a difference between distanceSignCorrection and generateMSDF, the symbol borders get slightly muddled between the two, and the pixels getting flipped is the range where distanceSignCorrection believed the region was inside the glyph, but generateMSDF believed that area was not inside the glyph.

It appears so far that generateMSDF is the correct one, and distanceSignCorrection is the one having trouble.

Earthmark commented 2 years ago

The library author has provided a fix for the う character, it is in pre-release for now so I will wait until I get sign off from Foox or Prime as to if the pre-release fix should go into https://github.com/Neos-Metaverse/msdfgen/pull/1 . Having the change in the PR would mean we only need to test once, but I believe the risk is slightly higher.

A branch with the fix is available in https://github.com/Earthmark/msdfgen/pull/7 . If people want to they should be able to test using the files produced by this build. To test them manually place the files into the Neos_Data\Plugins\x86_64 folder of your neos instillation, please make a backup of the msdfgen.dll (or whichever file you use) before trying out the library.

Earthmark commented 2 years ago

I opted to merge the authors character fix into the PR, there's no real reason to review and merge them as separate versions.

Earthmark commented 2 years ago

Due to scheduling issues, the review of the library updates are likely to be delayed until next month. So, hopefully they get resolved before the end of the year, I will poke at the library to get updated on the 10th of next month (I have some vacation before that point and will not be as available).

bontebok commented 2 years ago

I tested @Earthmark's libmsdfgen.so on Linux x64 and I can happily report that the issue for the two characters that were reported in this issue that were rendering incorrectly appear to be resolved using his build. Would a member of the Japanese community comment if the screenshot below shows a correct render of the characters?

75495bd597304d6700a5e062d086cd367fbc80c9cf5a2a37530867d164c3b4d5 ./libmsdfgen.so

Here is what I see on my screen -

Screenshot from 2021-12-09 20-03-35

Here is what a user sees of the same text object -

Screenshot from 2021-12-09 20-03-02

Psychpsyo commented 2 years ago

I tested @Earthmark's libmsdfgen.so on Linux x64 and I can happily report that the issue for the two characters that were reported in this issue that were rendering incorrectly appear to be resolved using his build. Would a member of the Japanese community comment if the screenshot below shows a correct render of the characters?

75495bd597304d6700a5e062d086cd367fbc80c9cf5a2a37530867d164c3b4d5 ./libmsdfgen.so

Here is what I see on my screen -

Screenshot from 2021-12-09 20-03-35

Here is what a user sees of the same text object -

Screenshot from 2021-12-09 20-03-02

Not a member of the Japanese community but the first image is correct.

Earthmark commented 2 years ago

Thank you for verifying, excelent that it works on linux. If someone can verify with android that would be good, but I'm doubtful on that one.

A check in for reviewing the PR to fix this issue occurred, but the PR itself is delayed. The next check in time is 2021/12/20.

Earthmark commented 2 years ago

Check in happened, I scheduled things very badly. This is frozen at the moment, but I have a personal poke to check on this on the 27th of January.

For now, those who are directly affected by this bug can use the patched build of the affected library attached to this pull request https://github.com/Earthmark/msdfgen/pull/6 go to checks, and your platform of choice. Download and overwrite the file with the same name in the Neos installation directory.

If you have any questions please let me know!

KisaragiEffective commented 1 year ago

I've built it locally, it works.

Executed command list:

$ cd /tmp
$ git clone https://github.com/Earthmark/msdfgen
$ cd msdfgen/
$ git checkout neos_update
$ cmake -DCMAKE_BUILD_TYPE=Release -DMSDFGEN_BUILD_SHARED_LIBRARY=ON -DMSDFGEN_BUILD_STANDALONE=OFF -DMSDFGEN_USE_FREETYPE=OFF -DMSDFGEN_INSTALL=OFF -S .
$ make
$ cd out
$ sha256sum libmsdfgen.so
2d88296420d9103518348fea972c875c2e6145e6b4cdfc1c7d02639e777cbabb  libmsdfgen.so
$ cp libmsdfgen.so $STEAM_APP_ROOT/SteamLibrary/steamapps/common/NeosVR/Neos_Data/Plugins
$ git log
commit b5c34966499353823a4c5096e0fea3c7a4efc7cf (HEAD -> neos_update, origin/neos_update)
Merge: 9898970 790b214
Author: Daniel Miller <earthmark.miller@gmail.com>
Date:   Sun Nov 14 13:47:49 2021 -0800

    Merge pull request #7 from Earthmark/neos_jp_character_fix

    Neos jp character fix
$ uname -a
Linux kisaragi-deb11 5.10.0-16-amd64 #1 SMP Debian 5.10.127-1 (2022-06-30) x86_64 GNU/Linux

However, there's still blurry glyph: my friend found another one. I'll create new ticket for it. P.S. I hope you distribute a pre-compiled shared library. It does not mention about cmake flags (I got it from azure-workflow.yml, which is supposed to be same configuration as the released asset)