RazrFalcon / rustybuzz

A complete harfbuzz's shaping algorithm port to Rust
MIT License
498 stars 34 forks source link

Sync to 4.4.1 [I have hit some roadbumps, help would be appreciated] #104

Closed Pi-Cla closed 2 weeks ago

Pi-Cla commented 2 months ago

Not complete

I have hit a roadbump with https://github.com/harfbuzz/harfbuzz/commit/58eeb3a180d03c9b39b39d99a6b9dbf30d17fd9f

Also, tests fail after I started copying over the altered ragel machine category numbers in https://github.com/RazrFalcon/rustybuzz/pull/104/commits/e233856b96abc9b4eb9af13a670e71c6f75c16ce

I think there is probably a later commit somewhere that updates the tests to match back up... Update: I was able to build again by this little edit but it seems like none of the tests got changed. It seems I missed a commit somewhere before cc7ebb0 because new myanmar tests get added which my commit at cc4ed2b fails

I also have not been able to build harfbuzz (to update tests) with the instructions in backporting.md:

meson builddir --reconfigure
ninja -Cbuilddir

after this commit: https://github.com/harfbuzz/harfbuzz/commit/d8d96b26e74aa02aae6af96d35648981d5cea38d

I will just leave this PR here so that people can take a look at what I have done so far

All commits between 4.3.0 and 4.4.1: https://github.com/harfbuzz/harfbuzz/compare/4.3.0...4.4.1

RazrFalcon commented 2 months ago

Problem is that, I also have not been able to build harfbuzz (to update tests) with the instructions in backporting.md:

What errors are you getting? I'm able to compile this commit commit fine on macOS.

RazrFalcon commented 2 months ago

I have hit a roadbump with https://github.com/harfbuzz/harfbuzz/commit/58eeb3a180d03c9b39b39d99a6b9dbf30d17fd9f

What issue are you having?

Pi-Cla commented 2 months ago

What errors are you getting? I'm able to compile this commit commit fine on macOS.

This is what I got:

[169/270] Compiling C++ object test/threads/hb-subset-threads.p/hb-subset-threads.cc.o
FAILED: test/threads/hb-subset-threads.p/hb-subset-threads.cc.o
ccache c++ -Itest/threads/hb-subset-threads.p -Itest/threads -I../test/threads -I. -I.. -Isrc -I../src -fdiagnostics-color=always -D_GLIBCXX_ASSERTIONS=1 -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++11 -fno-rtti -O0 -g -fno-exceptions -fno-rtti -fno-threadsafe-statics -fvisibility-inlines-hidden -DHAVE_CONFIG_H -pthread -MD -MQ test/threads/hb-subset-threads.p/hb-subset-threads.cc.o -MF test/threads/hb-subset-threads.p/hb-subset-threads.cc.o.d -o test/threads/hb-subset-threads.p/hb-subset-threads.cc.o -c ../test/threads/hb-subset-threads.cc
../test/threads/hb-subset-threads.cc: In function ‘void test_operation(operation_t, const char*, const test_input_t&)’:
../test/threads/hb-subset-threads.cc:127:3: error: ‘printf’ was not declared in this scope
  127 |   printf ("Testing %s\n", name);
      |   ^~~~~~
../test/threads/hb-subset-threads.cc:12:1: note: ‘printf’ is defined in header ‘<cstdio>’; did you forget to ‘#include <cstdio>’?
   11 | #include "hb-subset.h"
  +++ |+#include <cstdio>
   12 |
../test/threads/hb-subset-threads.cc: In function ‘int main(int, char**)’:
../test/threads/hb-subset-threads.cc:156:19: error: ‘atoi’ was not declared in this scope
  156 |     num_threads = atoi (argv[1]);
      |                   ^~~~
../test/threads/hb-subset-threads.cc:158:23: error: ‘atoi’ was not declared in this scope
  158 |     num_repetitions = atoi (argv[2]);
      |                       ^~~~
../test/threads/hb-subset-threads.cc:163:30: error: ‘calloc’ was not declared in this scope
  163 |     tests = (test_input_t *) calloc (num_tests, sizeof (test_input_t));
      |                              ^~~~~~
../test/threads/hb-subset-threads.cc:12:1: note: ‘calloc’ is defined in header ‘<cstdlib>’; did you forget to ‘#include <cstdlib>’?
   11 | #include "hb-subset.h"
  +++ |+#include <cstdlib>
   12 |
../test/threads/hb-subset-threads.cc:170:3: error: ‘printf’ was not declared in this scope
  170 |   printf ("Num threads %u; num repetitions %u\n", num_threads, num_repetitions);
      |   ^~~~~~
../test/threads/hb-subset-threads.cc:170:3: note: ‘printf’ is defined in header ‘<cstdio>’; did you forget to ‘#include <cstdio>’?
../test/threads/hb-subset-threads.cc:179:5: error: ‘free’ was not declared in this scope
  179 |     free (tests);
      |     ^~~~
../test/threads/hb-subset-threads.cc:179:5: note: ‘free’ is defined in header ‘<cstdlib>’; did you forget to ‘#include <cstdlib>’?
[174/270] Compiling C++ object src/libharfbuzz.so.0.40300.0.p/hb-ot-shape-complex-use.cc.o
ninja: build stopped: subcommand failed.
Pi-Cla commented 2 months ago

I have hit a roadbump with harfbuzz/harfbuzz@58eeb3a

What issue are you having?

I am unsure how to translate these two lines in gen-indic-table.py https://github.com/harfbuzz/harfbuzz/commit/58eeb3a180d03c9b39b39d99a6b9dbf30d17fd9f#diff-87db7dbadcc69922c030618ee1e154793f8fe35f4b9f0f88e61f5b3374fe67baL136 https://github.com/harfbuzz/harfbuzz/commit/58eeb3a180d03c9b39b39d99a6b9dbf30d17fd9f#diff-87db7dbadcc69922c030618ee1e154793f8fe35f4b9f0f88e61f5b3374fe67baL153

LaurenzV commented 2 months ago

For me the commit you linked seems to compile fine, too... Although I'm on MacOS, maybe a platform-dependent thing? But I'm not knowledgeable enough about C++ to judge what's going on here... Only thing I can suggest is to maybe try deleting the whole builddir and trying it again.

Pi-Cla commented 2 months ago

When I added

#include <cstdio>
#include <cstdlib>

to hb-subset-threads.cc I was then able to compile it so I guess I will just do that awkward workaround for now... Should I add this to the backporting documentation?

RazrFalcon commented 2 months ago

Should I add this to the backporting documentation?

I bet harfbuzz fixed this in future commits on their side. It's somewhat common to have compilation issues with the old commits.

As for gen-indic-table.py, you should just copy-paste the whole file and then replace C++ output with Rust and download input files via urllib (just like it currently does). Basically, in rustybuzz this file is just a fork with minor changes.

LaurenzV commented 3 weeks ago

@RazrFalcon I'm currently trying to investigate why the new test cases don't pass, and it seems to somehow be related to global_bit_shift/global_bit_mask? In harfbuzz, they are set to:

  unsigned int global_bit_shift = 8 * sizeof (hb_mask_t) - 1;
  unsigned int global_bit_mask = 1u << global_bit_shift;

while we have

    const GLOBAL_BIT_MASK: hb_mask_t = glyph_flag::DEFINED + 1;
    const GLOBAL_BIT_SHIFT: u32 = glyph_flag::DEFINED.count_ones();

Do you happen to know why it looks completely different? I did try to change it to what harfbuzz has, but then nearly all test cases suddenly fail...

EDIT: Okay, I've at least managed to not make them fail again. But unfortunately didn't solve the current failing test cases. :(

RazrFalcon commented 3 weeks ago

This code was written by @laurmaedje. Maybe he remembers the reason.

laurmaedje commented 3 weeks ago

I ported this piece of code in this commit as:

let global_bit_mask = glyph_flag::DEFINED + 1;
let global_bit_shift = glyph_flag::DEFINED.count_ones();

The Harfbuzz source code at the time was (as visible in that commit's diff):

unsigned int global_bit_mask = RB_GLYPH_FLAG_DEFINED + 1;
unsigned int global_bit_shift = rb_popcount(RB_GLYPH_FLAG_DEFINED);

Not sure what changed since then.

behdad commented 3 weeks ago

Do you happen to know why it looks completely different?

This was changed in harfbuzz in https://github.com/harfbuzz/harfbuzz/commit/a150baf32c

LaurenzV commented 3 weeks ago

Thanks! I'll take a look.

LaurenzV commented 2 weeks ago

Okay I figured it out... two very annoying bugs :(, but I managed to fix them.

Let's see if I can get the updated state machines to work as well.