raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
158 stars 57 forks source link

TrueType font support #80

Closed gvissers closed 4 years ago

gvissers commented 4 years ago

So here's a pull request for my ttf branch, adding TrueType font support to the Eternal Lands client using the SDL_ttf library. I have fixed up all places I could find that assume a fixed width font, except for the encyclopedia which would require either extensive heuristics or a complete rewrite of all encyclopedia XML files. I have been using this for a few weeks now, and everything looks to be stable.

A few things that I'm aware of:

As i had to change quite a few things, the diff is somewhat large:

> git diff origin/HEAD HEAD --shortstat
 132 files changed, 14646 insertions(+), 7649 deletions(-)

As I said before, I'm leaving the decision on whether to merge or not up to @pjbroad. I'm pretty happy with it, though.

pjbroad commented 4 years ago

Wow, I didn't know we had that many files! I'll give it a try. Any objection if I use the "squash and merge" for the pull request? That way we have a single commit to manage.

I build for windows on a VM too. I documented the build process in a new git repo. It uses the new CMAKE file but it is easy to add new libraries, particularly if its just SDL_ttf. For me I have to copy the OpenGL DLL in with the other DLLs for it to run in the VM OK.

gvissers commented 4 years ago

Wow, I didn't know we had that many files!

Since I encountered UI scaling code all over the place (which made this series a lot easier, thank you!), I think you must have gone through them all at some point :)

I'll give it a try. Any objection if I use the "squash and merge" for the pull request? That way we have a single commit to manage.

Thanks. I don't mind if you combine it in a single commit.

I build for windows on a VM too. I documented the build process in a new git repo. It uses the new CMAKE file but it is easy to add new libraries, particularly if its just SDL_ttf. For me I have to copy the OpenGL DLL in with the other DLLs for it to run in the VM OK.

Thanks again, this looks useful. I didn't find msys on my quest for a usable build environment. I knew about mingw and dev-c++, but both are hopelessly out of date now, and msvc is just a pain (though strictly speaking, it is correct in a lot of its warnings). I'll give Msys a try later.

pjbroad commented 4 years ago

I've had a quick play with the new code. Sorry for a quick dump of issues I've experienced:

  1. Even after a restart, I'm not seeing any TTF fonts even with the directory set to my systems path, just the dds files. I'm on Ubuntu 20.04 and the path is /usr/share/fonts/truetype which contains a lot of fonts.
  2. I'm also getting crashes while editing that path field.
  3. The UI font scaling does not adjust correctly for the right-hand hud stuff.
  4. If you scale the help, skills or encyclopaedia using the window original scaling code, it no longer scales correctly.
  5. The Counter, Knowledge window is not resizing correctly, loosing some widgets and not always shrinking back when scaled down.
  6. There's an interesting shrinking of the font options if you repeatedly toggle the TTF option.
  7. I think generally, there's a bit of mismatch between the UI scaling code and the font scaling code. It may be simpler to remove the UI font scaling setting, and leave it to the UI scaling which scales everything together. Big fonts on a little window are perhaps not useful, similarly, little fonts on a big window. Sorry if that's not helpful, but there seem to be a fair number of issues.
gvissers commented 4 years ago

Thanks for trying this out, I'm sorry to hear it was disappointing.

1. Even after a restart, I'm not seeing any TTF fonts even with the directory set to my systems path, just the dds files.  I'm on Ubuntu 20.04 and the path is /usr/share/fonts/truetype which contains a lot of fonts.

I assume the fonts have a .ttf extension (that's what it globs for) and the client has read permission for the directory? It is possible not all fonts show up, because they don't all have glyphs for our character set, but none at all is surprising. Is there anything font related in the log?

2. I'm also getting crashes while editing that path field.

Obviously that shouldn't happen either, and I'm unable to reproduce it. Are you able to create a way to reliably crash it? I'll try it under valgrind later, see if that reports anything.

3. The UI font scaling does not adjust correctly for the right-hand hud stuff.

Yes/no/maybe. It does try to fit things horizontally, but space is very limited. Vertical scaling of the buttons is indeed not implemented. I'll have a look at this, I'll probably end up having to restrict the zoom level for this panel.

4. If you scale the help, skills or encyclopaedia using the window original scaling code, it no longer scales correctly.

Yup, that's a bug. WIll fix.

5. The Counter, Knowledge window is not resizing correctly, loosing some widgets and not always shrinking back when scaled down.

Can you give me a bit more detail on what you mean here?

6. There's an interesting shrinking of the font options if you repeatedly toggle the TTF option.

Huh, never noticed that. Yes, another bug. No idea how that happens.

7. I think generally, there's a bit of mismatch between the UI scaling code and the font scaling code.  It may be simpler to remove the UI font scaling setting, and leave it to the UI scaling which scales everything together.  Big fonts on a little window are perhaps not useful, similarly, little fonts on a big window.

Perhaps, but I'm not yet willing to give up on it. The main reason is that not all fonts are equal, and some fonts, although they are scaled to the same font height, look smaller than others. A font specific scaling factor is useful if you want to tune things a bit.

   Sorry if that's not helpful, but there seem to be a fair number of issues.

On the contrary, it is very helpful you found these bugs. I must say I am disappointed in myself for not noticing some of these earlier.

pjbroad commented 4 years ago
  1. I've got the TTF selection working by using one of the /usr/share/fonts/truetype/ sub-directories like /usr/share/fonts/truetype/ubuntu.

  2. I noticed that the text is not aligned correctly for me using the hacky gx_adjust and gy_adjust values from the trouble shooting options. I can no longer find one setting that aligns the text in buttons for example, with the tabs on a tabbed window.

  3. The coloured font used in the login window looks weird, washed out, when using a TTF font.

  4. The issue with the counter etc window, looks to be due to the UI_SCALE and TTF_SCALE handlers getting out of sync. Try changing the UI font size, and then changing the UI scale value. When the window gets messed up, you can fix it by triggering an additional call to the UI_SCALE handler for example.

  5. Edit: Here's the crash:

    =================================================================
    ==18251==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55814570cea0 at pc 0x55814506b85a bp 0x7ffd3d9e7c20 sp 0x7ffd3d9e7c10
    WRITE of size 1 at 0x55814570cea0 thread T0
    #0 0x55814506b859 in pword_insert /home/paul/el/ttf/Eternal-Lands/widgets.c:3545
    #1 0x55814506cf1b in pword_field_keypress /home/paul/el/ttf/Eternal-Lands/widgets.c:3754
    #2 0x55814507221a in widget_handle_keypress /home/paul/el/ttf/Eternal-Lands/widgets.c:637
    #3 0x558144f04818 in keypress_in_window /home/paul/el/ttf/Eternal-Lands/elwindows.c:1893
    #4 0x558144f049b0 in keypress_in_windows /home/paul/el/ttf/Eternal-Lands/elwindows.c:660
    #5 0x558144f0fa97 in HandleEvent /home/paul/el/ttf/Eternal-Lands/events.c:286
    #6 0x558144f6ffd7 in start_rendering /home/paul/el/ttf/Eternal-Lands/main.c:183
    #7 0x558144f713ae in main /home/paul/el/ttf/Eternal-Lands/main.c:550
    #8 0x7fd5ea0990b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #9 0x558144e2348d in _start (/home/paul/el/ttf/Eternal-Lands/build/el.linux.bin+0xe248d)
  6. Adding the CMAKE stuff, I get some "may be used uninitialized" warnings:

    /home/paul/el/ttf/Eternal-Lands/rules.c: In function ‘draw_rules’:
    /home/paul/el/ttf/Eternal-Lands/rules.c:734:58: warning: ‘ydiff’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    734 |    y_curr += (rule->long_str_nr_lines - 1) * line_height + ydiff;
      |              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~
    /home/paul/el/ttf/Eternal-Lands/rules.c:720:33: warning: ‘color_idx’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    720 |    y_in, leny, line_height, &rgb[color_idx][0],
      |                                 ^
    
    /home/paul/el/ttf/Eternal-Lands/font.cpp: In member function ‘void eternal_lands::Font::draw(const unsigned char*, size_t, int, int, const eternal_lands::TextDrawOptions&, size_t, size_t) const’:
    /home/paul/el/ttf/Eternal-Lands/font.cpp:814:20: warning: ‘x_left’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    814 |    x_draw = x_left + line_width - ellipsis_width;
      |             ~~~~~~~^~~~~~~~~~~~

7. I also noticed you are using auto pointers, this needs "-std=c++14" rather than "-std=c++11".  Steady on :)

8. One more thing.  I note you have #defined TTF some of the code.  However, it does not compile without that set.

I have some comparatively minor changes I've been working ready on to commit.  I'll continue to commit these to the master branch as they should be an easy merge for you I hope.
gvissers commented 4 years ago
  1. So we need to scan subdirectories as well. But how deep do we want to go? I would not like to scan the entire file system because a user enter '/' as a font path by mistake.

  2. Ugh, I'll see what I can do. If you can give me a specific example (maybe with screenshot), I know better what to work towards.

  3. You mean the username and password? Hmm... I think I see what you mean. I'll see if adding a shadow makes a difference.

  4. Yes, I think I can fix this, but will need a bit of time to dig into this.

  5. Thanks, I'll have a look, see if I can figure out what goes wrong. EDIT: found the problem. I remember being absolutely sure when I wrote this that the buffer was allocated one byte longer than max_chars, and now I can't for the life of me remember why I thought this. Oh well.

  6. Those warnings are bogus, there's no way any of those variables are used without initialization, and a decent tracker should be able to see that. I'll add an =0 to the declarations to shut the warning ups, though.

  7. Ugh, I never noticed gcc defaults to c++14 now. I'll add a std=c++11 to my makefile and fix the code. I'll be happy when we can use c++17 BTW (in a decade or two :P)

  8. I actually had that fixed, but forgot to push out.

Please continue committing to the master branch (I actually have a small fix for the trade window close button as well, which I will push out later), I'll handle the merge.

PS: And BTW, thanks for the review!

pjbroad commented 4 years ago

Examples for point 2 above. It looks minor but more noticeable on the hub for example. Non TTF font TTF font same gy_adjust TTF font gy_adjust adjusted for numbers y

pjbroad commented 4 years ago

I see you've added ELW_HANDLER_UI_SCALE callbacks to each of the sub-windows in your latest commits. I think I had a really good reason for the using the callback in the container window instead. I wish I could remember what it was. It looks to be working OK now though.

I've also noticed that the right side hud stats bars look to be too short even with TTF turned off.

gvissers commented 4 years ago

I see you've added ELW_HANDLER_UI_SCALE callbacks to each of the sub-windows in your latest commits. I think I had a really good reason for the using the callback in the container window instead. I wish I could remember what it was. It looks to be working OK now though.

You did, you need the vertical offset of the window contents to change with the change in tab height. The UI change handlers in the sub-windows only set a minimum size for the window, based on the contents. The actual work is still done in the container window, though the actual UI scale handler for the container window only sets a flag that is picked up by a display handler. This is because the UI scale handler for the container runs before those of the child windows. It might have been possible to change the order around, but I did not want to rely on any particular ordering.

pjbroad commented 4 years ago

I see you've added ELW_HANDLER_UI_SCALE callbacks to each of the sub-windows in your latest commits. I think I had a really good reason for the using the callback in the container window instead. I wish I could remember what it was. It looks to be working OK now though.

You did, you need the vertical offset of the window contents to change with the change in tab height. The UI change handlers in the sub-windows only set a minimum size for the window, based on the contents. The actual work is still done in the container window, though the actual UI scale handler for the container window only sets a flag that is picked up by a display handler. This is because the UI scale handler for the container runs before those of the child windows. It might have been possible to change the order around, but I did not want to rely on any particular ordering.

That sounds familiar, thanks for the explanation.

pjbroad commented 4 years ago

BTW, I tried building on Windows: font.cpp fails to include glob.h. There's a windows way of doing that elsewhere in the client.

pjbroad commented 4 years ago

Note sure I'd used the buddy handler before but I got this crash when changing the font:

==70089==ERROR: AddressSanitizer: global-buffer-overflow on address 0x556c911ee4bf at pc 0x556c90979f33 bp 0x7ffd847f77d0 sp 0x7ffd847f77c0
READ of size 1 at 0x556c911ee4bf thread T0
    #0 0x556c90979f32 in click_buddy_handler /home/paul/el/ttf/Eternal-Lands/buddy.c:188
    #1 0x556c909cb15c in click_in_window /home/paul/el/ttf/Eternal-Lands/elwindows.c:1669
    #2 0x556c909cb3bb in click_in_windows /home/paul/el/ttf/Eternal-Lands/elwindows.c:303
    #3 0x556c909d94b7 in HandleEvent /home/paul/el/ttf/Eternal-Lands/events.c:459
    #4 0x556c90a399a8 in start_rendering /home/paul/el/ttf/Eternal-Lands/main.c:183
    #5 0x556c90a3ad7f in main /home/paul/el/ttf/Eternal-Lands/main.c:550
    #6 0x7fc4665670b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #7 0x556c908ec46d in _start (/home/paul/el/ttf/Eternal-Lands/build/el.linux.bin+0xe246d)

0x556c911ee4bf is located 33 bytes to the left of global variable 'buddy_list' defined in '/home/paul/el/ttf/Eternal-Lands/buddy.c:64:15' (0x556c911ee4e0) of size 3300
0x556c911ee4bf is located 55 bytes to the right of global variable 'main_bbox_tree' defined in '/home/paul/el/ttf/Eternal-Lands/bbox_tree.c:22:12' (0x556c911ee480) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow /home/paul/el/ttf/Eternal-Lands/buddy.c:188 in click_buddy_handler

The latest changes fix lots of the scaling glitches and the right-hand hud is better; though it still works better to increase the HUD size IMHO. The alignment stuff gx_adjust/gy_adjust is still wrong for me. I spent ages getting that right all across the UI. The are also still issues with the counters widgets going off frame until I touch the UI_SCALE.

What is your intended strategy when changing font scale? Some windows increase in size, some increase just the widgets containing text, and some look to do a bit of both.

Scanning one level down for ttf files work for me though I have one set that is two levels. I note that some of the font names exceed the width of the multi-select button.

Its weird, but some of the fonts I've tied look very pixelated, just poor fonts I guess. Is there anything we could do with anti-alising? I notice the anti-alising option in the client does not appear to do anything for me. I'll have a look at it.

pjbroad commented 4 years ago

Its weird, but some of the fonts I've tied look very pixelated, just poor fonts I guess. Is there anything we could do with anti-alising? I notice the anti-alising option in the client does not appear to do anything for me. I'll have a look at it.

OK, FSAA was disabled in the build (must have been years ago) and when turn on fully, makes things quite a lot better.

pjbroad commented 4 years ago

Another crash, this was with FSAA support built in so may not be valid:

==70089==ERROR: AddressSanitizer: global-buffer-overflow on address 0x556c911ee4bf at pc 0x556c90979f33 bp 0x7ffd847f77d0 sp 0x7ffd847f77c0
READ of size 1 at 0x556c911ee4bf thread T0
    #0 0x556c90979f32 in click_buddy_handler /home/paul/el/ttf/Eternal-Lands/buddy.c:188
    #1 0x556c909cb15c in click_in_window /home/paul/el/ttf/Eternal-Lands/elwindows.c:1669
    #2 0x556c909cb3bb in click_in_windows /home/paul/el/ttf/Eternal-Lands/elwindows.c:303
    #3 0x556c909d94b7 in HandleEvent /home/paul/el/ttf/Eternal-Lands/events.c:459
    #4 0x556c90a399a8 in start_rendering /home/paul/el/ttf/Eternal-Lands/main.c:183
    #5 0x556c90a3ad7f in main /home/paul/el/ttf/Eternal-Lands/main.c:550
    #6 0x7fc4665670b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #7 0x556c908ec46d in _start (/home/paul/el/ttf/Eternal-Lands/build/el.linux.bin+0xe246d)

0x556c911ee4bf is located 33 bytes to the left of global variable 'buddy_list' defined in '/home/paul/el/ttf/Eternal-Lands/buddy.c:64:15' (0x556c911ee4e0) of size 3300
0x556c911ee4bf is located 55 bytes to the right of global variable 'main_bbox_tree' defined in '/home/paul/el/ttf/Eternal-Lands/bbox_tree.c:22:12' (0x556c911ee480) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow /home/paul/el/ttf/Eternal-Lands/buddy.c:188 in click_buddy_handler
gvissers commented 4 years ago

Thanks again for your reports. I really appreciate the effort you're putting into this.

I'll have a look at the crashes first. Things like that shouldn't happen.

The latest changes fix lots of the scaling glitches and the right-hand hud is better; though it still works better to increase the HUD size IMHO.

I absolutely agree. This is only to ensure things keep working correctly when someone does decide to change the font size. I think once everything has settles I'll restrict the UI font range further, maybe 0.8-1.2. But being able to scale up to 2.0 helps in testing.

What is your intended strategy when changing font scale? Some windows increase in size, some increase just the widgets containing text, and some look to do a bit of both.

In general I'd like to keep windows as small as possible, not changing their size until things don't fit anymore.

Scanning one level down for ttf files work for me though I have one set that is two levels. I note that some of the font names exceed the width of the multi-select button.

Well, one can always copy the fonts one wants to use to a separate folder and point the client to that :) Width issue should be fixed (well, clipped, really. Perhaps I'll have a look later at scaling the width of the multiselects if the content exceeds the buttons).

The gx/gy adjust I will look at at some later point, mainly because it will probably involve studying how a font is represented some more. Let's get the basic stuff right before I start pixel adjusting.

gvissers commented 4 years ago

The buddy handler crash has nothing to do with the TTF changes. Fix pushed to master.

pjbroad commented 4 years ago

The gx/gy adjust I will look at at some later point, mainly because it will probably involve studying how a font is represented some more. Let's get the basic stuff right before I start pixel adjusting.

Agreed. I've just been looking at the gx/gy adjust stuff a bit more. It's a real hack and I think has been used (mostly by me) to fix other underlying issues. I've just studied the tab drawing code and that can be made to look fine, by removing the gx/gy adjust and just fixing the underlying tab drawing. Sometimes changing code is like pushing on a balloon.

The buddy handler crash has nothing to do with the TTF changes. Fix pushed to master.

Huh, I guess it just showed up due to me messing with the scaling.

pjbroad commented 4 years ago

Hi. I've not had much time to test the last few days but I just looked at the latest changes. Some text on buttons is not visible, for example on the mix window, or if it is visible bits appear and disappears as you alter the UI scaling or the font scaling. I also noticed that for the quest log, NPC window, the check boxes become misaligned when you change the font scale, though not the UI scale.

gvissers commented 4 years ago

Hi. I've not had much time to test the last few days but I just looked at the latest changes.

No worries, I have not had much time to wok on this either,

Some text on buttons is not visible, for example on the mix window, or if it is visible bits appear and disappears as you alter the UI scaling or the font scaling. I also noticed that for the quest log, NPC window, the check boxes become misaligned when you change the font scale, though not the UI scale.

Got those two fixed, will check to see if more buttons clip their text. I'm also taking a closer look at imprving vertical alignment now, but that may take a bit of time to materialize.

pjbroad commented 4 years ago

In windows, using the build process I mentioned, I'm getting a link error:

[100%] Linking CXX executable el.exe
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/10.1.0/../../../../x86_64-w64-mingw32/bin/ld.exe: CMakeFiles/el.dir/objects.a(elpathwrapper.c.obj): in function `search_files_and_apply':
C:/msys64/home/paul/build/elc/io/elpathwrapper.c:811: undefined reference to `__imp_PathMatchSpecA'
collect2.exe: error: ld returned 1 exit status
make[2]: *** [CMakeFiles/el.dir/build.make:3101: el.exe] Error 1
make[1]: *** [CMakeFiles/Makefile2:96: CMakeFiles/el.dir/all] Error 2
make: *** [Makefile:150: all] Error 2
pjbroad commented 4 years ago

When I include linking with libshlwapi, it runs but I get a segfault on startup. Here's a gdb backtrace:

#0  safe_snprintf (dest=dest@entry=0x3d0e9b0 "C:/Windows/Fonts",
    len=len@entry=6635050,
    format=format@entry=0x6301e0 <ttf_directory> "C:/Windows/Fonts")
    at C:/msys64/home/paul/build/elc/asc.c:199
        ap = 0x3d0e738 "ñÞð\003"
        ret = 16
#1  0x00000000004fbde8 in search_files_and_apply (
    base_path=base_path@entry=0x6301e0 <ttf_directory> "C:/Windows/Fonts",
    pattern=pattern@entry=0x695f09 <CommandQueue::Queue::min_wait_time_ms+1721> "*.ttf", fn=fn@entry=0x533c29 <_FUN(char const*)>,
    max_depth=max_depth@entry=1)
    at C:/msys64/home/paul/build/elc/io/elpathwrapper.c:808
        nr_found = 0
        full_path = "C:/Windows/Fonts\000\000\000\000\000\000\000\000p\001", '\000' <repeats 14 times>, "`\000\000@\000\000\000\000\030\000\000\000\000\000\000\000\200\001\000\000\000\000\000\000\000\000u\004\000\000\000\000;║ι\177\000\000\000\000u\004\000\000\000\000b\000\000@¹\177\000\000\017\000\000\000\000\000\000\000ðYk\004\000\000\000\000\020(L\a\000\000\000\000P\033L\a\000\000\000\000Ó*Áj\000\000\000\000á\235\217Á¹\177\000\000\004\000\000\000\000\000\000\000\b\000\000\000\000\000\000\000▄Ûð\003\000\000\000\000\b\000\000\000\000\000\000\000\023\000\000\000\000\000\000\000└QL\a\000\000\000\000`BÁj\000\000\000\000"...
        c_file = {attrib = 21, time_create = 1552974764,
          time_access = 1591219178, time_write = 1570417221, size = 0,
          name = "Fonts", '\000' <repeats 254 times>}
        hFile = 730240
#2  0x000000000053431c in eternal_lands::FontManager::initialize_ttf (
    this=this@entry=0x6307e0 <eternal_lands::FontManager::get_instance()::manager>) at C:/msys64/home/paul/build/elc/font.cpp:1364
No locals.
#3  0x000000000053443e in eternal_lands::FontManager::initialize (
    this=this@entry=0x6307e0 <eternal_lands::FontManager::get_instance()::manager>) at C:/msys64/home/paul/build/elc/font.cpp:1342
No locals.
#4  0x000000000053447d in initialize_fonts ()
    at C:/msys64/home/paul/build/elc/font.h:927
No locals.
#5  0x00000000004664d9 in init_stuff ()
    at C:/msys64/home/paul/build/elc/init.c:671
        seed = <optimized out>
        file_name = '\000' <repeats 16 times>, "á!h\004\000\000\000\000\020\000\000\000\000\000\000\000áÒ-\a\000\000\000\000æ¹Î¹\177\000\000ö║c\000\000\000\000\000\000\000u\004\000\000\000\000\001", '\000' <repeats 31 times>, " \002\000\000\000\000\000\000³£\217Á¹\177\000\000â)d\000\000\000\000\000°#d\000\000\000\000\000â)d\000\000\000\000\000°#d\000\000\000\000\000â)d\000\000\000\000\000×\201R", '\000' <repeats 21 times>, "\001\000\000\000\000\000\000\000\061\000\000\000\000\000\000\000└³ð\003\000\000\000\000"...
        i = <optimized out>
        config_location = "\000\000u\004\000\000\000\000;║ι\177\000\000\000\000u\004\000\000\000\000b\000\000@\000\000\000\000%\000\000\000\000\000\000\000\060", '\000' <repeats 15 times>, "Ó¹ð\003\000\000\000\000áÒ-\a\000\000\000\000¬\000\000\000\000\000\000\000­\006\000\000\000\000\000\000ÔÎC", '\000' <repeats 13 times>, "Ì", '\000' <repeats 15 times>, "\r", '\000' <repeats 23 times>, "\220Ò-\a", '\000' <repeats 20 times>, "¢\aϹ\177\000\000\000\000u\004\000\000\000\000\000\000u\004\000\000\000\000\220Ò-\a\000\000\000\000"...
        cfgdir = <optimized out>
        __FUNCTION__ = "init_stuff"
#6  0x000000000047dc51 in Main (argc=<optimized out>, argv=<optimized out>)
    at C:/msys64/home/paul/build/elc/main.c:546
No locals.
#7  0x000000000047dcbf in WinMain (hInst=<optimized out>,
    hPrev=hPrev@entry=0x0, lpCmd=<optimized out>, nShow=<optimized out>)
    at C:/msys64/home/paul/build/elc/main.c:623
        argv = 0x46ad440
        argc = <optimized out>
#8  0x000000000062c6d2 in main (flags=<optimized out>,
    cmdline=<optimized out>, inst=<optimized out>)
    at D:/mingwbuild/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crt0_c.c:18
No locals.
#9  0x00000000004013c1 in __tmainCRTStartup ()
    at D:/mingwbuild/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:335
        lock_free = <optimized out>
        fiberid = <optimized out>
        nested = <optimized out>
        lpszCommandLine = <optimized out>
        StartupInfo = {cb = 104, lpReserved = 0xca160 "",
          lpDesktop = 0xbf150 "Winsta0\\Default",
          lpTitle = 0xc8670 "C:\\msys64\\home\\paul\\install\\Eternal Lands\\el.exe", dwX = 0, dwY = 0, dwXSize = 0, dwYSize = 0, dwXCountChars = 0,
          dwYCountChars = 0, dwFillAttribute = 0, dwFlags = 0,
          wShowWindow = 0, cbReserved2 = 0, lpReserved2 = 0x0,
          hStdInput = 0xffffffffffffffff, hStdOutput = 0xffffffffffffffff,
          hStdError = 0xffffffffffffffff}
        inDoubleQuote = <optimized out>
#10 0x00000000004014d6 in WinMainCRTStartup ()
    at D:/mingwbuild/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crtexe.c:192
        ret = 255
pjbroad commented 4 years ago

Some text on buttons is not visible, for example on the mix window, or if it is visible bits appear and disappears as you alter the UI scaling or the font scaling. I also noticed that for the quest log, NPC window, the check boxes become misaligned when you change the font scale, though not the UI scale.

Got those two fixed, will check to see if more buttons clip their text. I'm also taking a closer look at imprving vertical alignment now, but that may take a bit of time to materialize.

The latest changes do not fix the problem with text in buttons for me.

Also, the quest log NPC window now prints all the NPC names on top of each other.

gvissers commented 4 years ago

The latest changes do not fix the problem with text in buttons for me.

Small text only (like the arrow buttons in the manufacture window)? That's possible, I missed a fix in the commit. If other buttons (apart from the font names in the settings window) miss text, please let me know.

Also, the quest log NPC window now prints all the NPC names on top of each other.

Yeah, that was stupid. Missed that with only a single NPC in my test account.

WRT shlwapi: it was still untested on windows. I think I'll rewrite it to simply scan the directory twice, once to pick up the matching files, and once again for the subdirectories. That should allow me to get rid of PathMatchSpecA.

pjbroad commented 4 years ago

The latest changes do not fix the problem with text in buttons for me.

Small text only (like the arrow buttons in the manufacture window)? That's possible, I missed a fix in the commit. If other buttons (apart from the font names in the settings window) miss text, please let me know.

Its better but still causing a issue when using the UI Scaling at least. The "Clear" button and the "Mix" buttons on the manufacturing window, and the "Cast" and "Clear" buttons on the Sigils window at least show the problem. The invisible text problem looks to be confined to the last character of the labels.

On the manu window, when scaling the font, the text quickly extended beyond the height of buttons and the +/- used to open the mix pipeline merges into one.

The NPC list window of the quest log looks right now.

Windows runs without crashing.

I get an uninitialised var warning on compile too.

In file included from /home/paul/el/ttf/Eternal-Lands/vmath.h:13,
                 from /home/paul/el/ttf/Eternal-Lands/bbox_tree.h:5,
                 from /home/paul/el/ttf/Eternal-Lands/actors.h:15,
                 from /home/paul/el/ttf/Eternal-Lands/achievements.h:5,
                 from /home/paul/el/ttf/Eternal-Lands/elconfig.c:23:
/home/paul/el/ttf/Eternal-Lands/elconfig.c: In function ‘get_elconfig_content_width’:
/home/paul/el/ttf/Eternal-Lands/misc.h:242:21: warning: ‘line_width’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  242 |  return (x >= y)? x : y;
      |         ~~~~~~~~~~~~^~~
/home/paul/el/ttf/Eternal-Lands/elconfig.c:3304:15: note: ‘line_width’ was declared here
 3304 |  int i, iopt, line_width, max_line_width = 0;
      |               ^~~~~~~~~~
gvissers commented 4 years ago

Its better but still causing a issue when using the UI Scaling at least. The "Clear" button and the "Mix" buttons on the manufacturing window, and the "Cast" and "Clear" buttons on the Sigils window at least show the problem. The invisible text problem looks to be confined to the last character of the labels.

I'm sorry, I have tried with a rather wide variety of ui scale factors, font scales, and fonts, but I am unable to reproduce this, and I have no clear idea what could cause this. Does it help if you give it a few pixels extra wiggle room in draw_smooth_button() (widgets.c, line 1066, w -> w+2). If so, perhaps a strange roundoff error somewhere? Otherwise, could you please give me a font, ui scale, and font scale with which you see characters disappeat?

On the manu window, when scaling the font, the text quickly extended beyond the height of buttons

Quickly? Yes, it's possible, the button height is not scaled with the font scale,. But with the fonts I have tried so far, I have to ramp the UI scale factor up rather high (1.8) to hit the edge of the button. I was planning to restrict the UI scale to something like 0.8-1.2, at that level I don't think we extend the button height yet.

and the +/- used to open the mix pipeline merges into one.

Yeah, that looks sloppy. Will have a look.

I get an uninitialised var warning on compile too.

This one's actually valid. Adding -Wmaybe-uninitialized to compiler flags. Or rather -W -Wno-unused-parameter -Wno-sign-compare. Even without warning about unused parameteres or different signs in comparisons, it still prints a bunch of warnings, some of which seem actually valid.

pjbroad commented 4 years ago

Its better but still causing a issue when using the UI Scaling at least. The "Clear" button and the "Mix" buttons on the manufacturing window, and the "Cast" and "Clear" buttons on the Sigils window at least show the problem. The invisible text problem looks to be confined to the last character of the labels.

I'm sorry, I have tried with a rather wide variety of ui scale factors, font scales, and fonts, but I am unable to reproduce this, and I have no clear idea what could cause this. Does it help if you give it a few pixels extra wiggle room in draw_smooth_button() (widgets.c, line 1066, w -> w+2). If so, perhaps a strange roundoff error somewhere? Otherwise, could you please give me a font, ui scale, and font scale with which you see characters disappeat?

OK, I've avoided digging into the new code too much so far, I'll see what I can find.

On the manu window, when scaling the font, the text quickly extended beyond the height of buttons

Quickly? Yes, it's possible, the button height is not scaled with the font scale,. But with the fonts I have tried so far, I have to ramp the UI scale factor up rather high (1.8) to hit the edge of the button. I was planning to restrict the UI scale to something like 0.8-1.2, at that level I don't think we extend the button height yet.

Sorry, not sure why I said quickly. Its that the width scales better the the height. I'm not sure you mean to say you UI scaling in the above rather than font scaling. If you did mean UI scaling (i.e. the full UI and font scaling work that is in the current release client), then I would not want to clamp or change the current range, its actually used and work just fine.

I get an uninitialised var warning on compile too.

This one's actually valid. Adding -Wmaybe-uninitialized to compiler flags. Or rather -W -Wno-unused-parameter -Wno-sign-compare. Even without warning about unused parameteres or different signs in comparisons, it still prints a bunch of warnings, some of which seem actually valid.

IMHO, perhaps we should just concentrate on fixing any warnings for the new code rather than make too many changes elsewhere. That sounds like a good thing to do but under another PR or set of commits.

gvissers commented 4 years ago

OK, I've avoided digging into the new code too much so far, I'll see what I can find.

Thanks. I actually felt bad about asking, but I'm unable to find out where it goes wrong without being able to reproduce it.

I'm not sure you mean to say you UI scaling in the above rather than font scaling.

Sorry, I meant font scaling of course.

IMHO, perhaps we should just concentrate on fixing any warnings for the new code rather than make too many changes elsewhere.

Oh, I agree, I was just pointing it out.

pjbroad commented 4 years ago

I've tried out the latest changes. The trouble with the manufacture window buttons is fixed. I've also had quite a run though the various windows and have found some minor issues:

  1. The item list has a "+" in the right side. This does not maintain its central vertical position when changing the UI font scale. Also, for font scaling, the list window is not moved as the inventory window increases in size due to font changes.
  2. When using UI scaling for the Book window, the navigation bar at the bottom of the window is no longer moved as the scale changes. Touching the font scale fixes the position.
  3. For the NPC list of quest log, the window does not resize as the scale is changes, it is always one resize behind. To see this, set the font scale to say 1.99 then delete the .99 bit, the window does not resize until you touch the scale again, by adding the "." for example.
  4. On the tab map window, both the UI font size and the map mark font size effect the actually drawn font size.
  5. For the buddy window, the "add buddy" text is not well aligned vertically.
  6. For the user menus, main window. The separation of the menu titles is not maintained when scaling the UI font.
  7. The login window does not scale correctly if you change the font scaling. This is fixed on the next restart or if you touch the UI scaling.
  8. The options window looks a little short if you increase the UI font scale The text is all aligned correctly and the width is increased, but the height is not increased. This may be by design.
  9. In the new character window, The font scaling work OK but there are glitches. The <> controls do not scale well and the top label does not scale well. The top centre "Character Creation Screen" window does not scale when the font is changed at all. It does snap to the correct size if you touch the UI scaling.
  10. An uninitialised var warning:
    In file included from /home/paul/el/ttf/Eternal-Lands/vmath.h:13,
                 from /home/paul/el/ttf/Eternal-Lands/bbox_tree.h:5,
                 from /home/paul/el/ttf/Eternal-Lands/actors.h:15,
                 from /home/paul/el/ttf/Eternal-Lands/achievements.h:5,
                 from /home/paul/el/ttf/Eternal-Lands/elconfig.c:23:
    /home/paul/el/ttf/Eternal-Lands/elconfig.c: In function ‘get_elconfig_content_width’:
    /home/paul/el/ttf/Eternal-Lands/misc.h:242:21: warning: ‘line_width’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    242 |  return (x >= y)? x : y;
      |         ~~~~~~~~~~~~^~~
    /home/paul/el/ttf/Eternal-Lands/elconfig.c:3306:15: note: ‘line_width’ was declared here
    3306 |  int i, iopt, line_width, max_line_width = 0;
      |               ^~~~~~~~~~

    I'm also wondering again about the FSAA stuff on Linux. The fonts I'm using look a bit messy with FSAA not enabled fully. That is using the fsaa_glx.c module rather than the fsaa_dummy.c module that is the default in the current build. I'm doing a bit of research to understand how this works but I'd suggest we turn it on the allow the new TTF usage to shine fully.

This change is looking really good. Apart from potentially fixing the glitches above, are you planning any other major changes for this pull request? Could it soon be time to merge, build some test clients and get further testing help?

gvissers commented 4 years ago

Once more, thanks for the thorough testing.

I've tried out the latest changes. The trouble with the manufacture window buttons is fixed.

Yes, well... it may be hidden. (Rounded) buttons now scale the font size if the text is too wide. So you may just be seeing the effect of that.

I've also had quite a run though the various windows and have found some minor issues:

1. The item list has a "+" in the right side.  This does not maintain its central vertical position when changing the UI font scale.  Also, for font scaling, the list window is not moved as the inventory window increases in size due to font changes.

The + issue is fixed, but I cannot figure out what you mean by the second. Maybe I am looking at the wrong thing, but when I change the font size, and the inventory window is expanded or reduced, the list window moves with it.

2. When using UI scaling for the Book window, the navigation bar at the bottom of the window is no longer moved as the scale changes.  Touching the font scale fixes the position.

Fixed, but while doing so I found a layout issue that seems to trigger for some very specific font scales/book contents/something else that I don't yet know affects it, that needs some more investigation. Text is rendered outside the page, so that's a no go.

3. For the NPC list of quest log, the window does not resize as the scale is changes, it is always one resize behind.  To see this, set the font scale to say 1.99 then delete the .99 bit, the window does not resize until you touch the scale again, by adding the "." for example.

This may also be a problem in the main branch, though perhaps it is not triggered by the UI scaling: the minimum window size was set after resizing the window, so that the resize took the old minimum size into acoount. Fixed in ttf branch.

5. For the buddy window, the "add buddy" text is not well aligned vertically.

There are two issues here. First is that the button height was fixed at ui_scale * 20 pixels. This is solved, so the alignment issue is less noticeable now. The other is that vertical alignment of text is, in general, a pain. Different fonts have different ideas on how much different characters ascend or descend from the baseline. What I could do in this specific instance, because this is the only button in the window, is center vertically based on the button content. I don't want to do this for all buttons in general though, because (aligned) buttons in the same window could then have different internal alignment because of different content.

6. For the user menus, main window.  The separation of the menu titles is not maintained when scaling the UI font.

Well, well, there's always something new to learn. Never used user menus, so this was never tested. Fun feature, though I doubt I'll use it again :) Fixed now.

7. The login window does not scale correctly if you change the font scaling.  This is fixed on the next restart or if you touch the UI scaling.

How do you...ahhhhh... :man_facepalming: Would you believe that in probably 20+ years I never noticed there is also a settings button on the login window? :astonished: Ok, will fix.

8. The options window looks a little short if you increase the UI font scale  The text is all aligned correctly and the width is increased, but the height is not increased. This may be by design.

Not by design at least :). I'll have a look and see if I think it needs to scale vertically as well.

9. In the new character window, The font scaling work OK but there are glitches.  The <> controls do not scale well and the top label does not scale well.  The top centre "Character Creation Screen" window does not scale when the font is changed at all. It does snap to the correct size if you touch the UI scaling.

Will check. The new character window is a bit odd in places, so I'm not surprised there are some issues left.

10. An uninitialised var warning:

Thought I fixed that? Maybe it wasn't pushed out yet.

I'm also wondering again about the FSAA stuff on Linux. The fonts I'm using look a bit messy with FSAA not enabled fully. That is using the fsaa_glx.c module rather than the fsaa_dummy.c module that is the default in the current build. I'm doing a bit of research to understand how this works but I'd suggest we turn it on the allow the new TTF usage to shine fully.

Yeah I was wondering about that when I saw you first screenshots, I thought things looked better on my screen. When you first mentioned FSAA I played a bit with it in settings menu, but I didn't notice any difference, so ... dunno. I may have to look at it further to see if I can find out which aliasing settings are actually used on my machine.

This change is looking really good. Apart from potentially fixing the glitches above, are you planning any other major changes for this pull request? Could it soon be time to merge, build some test clients and get further testing help?

I don't plan on any more big changes, the only thing I can think of is perhaps adding one or two more vertical alignment options (\see e.g. pt.5), which should be a relatively minor operation. I'll let you know when I've finished fixing the remainder of this list, by that time it should probably be good enough for wider testing.

pjbroad commented 4 years ago

The second part of 1. above was fixed when you added the font change handler. As the inventory window increases in size, the item list window should be moved so that it stays to the right/left and does not overlap.

  1. I've found another issue. The password manager window does not scale properly for font scaling. The text does off the bottom of the window and the checkbox is not aligned with the label at the bottom.

I've been reading and experimenting with the FSAA stuff. This page is interesting and suggests, to use it cleanly, we need to remove some other settings like the use of GL_LINE_SMOOTH and others scattered around the client. Probably why you don't really notice the difference.

pjbroad commented 4 years ago

All those changes look to be working great. Sorry, another batch of testing....

  1. The popup window (popup.c) you get for the summoning menu does not scale for font changes. What I did with the UI scaling was simply close the window and leave it to the next time the window was opened. The font scaling works fine the next time you open the window. The only way to easily test this is to summon something, but you can now press CTRL+U to open the window.

  2. The serverpopup window (serverpopup.c) also does not scale. For some reason uses the chat font (I wrote the code many many years ago so who know why). You should be able to just hook into the UI scaling handler like you have done elsewhere. When testing this I usually hack in a quick #command to open the window.

The rest are minor :)

  1. I see you have implemented more bounds checking and red highlighting for option spin boxes - nice. Did you mean it to kick in for the lower bound you can reach via the spin buttons but not the upper?

  2. The quantity numbers for the item lists and the quick items window are not well aligned with the slot when the UI font changes. They are fine elsewhere.

  3. The category buttons for the counters, overlap a bit at low font sizes.

  4. The inventory button text reaches outside the button bounds a bit, even with the value clamped.

  5. The hover text on the right size hud, looks very close to the hud border. Previously there was a small gap that I think looked better.

  6. Rules and Encyclopaedia windows have different font and size controls, but share a parent window. It looks odd that if you scale the encyclopaedia font you get a big window but the rules font stays the same. This is even more odd if you do this while viewing the rules. Nothing you can do I guess but is a little odd. I wondered if having a single font size would be better but you could still have different fonts so, perhaps just leave it.

  7. The options window is sill short when the font is scaled. Did you decide to leave it that way?

gvissers commented 4 years ago
1. The popup window (popup.c) you get for the summoning menu does not scale for font changes.  What I did with the UI scaling was simply close the window and leave it to the next time the window was opened. 

Okay, using the same cop-out :)

The font scaling works fine the next time you open the window. The only way to easily test this is to summon something, but you can now press CTRL+U to open the window.

Well, that must have been the first time in years I've summoned anything. Thanks for giving me the opportunity :P BTW, Ctrl+U for the summoning window clashes with K_USE. Not that I've used either of them, ever, but still.

2. The serverpopup window (serverpopup.c) also does not scale.  For some reason uses the chat font (I wrote the code many many years ago so who know why).  You should be able to just hook into the UI scaling handler like you have done elsewhere. 

Unfortunately, no, as the UI scaling code does not scale the text. I had lots of "fun" with this one, as it took me quite a while to figure out why the window kept changing size (and earlier on, position) without me changing the font size. See the updated "fudge" comment in the code.

When testing this I usually hack in a quick #command to open the window.

I hijacked a channel :)

1. I see you have implemented more bounds checking and red highlighting for option spin boxes - nice.  Did you mean it to kick in for the lower bound you can reach via the spin buttons but not the upper?

TBH, I just fixed the code that was already in place (the red highlighting was done, but immediately overridden by the default color). Too high numbers are currently not highlighted, because the code immediately replaces too high numbers with the upper bound (again, that was already in place). The same cannot be done for the lower bound though, since it would then be impossible to e.g. enter 0.78 when the lower bound is 0.75, I would've liked to correct the lower bound when the spinbutton loses focus (like a javascript onchange()), but there is currently no support for such an event in EL, and hacking it in was a bit outside the current scope.

2. The quantity numbers for the item lists and the quick items window are not well aligned with the slot when the UI font changes.  They are fine elsewhere.

Fixed.

3. The category buttons for the counters, overlap a bit at low font sizes.

Hmm, here the buttons come close but do not touch or overlap. [Not so sure about that anymore, might have set the font size too large still]. But I agree that having one or a few pixels separation might look nicer. I'll have a look at the multiselect widget (not done yet) [Fixed now].

4. The inventory button text reaches outside the button bounds a bit, even with the value clamped.

Yeah, I noticed that as well. I don't want to increase the button size since they align nicely with the grid, so the font size is now reduced for these buttons if it grows too large.

5. The hover text on the right size hud, looks very close to the hud border.  Previously there was a small gap that I think looked better.

I agree. Fixed.

6. Rules and Encyclopaedia windows have different font and size controls, but share a parent window. It looks odd that if you scale the encyclopaedia font you get a big window but the rules font stays the same.  This is even more odd if you do this while viewing the rules.  Nothing you can do I guess but is a little odd.  I wondered if having a single font size would be better but you could still have different fonts so, perhaps just leave it.

Fow now, I'll leave it as is. I agree it looks odd, but people will have to explicitly change the font size to see the effect, and at that point they will probably also know how to undo it. TBH, I was surprised there was such a thing as a separate rules font at all. I think, that if I ever get round to rewriting the encyclopedia, it would be better to combine the encyclopedia and rules fonts into a a single help font.

7. The options window is sill short when the font is scaled.  Did you decide to leave it that way?

I was still on the fence about that. On the one hand, I agree it looks somewhat odd for large font sizes, on the other hand it takes up so much screen estate if we increase the size vertically as well. I think with the reduced size range, it is not too bad, though, so I think I will leave it as is.

pjbroad commented 4 years ago
  1. The latest changes look to have fixed all but the counters issue. I'm still seeing same overlap at small font sizes. I think you say you have fixed it above.

  2. I'm also seeing a bigger issue with all tabs on the statistics window (counters etc), not setting the window size correct when opened for the first time. I set the font small but increased the UI scale then saw the problem on restart. Touching either the font of UI scale immediately sets the window size correctly.

  3. I hadn't checked the dialogue window before. I saw some issues with the text not staying inside the window for some combinations of font and UI scale; for example if you change the font scale, then the UI scale. If you touch the font scale, the text wraps correctly.

  4. I've also tried to compile the map editor - it no longer compiles.

gvissers commented 4 years ago
1. The latest changes look to have fixed all but the counters issue.  I'm still seeing same overlap at small font sizes.  I think you say you have fixed it above.

Sigh. Forgot to push again.

2. I'm also seeing a bigger issue with all tabs on the statistics window (counters etc), not setting the window size correct when opened for the first time.  I set the font small but increased the UI scale then saw the problem on restart.  Touching either the font of UI scale immediately sets the window size correctly.

Not seeing this, but it's possible that the fix for 1. also solved this. If not, please let me know a font name and scale factors so that I can try to reproduce it.

3. I hadn't checked the dialogue window before.  I saw some issues with the text not staying inside the window for some combinations of font and UI scale; for example if you change the font scale, then the UI scale.  If you touch the font scale, the text wraps correctly.

Yes, I naively assumed that scaling the UI does not affect the line breaks, but due to rounding the character widths it can. Line breaks are now also recomputed on UI scale change, so should be fixed now.

4. I've also tried to compile the map editor - it no longer compiles.

Fixed. BTW, I believe there are only two projects left in the code base: the client and the map editor. What is the preferred define for restricting code to the client: #ifdef ELC or #ifndef MAP_EDITOR? Can we get rid of either one of them at some point? And then there's also MAP_EDITOR2 hanging around which I believe nothing in the code ever sets...

pjbroad commented 4 years ago
  1. The map editor builds fine. I agree we should sort out the #defs but perhaps after merging this PR?

  2. The dialogue window is fixed.

  3. The counter category buttons no-longer overlap but the button height reaches a minimum size and does not scale down further. Leading to some very small text in tall buttons.

  4. The issue with the widgets extending beyond the window still exists. I'm using the "ubuntu mono regular" font at size 1.00. If you set the stats window UI scale to say 1.25 then close the client. On restart when you open the window the widgets are larger than the window. Simply touching the font or UI scale options immediately set the correct window size.

  5. Looks like you left some debug in elconfig.c, line 1141 "printf("fsaa = %#x\n", fsaa);".

  6. I see you have remove the limits on the font sizes again. Is that how you are going to leave it?

gvissers commented 4 years ago
1. The map editor builds fine.  I agree we should sort out the #defs but perhaps after merging this PR?

Of course.

3. The counter category buttons no-longer overlap but the button height reaches a minimum size and does not scale down further.   Leading to some very small text in tall buttons.

Yes, this is by design. Changing the UI font size is meant to adjust for some fonts looking just a tad too small, so the button height is not scaled. If you wish to make everything larger, you're better off using the global UI scale. The only reason the buttons scale horizontally with UI font changes is because they run out of space so easily. With the restricted UI font range (which I fully intend to reinstate), I think the buttons will look allright.

4. The issue with the widgets extending beyond the window still exists.  I'm using the "ubuntu mono regular" font at size 1.00.  If you set the stats window UI scale to say 1.25 then close the client.  On restart when you open the window the widgets are larger than the window.  Simply touching the font or UI scale options immediately set the correct window size.

Window-specific scale, check. Should be fixed now.

5. Looks like you left some debug in elconfig.c, line 1141 "printf("fsaa = %#x\n", fsaa);".

Fixed.

6. I see you have remove the limits on the font sizes again.  Is that how you are going to leave it?

Nope :)

pjbroad commented 4 years ago

Looking good! I see you have removed some of the use of the gx/gy_adjust stuff. For me when I turn off TTF, my fonts are no longer correctly positioned relative to lines and I cannot correct them now. I can see this in tabs, and buttons etc. This was always a hack but worked across different systems (they use different values). You're probably fed up with me mentioning this but any ideas what to do?

Edit: I just noticed that the quantity numbers for the trade window are not aligned correctly to the grid with or without TTF enabled.

gvissers commented 4 years ago

Looking good! I see you have removed some of the use of the gx/gy_adjust stuff. For me when I turn off TTF, my fonts are no longer correctly positioned relative to lines and I cannot correct them now. I can see this in tabs, and buttons etc. This was always a hack but worked across different systems (they use different values). You're probably fed up with me mentioning this but any ideas what to do?

I would prefer to burn them with fire, but that's probably not an option :)

I've actually only now gotten to the point where we can start pixel fiddling. If at all possible, I would like to remove the need for g?_adjust, so let's see how far we can get. Let me start with a picture of how the default font looks on my system: settings This is with UI scale=2, and font scale obviously 1. Numbers in the spin boxes are aligned well, IMO. The text in the input line looks a bit high, but adding a few characters reveals why: textfield Could be set a pixel lower, perhaps. Tab heading: again slightly above center, but also here there are no characters that descend below the baseline for these texts. Alignment of the text with the check boxes could be better, looks a bit like the line spacing is all added at the bottom. I'll take a look to see if I can fix that. Horizontal alignment I'm pretty happy with.

Update: Nope, no extra line sapcing added at the bottom, the font is just difficult: checkbox

Could you perhaps share a few examples of where you would use the adjustment, and also explain why there are differences between different systems (I can imagine UI scaling being a factor, obviously). And are these adjustments for specific buttons/tabs/whatever, or should they be applied to an entire class of widgets?

As a final note: there is an option to center text vertically based on contents alone (which as I realize now would not yet work with the bundled fonts, as I haven't gone through the fonts yet and added the top and bottom offsets of all characters). Anyway, that could be made to work, and possibly help in cases where e.g. all text is above the baseline. Of course, this is not an option if the text has to be aligned between different widgets.

Worst case scenario we add the adjustments back :)

Edit: I just noticed that the quantity numbers for the trade window are not aligned correctly to the grid with or without TTF enabled.

Looks like they never were. They are now, though :)

pjbroad commented 4 years ago

I think we're pretty close to removing the gx/gy_adjust. I've produced some screen shots of clients using the latest ttf code but with ttf disabled and using the exact same config files (copied) so scaling etc are the same. I have my normal desktop (previously best setting -1,1) and from a raspberry pi (previously best setting -1,0). With gx/gy set to 0,0 most things look just fine. The major differences are with the shape of the tabs and the "+" on the selected channel chat. I also think the quantity buttons on the inventory show a difference vertically.

I've been looking at the tab drawing code and have a much simpler version that nearly fixes the issue. I've not looked at the "+" on the chat tab but the replacement you did for the manu and item list windows does not show the same issue and is aligned just fine on both systems. Perhaps its because you are using QUADs rather than LINES.

Edit: The vertical space for the right-hand hud stats looks a bit short too.

gvissers commented 4 years ago

I've been looking at the tab drawing code and have a much simpler version that nearly fixes the issue. I've not looked at the "+" on the chat tab but the replacement you did for the manu and item list windows does not show the same issue and is aligned just fine on both systems. Perhaps its because you are using QUADs rather than LINES.

I'm reasonably sure the problem with the '+' in the channel chat tab is that the length is an even number of pixels (4), while a single pixel crossbar has an odd-numbered height so it cannot be aligned exactly in the middle. Of course, if the UI scale is large enough (EDIT: or an even integer multiple), the difference becomes less noticable. The plus signs i drew in the manu and item list windows avoid this by enuring both numbers are even. All right, that was complete and utter nonsense, the bars are 7 pixels long before scaling. But, interestingly, the vertical bar is drawn at x=3, while the horizontal bar is drawn at y=4. On my system, that makes the vertical bar stand slightly to the left of the center. Another interesting thing is that scaling does not help in this instance, because difference is scaled also...

EDIT 2: And can I just mention that I find it absolutely hilarious/heroic/fantastic (take your pick) that EL can be made to run on a RPi?

EDIT 3:

The vertical space for the right-hand hud stats looks a bit short too.

WHat do you mean, the spacing between the quickbar and the stats, or the height of the stats buttons themselves. Both look like they could use an extra pixel or two to me.

EDIT 4:

I also think the quantity buttons on the inventory show a difference vertically.

I may be able to fix that. After I finish the system updates on this laptop.

pjbroad commented 4 years ago

I've committed the tab clean up to the master branch. Sorry, it conflicts with this branch but its trivial to resolve.

pjbroad commented 4 years ago

WHat do you mean, the spacing between the quickbar and the stats, or the height of the stats buttons themselves. Both look like they could use an extra pixel or two to me.

The latter. The individual bars do not quite fix the text, but both would be OK.

pjbroad commented 4 years ago

...I've not looked at the "+" on the chat tab but the replacement you did for the manu and item list windows does not show the same issue and is aligned just fine on both systems. Perhaps its because you are using QUADs rather than LINES.

~I'm reasonably sure the problem with the '+' in the channel chat tab is that the length is an even number of pixels (4), while a single pixel crossbar has an odd-numbered height so it cannot be aligned exactly in the middle. Of course, if the UI scale is large enough (EDIT: or an even integer multiple), the difference becomes less noticable. The plus signs i drew in the manu and item list windows avoid this by enuring both numbers are even.~ All right, that was complete and utter nonsense, the bars are 7 pixels long before scaling. But, interestingly, the vertical bar is drawn at x=3, while the horizontal bar is drawn at y=4. On my system, that makes the vertical bar stand slightly to the left of the center. Another interesting thing is that scaling does not help in this instance, because difference is scaled also...

I could not get this to work with a plus made of single pixel lines. On both machines it was one pixel off centre but in different ways on the two machines. I tired the quads method with a fixed thickness of two pixels and that works just fine on both, and at all scales. I've committed this change to the master branch.

gvissers commented 4 years ago

I tired the quads method with a fixed thickness of two pixels and that works just fine on both, and at all scales. I've committed this change to the master branch.

Looks good, but now I think the close button looks a bit thin in comparison. How about adding

diff --git a/chat.c b/chat.c
index 9d3b23c9..99b4e67b 100644
--- a/chat.c
+++ b/chat.c
@@ -1923,12 +1923,16 @@ static int draw_tab_details (widget_list *W)
                }

        /* draw the closing x */
-       glBegin(GL_LINES);
-               glVertex2i(x-l4,y-l4);
-               glVertex2i(x+l3,y+l3);
-
-               glVertex2i(x-l4,y+l3);
-               glVertex2i(x+l3,y-l4);
+       glBegin(GL_QUADS);
+               glVertex2i(x-half_plus,   y-half_plus);
+               glVertex2i(x+half_plus-2, y+half_plus);
+               glVertex2i(x+half_plus,   y+half_plus);
+               glVertex2i(x-half_plus+2, y-half_plus);
+
+               glVertex2i(x+half_plus,   y-half_plus);
+               glVertex2i(x-half_plus+2, y+half_plus);
+               glVertex2i(x-half_plus,   y+half_plus);
+               glVertex2i(x+half_plus-2, y-half_plus);
        glEnd();
        glEnable(GL_TEXTURE_2D);
 #ifdef OPENGL_TRACE

Feels a bit silly to use quads for what is essentially two lines, but I think it looks better.

pjbroad commented 4 years ago

I tired the quads method with a fixed thickness of two pixels and that works just fine on both, and at all scales. I've committed this change to the master branch.

Looks good, but now I think the close button looks a bit thin in comparison. How about adding ...

Yes, I was thinking the same. It does mean the UI scaling changes I did previous (using repeated constants) are now looking shabby. I'll fix that at the same time and push to master in a bit.

pjbroad commented 4 years ago

I tired the quads method with a fixed thickness of two pixels and that works just fine on both, and at all scales. I've committed this change to the master branch.

Looks good, but now I think the close button looks a bit thin in comparison. How about adding ...

Yes, I was thinking the same. It does mean the UI scaling changes I did previous (using repeated constants) are now looking shabby. I'll fix that at the same time and push to master in a bit.

OK. That's done and pushed to master. I used a variant of your code snippet.

Perhaps we'll get to burn the hack soon..... I was thinking of looking through the places that still have gx/gy adjust and comparing them across the two system I have. It feels like I might clash with stuff you doing now so how to you want to coordinate?

Edit: I'll at least take a look at the non-text drawing related bits.

gvissers commented 4 years ago

I was thinking of looking through the places that still have gx/gy adjust and comparing them across the two system I have. It feels like I might clash with stuff you doing now so how to you want to coordinate?

Apart from taking a look at the stats bars, I don't have immediate plans for this branch. Also, I don't have a lot of spare time this week, so I don't think I 'll get to do much work on EL. So if you want to look at the coord adjustments, go ahead.

If you're thinking about merging this branch at some point, now might be a good time to do it, though. Otherwise, you might end up going over things that have already been removed in the ttf branch.

brunoramoslu commented 4 years ago

When chancing the UI Text Size and if you have the digital clock on showing the seconds, with some values you do not see the last digit of the seconds.

In my example changing from 0.83 to 0.84 makes the seconds visible again.

image

pjbroad commented 4 years ago

Looks like make without TTF defined is not working. -- Fixed, didn't know I could commit to your branch.

The right hand hub stats bars have green text when the stat is selected, it was brown before.

gvissers commented 4 years ago

When chancing the UI Text Size and if you have the digital clock on showing the seconds, with some values you do not see the last digit of the seconds.

Fixed, thanks for reporting

The right hand hub stats bars have green text when the stat is selected, it was brown before.

Yeah, I already thought it looked unfamiliar. Fixed.