Viewer py add magnum text #1862

jrreyna · 2022-09-13T00:47:53Z

Motivation and Context

Using the magnum text code merged in PR #1853 to add text to the viewer.py app. I had to update the settings.py code. For some reason, it was drastically different from what I originally used for my the code in my #1838 PR so it wasn't working. It couldn't find "habitat_sim.utils.settings" so I had to change it to "examples.settings." I think the former refers to the settings file used for viewer.cpp

How Has This Been Tested

Locally, and with the CI tests. Here are two screenshots that show the mouse mode changing. We don't have python bindings yet to expose frame profiler functionality

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have completed my CLA (see CONTRIBUTING)
I have added tests to cover my changes.
All new and existing tests passed.

…_sim.utils.settings as the latter module could not be found

Skylion007 · 2022-09-13T17:56:11Z

Seems like you accidentally reverting some submodules? Like pybind11?

jrreyna · 2022-09-13T18:52:47Z

Seems like you accidentally reverting some submodules? Like pybind11?

Yes I have no idea what is happening. I made a branch from origin/main, and it says I am up to date, but the magnum-bindings submodule is also not correct. I am missing several files and changes that I see were pushed in the #1853 PR. I also tried the command git submodule update src/deps/magnum-bindings but it is not doing anything

Skylion007 · 2022-09-13T22:46:26Z

Seems like you accidentally reverting some submodules? Like pybind11?

Yes I have no idea what is happening. I made a branch from origin/main, and it says I am up to date, but the magnum-bindings submodule is also not correct. I am missing several files and changes that I see were pushed in the #1853 PR. I also tried the command git submodule update src/deps/magnum-bindings but it is not doing anything

You need to cd into those folders, git checkout the proper commits (and then git add the submodule folder) (or git checkout the .submodule file directly and then do the git submodule update).

…mples/viewer.py'

examples/viewer.py

examples/settings.py

… viewer.cpp, just using a placeholder string instead of the FPS, triangles culled, frame duration, etc

mosra · 2022-09-14T09:53:51Z

examples/viewer.py

+Last {self.num_frames_to_track} frames:
+    Frame time: {placeholder} ms
+    CPU duration: {placeholder} ms
+    GPU duration: {placeholder} µs
+    Vertex fetch ratio: {placeholder}
+    Primitives clipped: {placeholder} %


Ah yes, I should expose the FrameProfiler to Python as well. Adding a TODO for myself.

Ah thank you, I was looking into the gl module in Python to see if there were any profiling options, but just decided to bring it up later with the team

Skylion007 · 2022-09-14T16:27:44Z

examples/viewer.py

+        self._shader.draw(self._window_text.mesh)
+
+    def get_sensor_subtype_string(self, sensor_subtype):
+        if sensor_subtype == SensorSubType.PINHOLE:


It's not quite as pretty, but can't we just figure out a way to stringify the name of the enum? Makes it easier to extend. Something like

return str(sensor_subtype.value)

If that is possible in Python, that would be sweet. I'll look into it. I just remember trying to do that in C++ one time and it was tough

examples/viewer.py

… have more information on what we want to display

johnrreyna · 2022-09-15T21:23:01Z

Where are these extra fonts coming from? Are they included on purpose?

Yeah, I just added them to this directory in case the font directory changes and the default font is no longer in the right place. I want a default font loaded so that the user doesn't always have to provide one in the command line, and right now it just loads it from the current directory. I can remove them and just provide the relative path of the font in a fonts directory to the function that loads it in _init_. The viewer.cpp app has a font file in its directory as well, so I did the same. But I amenable to whatever. In general, it's probably a bad idea to duplicate files

…t is in viewer.cpp, also removed some unnecessary fonts from the examples directory

change hardcoded glyphs to ascii imputs Co-authored-by: Aaron Gokaslan <aaronGokaslan@gmail.com>

src_python/habitat_sim/utils/settings.py

Skylion007 · 2022-09-20T16:58:58Z

Why can't we use the same proggytff file that C++ user is using btw?

johnrreyna · 2022-09-20T19:24:22Z

Why can't we use the same proggytff file that C++ user is using btw?

That proggy ttf file was copied directly into the same directory as viewer.cpp, so I did something similar and copied the file into the same directory as viewer.py. Though I think the best solution is to have a fonts directory and specify the relative path to it. I just wanted to be safe and put the ttf file in the same directory for now in case the font directory moves

…command line

…p. I made a new fonts directory in the data directory, moved all the ttf fonts there, and updated viewer.py and viewer.cpp to use relative paths to the new fonts folder

…to the new fonts directory

mosra · 2022-09-26T09:49:44Z

src/utils/viewer/resources.conf

@@ -1,4 +1,4 @@
 group=fonts

 [file]
-filename=ProggyClean.ttf
+filename=../../../data/fonts/ProggyClean.ttf


To avoid having to hardcode the ugly relative path in viewer.cpp, add an alias here (docs):

Suggested change

filename=../../../data/fonts/ProggyClean.ttf

filename=../../../data/fonts/ProggyClean.ttf

alias=ProggyClean.ttf

oo that's slick. Thanks, I wasn't exactly sure how the resources.conf file worked

mosra · 2022-09-26T09:50:09Z

src/utils/viewer/viewer.cpp

@@ -899,7 +899,8 @@ Viewer::Viewer(const Arguments& arguments)

    Cr::Utility::Resource rs{"fonts"};
    font_ = fontManager_.loadAndInstantiate("TrueTypeFont");
-    if (!font_ || !font_->openData(rs.getRaw("ProggyClean.ttf"), fontSize))
+    std::string font_relative_path = "../../../data/fonts/ProggyClean.ttf";
+    if (!font_ || !font_->openData(rs.getRaw(font_relative_path), fontSize))


... and this change then isn't needed.

Do you know if I can do something similar in viewer.py to avoid hardcoding the ugly relative path?

Is the viewer.py meant to be packaged / installed in some way (setup.py, pip etc.)? I know very little about Python packaging, but if it is, I'd put the font file into some "well-known package data location" (if there's such a thing) and reference it from there. If not, then I'm afraid the path has to be hardcoded that way. But I'd make it relative to __file__, at least, so it doesn't depend on CWD.

Another idea I had was that we embed the font directly inside the Habitat libraries instead of the C++ viewer, and expose that array to Python. Such as

Containers::ArrayView<const char> fontData() { Cr::Utility::Resource rs{"fonts"}; return rs.getRaw("ProggyClean.ttf"); }

and then, if the above would be exposed as habitat.font_data() to Python, you'd do

self.display_font.open_data(habitat.font_data(), size)

instead of self.display_font.open_file("that/ugly/path.ttf", size). A downside is that the binary now embeds a font file that's only needed if using a GUI viewer, but as long as we stay with that tiny 40 kB font file, I think the convenience wins over memory use :)

…ly relative path in viewer.cpp to refer to a font

ykarmesh

@aclegg3 @johnrreyna I am approving this PR.

mosra · 2022-09-27T08:36:36Z

examples/viewer.py

+        relative_path_to_font = "../data/fonts/ProggyClean.ttf"
+        self.display_font.open_file(
+            os.path.join(os.path.dirname(__file__), relative_path_to_font),
+            180,


Suggested change

180,

13,

mosra · 2022-09-27T08:38:01Z

examples/viewer.py

+        )
+
+        # Glyphs we need to render everything
+        self.glyph_cache = text.GlyphCache(mn.Vector2i(2048))


Suggested change

self.glyph_cache = text.GlyphCache(mn.Vector2i(2048))

self.glyph_cache = text.GlyphCache(mn.Vector2i(256))

... we discussed this on Slack some weeks ago, so just a reminder :) It'll also avoid unnecessary GPU memory use, and since the font is a bitmap one, rendering it at 10x the size will not make it any prettier.

mosra · 2022-09-27T08:39:54Z

examples/viewer.py

+                * HabitatSimInteractiveViewer.TEXT_DELTA_FROM_CENTER,
+            )
+        )
+        self.shader = shaders.DistanceFieldVectorGL2D()


Suggested change

self.shader = shaders.DistanceFieldVectorGL2D()

self.shader = shaders.VectorGL2D()

Since you aren't really using any outline anyway, let's use a simpler shader. (The same is used in the viewer.cpp.)

mosra · 2022-09-27T08:40:28Z

examples/viewer.py

+        self.shader.outline_color = [1.0, 1.0, 1.0]
+        self.shader.outline_range = (0.45, 0.40)


Suggested change

self.shader.outline_color = [1.0, 1.0, 1.0]

self.shader.outline_range = (0.45, 0.40)

Those aren't defined in the simpler non-distance-field shader.

mosra · 2022-09-27T08:44:27Z

data/fonts/README.txt

@@ -0,0 +1,364 @@
+----------------------------------------------------------------------


I'd remove everything from this directory except for the ProggyClean.ttf. Unnecessary and irrelevant now that the font file is hardcoded anyway. Tho it might be good to credit

ProggyClean.ttf by Tristan Grimmer

somewhere, although I don't know if there's even such a place in the docs or README.

…into viewer-py-add-magnum-text updating submodules

Skylion007 · 2022-09-27T18:39:53Z

Do we need the other font files besides proggy?

jrreyna · 2022-09-27T19:02:28Z

Do we need the other font files besides proggy?

I'm not sure, I just used the same file that was in the viewer.cpp file. Those fonts were in a directory in the imgui submodule, so I don't even think those font files are on main, right?

johnrreyna · 2022-09-27T19:25:37Z

Do we need the other font files besides proggy?

Just talked to mosra, probably not. Will delete them for now

…xample of how to use the font in python with magnum. Also changed the text shader from DistanceFieldVectorGL2D() to VectorGL2D() to make things simpler

…into viewer-py-add-magnum-text pull and merge main into this branch

Skylion007 · 2022-09-27T23:07:52Z

@johnrreyna Please squash your commits next time when merging.

jrreyna added 2 commits September 12, 2022 17:34

commiting changes made during magnum text PR merge

f2a2c78

updated settings.py and imported examples.settings instead of habitat…

825a08e

…_sim.utils.settings as the latter module could not be found

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 13, 2022

jrreyna self-assigned this Sep 13, 2022

jrreyna requested review from Skylion007, mosra, aclegg3 and ykarmesh September 13, 2022 00:48

jrreyna added 3 commits September 13, 2022 16:51

updating submodules

01bc510

importing text from magnum now works

1f46fd1

default to ycb dataset and simple_room scene when you run 'python exa…

947475b

…mples/viewer.py'