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

[reve]Draw text with SDF fonts in screen or world coordinates #15812

Merged

Conversation

alja
Copy link
Collaborator

@alja alja commented Jun 10, 2024

This Pull request:

Add functionality to draw Scale Distance Field Fonts in the screen or world coordinates.

Example:

/tutorials/eve7/texts.C

@alja
Copy link
Collaborator Author

alja commented Jun 10, 2024

This change is related to development in RenderCore UL-FRI-LGM/RenderCore#20

@alja
Copy link
Collaborator Author

alja commented Jun 10, 2024

@linev @osschar Please check.

@alja
Copy link
Collaborator Author

alja commented Jun 10, 2024

tutorials/eve7/texts.C screenshot

Screenshot 2024-06-10 at 3 41 35 PM

Copy link

github-actions bot commented Jun 10, 2024

Test Results

    13 files      13 suites   3d 3h 5m 0s ⏱️
 3 021 tests  3 021 ✅ 0 💤 0 ❌
33 762 runs  33 762 ✅ 0 💤 0 ❌

Results for commit 6dcb57a.

♻️ This comment has been updated with latest results.

Copy link
Member

@linev linev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have several questions:

  1. Do we need to include all these fonts into ROOT repository? Font files are big and used only via random generator in tutorial.

  2. How to deal with other fonts? Maybe one can include one default font in ui5/eve7/ folder, one or two fonts as examples in tutorials and provide recipe where other fonts can be found?

  3. If I understand correct, REveText rendering will be only supported with RenderCore? That about three.js? Beside fonts support it is easy to implement text rendering there.

  4. Is it possible to support other font formats - like TTF or OTF? ROOT already includes such fonts, is it an option?

@osschar
Copy link
Contributor

osschar commented Jun 11, 2024

Sergey, all good questions, thank you :)

  1. Do we need to include all these fonts into ROOT repository? Font files are big and used only via random generator in tutorial.

No, we can remove most of them, rebase and force-push to the branch.

  1. How to deal with other fonts? Maybe one can include one default font in ui5/eve7/ folder, one or two fonts as examples in tutorials and provide recipe where other fonts can be found?

Yes, we could do that. Font textures and metrics files are generated from TTF via https://github.com/osschar/sdf_atlas. It's a small tool, easy to build on linux -- so we could provide instructions for this and also a catalog/web-site with a bunch of pre-generated fonts.

  1. If I understand correct, REveText rendering will be only supported with RenderCore? That about three.js? Beside fonts support it is easy to implement text rendering there.

Yes, I know ... they have a monster implementation(s) of fonts. Seeing that I went looking for something super simple and still good looking :)

Now, this is a bit unfortunate ... but I don't think I have the bandwidth to keep Three fully supported. Also, the low-level, renderer- and shader-level support we are getting from RenderCore (in particular, for picking & rendering of instanced objects and the upcoming spline-based line rendering) is making it possible to support features and performance optimizations that I do not think would be doable in Three with the time budget we all have and the level of changes we can do in core Three (zero, unless we can hack over it locally).

  1. Is it possible to support other font formats - like TTF or OTF? ROOT already includes such fonts, is it an option?

This would be nice, sdf_atlas could be incorporated into root (it requires minimal GL support which we already have) -- and one could then generate the missing SDF fonts during the startup of a demo/application. License is free to use in whatever way, just keep the copyright notice. But it only supports TTF, not OTF. Perhaps we should look for another library ... or at least a new TTF/OTF parser -- I don't know much about low-level font things so this maybe already exists in root?.

I understand this is not all perfect -- but at this point our priority is to get something usable in for the existing REve applications.

@linev
Copy link
Member

linev commented Jun 12, 2024

but at this point our priority is to get something usable in for the existing REve applications.

Then I propose to reduce number of fonts - one or two for now.

And think how one can allow to use external fonts - in this PR or may be later.

@alja
Copy link
Collaborator Author

alja commented Jun 12, 2024

@linev I will add today the change to reduce the number of fonts. Just Serif and Mono Space type fonts in only regular style (no italic and bold).

@alja alja force-pushed the waad-revetext-overlay-master-6.33-rb1 branch from f048507 to 0c9b119 Compare June 13, 2024 17:55
@alja
Copy link
Collaborator Author

alja commented Jun 13, 2024

@linev I have pushed changes. I have rebased to master and squashed commits to reduce the load on git repository. There are some pending changes from Matevz. Please wait for his commit.

Copy link
Member

@linev linev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change still required


protected:
std::string fText {"<no-text>"};
std::string fFont {"LiberationSans-Regular"};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be default font name should refer to existing font?

@osschar osschar force-pushed the waad-revetext-overlay-master-6.33-rb1 branch from 0c9b119 to fcb3b3f Compare June 14, 2024 20:41
@osschar
Copy link
Contributor

osschar commented Jun 14, 2024

OK, I force-pushed a new version, now there are no SDF fonts in the repo -- but I have added two ttf fonts that have all the accented and greek characters.

The creator of sdf font stuff is in graf3d/gl as it requires open-gl for rendering of the glyphs. texts.C demo will now make sure the fonts are available in the web server repo under ui5/eve7/fonts/.

There is only one thing I couldn't figure out -- how to link against libpng ... so I just added -lpng into cmake file for graf3d/gl/. Please help me figure out how to do this correctly.

@osschar
Copy link
Contributor

osschar commented Jun 15, 2024

Sergey, semi-related, have been running with valgrind (root supressions on), and this came up:

root [1] ==975849== Thread 8 civetweb-worker:
==975849== Conditional jump or move depends on uninitialised value(s)
==975849==    at 0x4CD6584: R__longest_match (ZDeflate.c:430)
==975849==    by 0x4CD6EA8: R__Deflate (ZDeflate.c:677)
==975849==    by 0x4CD6084: R__memcompress (Bits.c:329)
==975849==    by 0x22699978: THttpCallArg::CompressWithGzip() (THttpCallArg.cxx:420)
==975849==    by 0x22693E4F: begin_request_handler(mg_connection*, void*) (TCivetweb.cxx:467)
==975849==    by 0x226CCE61: handle_request (civetweb.c:14331)
==975849==    by 0x226BCF09: handle_request_stat_log (civetweb.c:6544)
==975849==    by 0x226D3421: process_new_connection (civetweb.c:18629)
==975849==    by 0x226D4038: worker_thread_run (civetweb.c:19036)
==975849==    by 0x226D416F: worker_thread (civetweb.c:19097)
==975849==    by 0x535D896: start_thread (pthread_create.c:444)
==975849==    by 0x53E48C3: clone (clone.S:100)

@linev
Copy link
Member

linev commented Jun 17, 2024

@osschar

==975849== at 0x4CD6584: R__longest_match (ZDeflate.c:430)

It is inside zlib. Hope it is just false positive warning from valgrind.

@osschar
Copy link
Contributor

osschar commented Jun 17, 2024

Yeah, they probably do some quirky simd stuff in there :)

I realized yesterday that the stuff for creation of sdf textures always puts them in $ROOTSYS/ui5/eve7/ ... which will not be writable for experiment-wide / cvmfs installs. Would it be possible to do something like this:

  1. if $ROOTSYS/ is not writable, write to ./fonts/;
  2. forward requests for files in $ROOTSYS/ui5/eve7/ to some REveManager function that can then check where these fonts are actually available and route the GET to the proper local file?

@linev
Copy link
Member

linev commented Jun 17, 2024

There is only one thing I couldn't figure out -- how to link against libpng ... so I just added -lpng into cmake file for graf3d/gl/. Please help me figure out how to do this correctly.

Not so simple. Sometime ROOT uses system-wide libpng, sometime - builtin version included in ASImage.
And they can conflict with each other. Major problem - builtin with ASImage is not available from outside.

You have to ensure that your code not linked with ASImage. And provide in your CMakeLists.txt file construct like

find_Package(PNG)
# handle include directories

Because you may need to use custom include directories. See here

@osschar
Copy link
Contributor

osschar commented Jun 18, 2024

OK, I've moved png-writer to asimage.

All that remains from my side is to figure out how to handle the case when $ROOTSYS is not writable -- see my comment above: #15812 (comment)

@linev
Copy link
Member

linev commented Jun 18, 2024

to figure out how to handle the case when $ROOTSYS is not writable

One can try to load fonts from current directory.
http server uses http://server/currentdir/ alias for files from current directory.
One can use it to load fonts.
The only to be done - call win.SetUseCurrentDir(true); in REve initialization - because of security reasons option is by default off.

Feature available only in master since 2 days - please rebase your code

@osschar
Copy link
Contributor

osschar commented Jun 18, 2024

Hmmh, but then I'd need to stream path information with every REveText object, now only the font name is streamed. It is indeed a reasonable thing to check if the font files exist before object's json is sent over to the client --- but creating a font during the streaming traversal would be pushing it a bit.

Is it possible to register custom prefixes and callbacks from REveManager (via RWebWindow) so they can be handled when requests come in? Like: http://server/sdf-fonts/

I went through the code a bit (but clearly do not have the full picture) ... one way would be to add THttpServer::fActiveLocations, where instead of replacement string one provides a lambda [](TString& prefix, TString& reminder, THttpRequest& req, THttpServer &srv) so one can then do appropriate lookup in the callback, potentially generating the font, and then calling srv->SendFile() (or sending back the default font, if the desired one can not be found/generated).

I think this functionality could be useful for other cases in REve, where semi-static data needs to be provided.

The font-generation code in REveText invokes TGL generator through the interpreter now, via gROOT->ProcessLine(), to avoid dependency of REve on RGL. Is this OK to do from a request handler thread or should cross-thread request to the main thread be made (and request told to try again in N seconds)?

@linev
Copy link
Member

linev commented Jun 18, 2024

In my mind, procedure can be:

  1. Before starting server and display window, one creates fonts from TTF file and put them in current directory. Like REveManager::CreateFonts(....).
  2. Enable usage of current dir with win.SetUseCurrentDir(true)
  3. Font name is file name. If font name does not include path - then automatically currentdir/ is prepended.

@osschar
Copy link
Contributor

osschar commented Jun 18, 2024

Hmmh, so you don't want to give me an active directory? :)
This could be really useful for accessing large tables per-partes ... or pre-processed parts of geometry, you know ;)

@linev
Copy link
Member

linev commented Jun 19, 2024

Hmmh, so you don't want to give me an active directory? :)

For the moment there is no direct support of such feature with RWebWindow.
But can be provided - if really necessary.

In my mind, solution with direct access of font files via currentdir/ path is much clear.
And does not require any extra threads locking.

You always can implement active directory - but does it necessary with fonts?

@osschar
Copy link
Contributor

osschar commented Jun 20, 2024

OK ... done. I have used THttpServer::AddLocation() to register sdf-fonts/ to either $ROOTSYS/ui5/eve7/sdf-fonts or ./sdf-fonts. One can also specify the location manually through REveText::SetSdfFontDir().

Copy link
Member

@linev linev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add missing docu and adjust LinkDef.h file in graf3d/gl/

And of course problem with png.h should be solved.

graf2d/asimage/inc/TASPngWriter.h Outdated Show resolved Hide resolved
graf3d/gl/inc/TGLSdfFontMaker.h Outdated Show resolved Hide resolved
graf3d/gl/src/TGLSdfFontMaker.cxx Outdated Show resolved Hide resolved
@@ -0,0 +1,49 @@
#include "TASPngWriter.h"

#include <png.h>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On several platforms png.h is not found!

This peace of code should be placed directly in libAfterImage - only it has proper configuration for libpng usage. TASImage is just C++ decoration around.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if one does not use the builtin libAfterImage? This seems to be happening with the CI builds :(

@osschar
Copy link
Contributor

osschar commented Jul 25, 2024

I've moved the png low-level code into libAfterImage, added docs, and added the two classes (TGLSdfFontMaker and TASPngWriter) into appropriate link-defs.

@osschar
Copy link
Contributor

osschar commented Jul 25, 2024

Hmmh, it seems CI builds can not find the new file from graf2d/asimage/libAfterImage/:

  [ 86%] Building CXX object graf2d/asimage/CMakeFiles/ASImage.dir/src/TASPngWriter.cxx.o
  Error: /github/home/ROOT-CI/src/graf2d/asimage/src/TASPngWriter.cxx:3:10: fatal error: afterrootpngwrite.h: No such file or directory
      3 | #include <afterrootpngwrite.h>
        |          ^~~~~~~~~~~~~~~~~~~~~
  compilation terminated.

I had to do rm -rf AFTERIMAGE-prefix in my build dir to have the cmake install the new files. Is it possible the CI builds do something different here?

The F39 CI build log only has this:

  [  0%] Built target AFTERIMAGE

while mine has (grepped out AFTERIMAGE parts):

[  0%] Creating directories for 'AFTERIMAGE'
[  2%] Performing download step for 'AFTERIMAGE'
[ 10%] No update step for 'AFTERIMAGE'
[ 12%] No patch step for 'AFTERIMAGE'
[ 16%] Performing configure step for 'AFTERIMAGE'
-- AFTERIMAGE configure command succeeded.  See also /home/matevz/root-dev/dev-1-bld/AFTERIMAGE-prefix/src/AFTERIMAGE-stamp/AFTERIMAGE-configure-*.log
[ 85%] Performing build step for 'AFTERIMAGE'
-- AFTERIMAGE build command succeeded.  See also /home/matevz/root-dev/dev-1-bld/AFTERIMAGE-prefix/src/AFTERIMAGE-stamp/AFTERIMAGE-build-*.log
[ 97%] Performing install step for 'AFTERIMAGE'
-- AFTERIMAGE install command succeeded.  See also /home/matevz/root-dev/dev-1-bld/AFTERIMAGE-prefix/src/AFTERIMAGE-stamp/AFTERIMAGE-install-*.log
[ 97%] Completed 'AFTERIMAGE'
[ 97%] Built target AFTERIMAGE
[ 97%] Built target G__ASImage
[ 97%] Building CXX object graf2d/asimage/CMakeFiles/ASImage.dir/src/TASPluginGS.cxx.o
[ 97%] Building CXX object graf2d/asimage/CMakeFiles/ASImage.dir/src/TASImage.cxx.o
[ 97%] Building CXX object graf2d/asimage/CMakeFiles/ASImage.dir/src/TASPngWriter.cxx.o
[ 97%] Linking CXX shared library ../../lib/libASImage.so
[ 97%] Built target ASImage

@osschar osschar added the clean build Ask CI to do non-incremental build on PR label Jul 25, 2024
@osschar
Copy link
Contributor

osschar commented Jul 25, 2024

Oh, I see now ... builtin_afterimage is not used in the CI builds -- and might or might not be used in an arbitrary build. That means my changes can not go into libAfterImage after all.

@osschar
Copy link
Contributor

osschar commented Jul 25, 2024

Vincenzo told me builtin_afterimage is always on now ... and Danilo told me to add the clean build tag. Now things seem to be building as expected on linuxen.

Something still goes wrong on macs but there is not much info in the CI log ... it says there is some more in the actual log file.

@osschar
Copy link
Contributor

osschar commented Jul 25, 2024

I don't know how or why but AFTERIMAGE build did not run for windows and mac.

@osschar
Copy link
Contributor

osschar commented Jul 25, 2024

Ah, checked on Alja's mac ... me R an idiot ... forgot to include config.h :)

@linev
Copy link
Member

linev commented Jul 26, 2024

@alja @osschar

Grate! Now it compiles!

Last question. Why you include TGLSdfFontMaker into libGL? It creates dependency of libGL from libAfterImage and indirectly from many libs like libpng, libjpeg, libungif. This can be dangerous - if somebody already uses ROOT and link against libGL.

Maybe one just can create special small library within REve folder? You anyway using ProcessLine and therefore independent from where class comes from.

@osschar
Copy link
Contributor

osschar commented Jul 26, 2024

Yay, thanks for your help Sergey!

I put TGLSdfFontMaker in RGL as it requires off-screen GL context to render the SDF font texture. RGL already depends on libASImage ... TGLViewer::SaveAs() calls it directly to store screenshots into png/jpg files.

Alja and I have checked it on mac and it works ok. There was an interesting side-story where libRGL got linked against /usr/X11R6/lib/libGL.so (presumably from an old install of XQuartz and libX11 stuff from homebrew -- it worked after we wiped both of those) instead of Apple's OpenGL framework.

I still want to test this on Windows, then I'll rebase and force-push so there's a single commit.

@linev
Copy link
Member

linev commented Jul 26, 2024

Ok, if libGL already depends from libAfterImage - then it is fine.

You may add simple gtest-based code directly for libGL - then it will be automatically tested on all platforms.

@osschar
Copy link
Contributor

osschar commented Jul 26, 2024

I want to see it :) Especially after the tricky issue on mac. It seemed to run fine but the font texture was just noise.

@osschar
Copy link
Contributor

osschar commented Jul 28, 2024

Yay, now it works on windows, too :)
Off to rebase ...

@osschar osschar force-pushed the waad-revetext-overlay-master-6.33-rb1 branch from e3c27fb to 938467c Compare July 28, 2024 23:54
Contains contributions from Waad, Alja and Matevz. Squashed to avoid adding of
extra files in the repo (some committed by mistake) and to avoid numerous commits.

* Waad: initial implementation & tuning.
* Alja: integration with REve.
* Matevz: final cleanup, loading of fonts & font metrics. Mac and Windows fixes.

Full set of initial development commits is available from:
https://github.com/alja/root/tree/waa-master-2024

Commits to get things working on Mac/Windows and libAfterImage/libpng:
https://github.com/alja/root/tree/waad-revetext-overlay-master-6.33-rb1-final

Related PR in RenderCore: UL-FRI-LGM/RenderCore#20

Demo: tutorials/eve7/texts.C

Update RenderCore version to 1.5.

Location of SDF font and metrics files:

When REveText::AssertSdfFont() is called for the first time it chooses a writable
location in eiter $ROOTSYS/ui5/eve7/sdf-fonts or a ./sdf-fonts in the current
directory.
Alternatively REveText::SetSdfFontDir() can be called beforehand to manually
specify the desired location.
@osschar osschar force-pushed the waad-revetext-overlay-master-6.33-rb1 branch from 938467c to 6dcb57a Compare July 28, 2024 23:58
@osschar
Copy link
Contributor

osschar commented Jul 28, 2024

OK, all done ... if everything passes, this ready to go. What a ride :)

Copy link
Member

@linev linev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After many iterations code is working now as intended.

Grate job!

@linev linev merged commit 3b9ed05 into root-project:master Jul 29, 2024
17 of 18 checks passed
@osschar
Copy link
Contributor

osschar commented Jul 29, 2024

Thank you for your guidance Sergey! I'm glad this got done right :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects:master clean build Ask CI to do non-incremental build on PR in:WebGui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants