mosra / magnum

Lightweight and modular C++11 graphics middleware for games and data visualization
https://magnum.graphics/
Other
4.76k stars 438 forks source link

[Possible Regresion] [Firefox] [loading font] Text not displaying on linux firefox 93 and X11- since commit "f360c19426684218006933cd1f6a60851c9dcfbc" #538

Closed werto87 closed 2 years ago

werto87 commented 2 years ago

My project is based on this magnum example "https://doc.magnum.graphics/magnum/examples-imgui.html". Loading a different font worked before commit "f360c19426684218006933cd1f6a60851c9dcfbc". Code for loading a different font:

  ImGuiIO &io = ImGui::GetIO ();
  ImVector<ImWchar> ranges;
  ImFontGlyphRangesBuilder builder;
  builder.AddText (from_u8string (std::u8string{ u8"♠♥♦♣" }).c_str ());
  builder.AddRanges (io.Fonts->GetGlyphRangesDefault ());
  builder.BuildRanges (&ranges);
  auto const fontFile = std::filesystem::path{ "bin/asset/DejaVuSans.ttf" };
  auto const fontSize = 100.0f;
  smallFont = io.Fonts->AddFontFromFileTTF (fontFile.c_str (), fontSize * 0.5f, nullptr, ranges.Data);
  bigFont = io.Fonts->AddFontFromFileTTF (fontFile.c_str (), fontSize * 0.5f * 1.2f, nullptr, ranges.Data);
  io.Fonts->Build ();

If I use this commit "f360c19426684218006933cd1f6a60851c9dcfbc". There is no Text in my app. I only see the special chars "♥♦♣". Note that I do not see "♠" and some of the symbols are in the wrong place. If the code above is commented out the text works fine.

Some Information: Problem happens in firefox 93.0 on arch linux using X11 and on windows with firefox 93.0. On android mobile with firefox 93.2 it crashes. When I use chrome everything works. Magnum console output Mozilla Firefox 93.0 on arch linux using X11:

Renderer: GeForce GTX 980/PCIe/SSE2 by Mozilla project.js:1:40379
OpenGL version: OpenGL ES 2.0 (WebGL 1.0) project.js:1:40379
Using optional features: project.js:1:40379
    GL_ANGLE_instanced_arrays project.js:1:40379
    GL_EXT_texture_filter_anisotropic project.js:1:40379
    GL_OES_vertex_array_object project.js:1:40379
    GL_WEBGL_draw_buffers project.js:1:40379
Using driver workarounds: project.js:1:40379
    firefox-deprecated-debug-renderer-info project.js:1:40379
    emscripten-pthreads-broken-unicode-shader-sources

I tried to reproduce this in a small project but it did not work. So I think the problem is loading a different font and doing something else. Source code of my project where the problem happens Link to the broken version of my website (click on fullscreen to start)

mosra commented 2 years ago

Hi, thanks for the report!

Indeed, the text appears in version 91, but not in 92 or 93. It's however strange that commit f360c19426684218006933cd1f6a60851c9dcfbc would cause such an issue, it only checks for browser version... So, just to be clear it's not somewhere else, if you use 13d62634d88608b263d5b659a738fe87840089c8 (the parent commit), everything works? Or that one has the same problem?

I tried to disable the newly-added and potentially offending extension and workaround using https://modern-durak.com/?magnum-disable-extensions=GL_WEBGL_debug_renderer_info&magnum-disable-workarounds=firefox-deprecated-debug-renderer-info, but seems like you're not passing the GET parameters to the application like the default Magnum runner does, so it had no effect. Could you update it so this kind of argument passing is possible? Makes remote debugging so much easier ;) The corresponding piece of JS code that does the magic is here: https://github.com/mosra/magnum/blob/c9a0ffd70ad55142038ef645524a760e9398b7d7/src/Magnum/Platform/EmscriptenApplication.js#L86-L98

Then, with the arguments propagated to the application, if you enter the above URL and the startup log correctly shows the extension as disabled and the workaround not as used, does the text appear properly? Or neither?

Thanks.


On android mobile with firefox 93.2 it crashes.

Hmm, potentially related to the crash -- if I press Fullscreen, click on a bunch of things and then escape the fullscreen again using Esc, the memory usage goes crazy and I have to kill the browser before the browser kills me:

image

Happens on FF 91, 92 and 93, independently of whether the text is broken or not. Not sure why that happens, but might be related.

werto87 commented 2 years ago

Hi mosra,

I double checked it for Mozilla Firefox 93.0 on arch linux with x11. "f360c19426684218006933cd1f6a60851c9dcfbc": Has no text "13d62634d88608b263d5b659a738fe87840089c8": Has text

I will try to add the option to pass the flags or if this does not work for me I will host a version build with "13d62634d88608b263d5b659a738fe87840089c8" and with "f360c19426684218006933cd1f6a60851c9dcfbc". Should I build them in debug?

mosra commented 2 years ago

Thanks for the confirmation, very useful info.

A debug build is not necessary (the compilation to wasm loses most of the useful info in the process anyway), but the argument propagation is rather essential to be able to pin down what in the commit actually causes the problem.

Alternatively, in case you don't manage to propagate the flags, you could try changing out parts of the code added by the commit. I suspect the javascript code querying the renderer: https://github.com/mosra/magnum/blob/03aeb4971f6fd97b152f742d55612072b07e720c/src/Magnum/GL/Implementation/driverSpecific.cpp#L585-L589

might cause some issues, if you replace it with just

const Int firefoxVersion = 93;

does it work? If not, and if you replace it with

const Int firefoxVersion = 0;

does that work? The latter is basically the same as adding ?magnum-disable-workarounds=firefox-deprecated-debug-renderer-info to the URL.

Thanks again!

werto87 commented 2 years ago

hi mosra, It looks like the problem comes from cxxopts. I use cxxopts as option parser library and then I fake the options to use Magnum::Platform::Sdl2Application::Arguments.

namespace
{
Corrade::Containers::Pointer<ImGuiExample> emscriptenApplicationInstance;
}

int
main (int argc, char **argv)
{
  cxxopts::Options options ("modern-durak-client", "A brief description");
  // clang-format off
      options.add_options()
        ("w,websocket-url", "url to websocket --websocket-url wss://www.example.com/socketserver", cxxopts::value<std::string>()->default_value("wss://localhost:55555"))
        ("t,touch", "set this option if input type is touch and not keyboard")
        ("h,help", "Print usage")
    ;
  // clang-format on
  auto result = options.parse (argc, argv);
  if (result.count ("help"))
    {
      std::cout << options.help () << std::endl;
      exit (0);
    }
  int tmpArgc = 1;      // work around to ignore commandline input for magnum imgui
  char *tmpArgv[1];     // work around to ignore commandline input for magnum imgui
  tmpArgv[0] = argv[0]; // work around to ignore commandline input for magnum imgui
  auto websocketUrl = result["websocket-url"].as<std::string> ();
  if (websocketUrl == "{{WEBSOCKET-URL}}")
    {
      websocketUrl = "wss://localhost:55555";
      std::cout << "websocketUrl == \"{{WEBSOCKET-URL}}\" using " << websocketUrl << std::endl;
    }
  emscriptenApplicationInstance.reset (new ImGuiExample{ { tmpArgc, tmpArgv }, result["touch"].as<bool> (), websocketUrl });
  emscriptenApplicationInstance->redraw ();
}

When I comment out the cxxopts thing and hard code the options it works.

namespace
{
Corrade::Containers::Pointer<ImGuiExample> emscriptenApplicationInstance;
}

int
main (int argc, char **argv)
{
  emscriptenApplicationInstance.reset (new ImGuiExample{ { argc, argv }, false, "wss://localhost:55555" });
  emscriptenApplicationInstance->redraw ();
}

I do not understand the problem and why it happens after the firefox-commit. Is there some support for custom options in magnum?

mosra commented 2 years ago

When I comment out the cxxopts thing and hard code the options it works.

Just to be sure I understand everything correctly -- if you don't use cxxopts, then the ImGui text is visible even on Firefox 93 and everything just works? Is that right?

In that case I fear you have some really bad case of memory corruption somewhere, something writing somewhere it shouldn't.... and the firefox workaround commit only changed the memory layout in a certain way, causing the memory corruption to happen in a way that has a visual effect in the application. If you have a native build, try building with Address Sanitizer (Linux and macOS, recent versions of MSVC on Windows should have it also), that's the easiest path to discovering such problems.

Just few minutes ago I was building the imgui example from the examples ports branch (where it uses a custom font as well) and it worked well... then I saw your reply :)

Is there some support for custom options in magnum?

Yes, happy to help -- Utility::Arguments. Just make sure to skip the magnum prefix so the magnum-specific options can still be passed in case there's again a need to debug something in the future. In your case, the options could look like this, the syntax is nothing fancy, just the usual method chaining:

Corrade::Utility::Arguments args;
args.addSkippedPrefix("magnum", "engine-specific options")
    .addOption('w', "websocket-url", "wss://localhost:55555").setHelp("websocket-url", "url to websocket")
    .addBooleanOption('t', "touch").setHelp("touch", "set this option if input type is touch and not keyboard")
    .setGlobalHelp("A brief description")
    .parse(argc, argv); // will print help and exit if -h or --help is passed

...  
std::string websocketUrl = args.value("websocket-url");

... new ImGuiExample{ {argc, argv}, args.isSet("touch") } ...

Magnum itself then consumes only the options prefixed with magnum-, so whatever you specify in args will be ignored by it.

werto87 commented 2 years ago

Just to be sure I understand everything correctly -- if you don't use cxxopts, then the ImGui text is visible even on Firefox 93 and everything just works? Is that right?

Yes. If I do not use cxxopts it works.

I managed to reproduce the error in a small example based on the imgui example. small example to reproduce the error

I will try to use the Sanitizers.

Thank you for the Argument example.

werto87 commented 2 years ago

I added address sanitizer "-fsanitize=address" which fixes the missing text problem. So I think you are correct here

In that case I fear you have some really bad case of memory corruption somewhere

werto87 commented 2 years ago

Hi mosra, thanks for your help with the arguments. I replaced my old solution with yours and now I only use one argument parser and I am also able to pass arguments in the url. I do not think that the firefox changes are the reason for my problem.

Just few minutes ago I was building the imgui example from the examples ports branch (where it uses a custom font as well) and it worked well... then I saw your reply :)

Do you have an example on how to load a font and add special letters. I load DejaVuSans.ttf and add the symbols "♠♥♦♣". I think this could be the issue for me.

mosra commented 2 years ago

(Sorry for the late reply, things piled up since.)

Glad to hear the missing text issue was caused by some bug in cxxopts and not Magnum.

Do you have an example on how to load a font and add special letters

Unfortunately I don't, this is something that's ImGui-specific, and I don't use ImGui myself that much. If those glyphs don't get displayed, could it be that DejaVu simply doesn't have them? If the TTF file contains those, then please ask on ImGui forums instead, they'll be able to help you much better ;)