Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AGS: Only load ttf font as outline for ttf font #3361

Merged

Conversation

@antoniou79
Copy link
Contributor

@antoniou79 antoniou79 commented Sep 11, 2021

Possible fix for https://bugs.scummvm.org/ticket/12923

This is quite an oblivious fix, though, since I use FontInfo Flags field (which as far as I can tell is unused) to set bit flags to track if a loaded font uses a TTF or WFN renderer and also I renamed a method (get_outline_font() to get_outline_ttf_font()) to specify that it's specifically for TTF fonts use cases.

Maybe there's a better way of fixing this.

@dreammaster dreammaster merged commit d561d86 into scummvm:master Sep 15, 2021
8 checks passed
@dreammaster
Copy link
Member

@dreammaster dreammaster commented Sep 15, 2021

Actually, I had to revert the commit, sorry. It reintroduced the very problem that the "outilne font" commits worked around in the first place. Try running Maniac Mansion Deluxe, and skipping the intro. As the three selected characters chat as they arrive at the mansion, you'll see outlines getting offset from the solid fonts on top of them.

@antoniou79
Copy link
Contributor Author

@antoniou79 antoniou79 commented Sep 15, 2021

Actually, I had to revert the commit, sorry. It reintroduced the very problem that the "outilne font" commits worked around in the first place. Try running Maniac Mansion Deluxe, and skipping the intro. As the three selected characters chat as they arrive at the mansion, you'll see outlines getting offset from the solid fonts on top of them.

Right, sorry for that. Thank for looking into it.
For reference, I checked my changes again today and there were a number of issues:

  1. Setting the fonts as TTF or WFN in wloadfont_size() was (almost always?) overridden by the code below that set the Info filed from the font_info parameter of wloadfont_size():
    _GP(fonts)[fontNumber].Info = font_info;

    I completely missed this.
  2. Searching for fontinfo in the engine code, I can find numerous other places where fontinfo is set explicitly from Flags, or Flags in fontInfo are set from data parameters. And also there are already defines for bit flags for the Flags field (ie. ). Eg. apparently FFLG_SIZEMULTIPLIER (1) is a legacy one which directly conflicts with me setting FONT_TTF_RENDERER to 1.
  • SetFontInfoFromLegacyFlags() and AdjustFontInfoUsingFlags() in ags\shared\ac\game_setup_struct.cpp
  • set_fontinfo() (which seems unused) in ags\shared\font\fonts.cpp
  • UpgradeFonts() in ags\shared\game\main_game_file.cpp
    There's just potential that my fix introduces regressions with those too.
    I also seem to completely have missed this.
    So overall, this PR would need a big rework and check all the affected cases to work.
    Since just removing the assert in TFFontRenderer::RenderText() is a more simple fix, it's probably better to ignore this PR.

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