Skip to content

Add ezgl Doxygen api documentation to to the VTR docs #3045

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

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

SamuelHo10
Copy link

@SamuelHo10 SamuelHo10 commented May 15, 2025

Integrated EZGL’s Doxygen documentation into the VTR ReadTheDocs site under a new "EZGL" tab in the vtrutil API section.

The header files containing only functions and no classes did not generate documentation correctly when I used
.. doxygenfile:: control.hpp
:project: ezgl

I am not sure why, but adding the functions manually in worked:
.. doxygenfunction:: ezgl::zoom_in(canvas*, double)
:project: ezgl

@SamuelHo10 SamuelHo10 linked an issue May 15, 2025 that may be closed by this pull request
@github-actions github-actions bot added the docs Documentation label May 15, 2025
@github-actions github-actions bot added the lang-python Python code label May 15, 2025
@SamuelHo10 SamuelHo10 removed the lang-python Python code label May 15, 2025
@github-actions github-actions bot added the lang-python Python code label May 15, 2025
@SamuelHo10 SamuelHo10 changed the title [WIP] Add ezgl Doxygen api documentation to to the VTR docs Add ezgl Doxygen api documentation to to the VTR docs May 15, 2025
@SamuelHo10 SamuelHo10 requested review from amin1377 and vaughnbetz May 15, 2025 21:01
Copy link
Contributor

@vaughnbetz vaughnbetz left a comment

Choose a reason for hiding this comment

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

Thanks @SamuelHo10 !

@vaughnbetz
Copy link
Contributor

This looks good and I'm merging it, but I have one suggestion:

  • Either in Doxygen comments in the code that will show up at the top of ezgl.rst, or in the ezgl.rst file itself, I think you should add a short overview of ezgl. Here's some potential text:
    EZGL is a graphics layer on top of version 3.x of the GTK graphics library. It allows drawing in an arbitrary 2D world coordinate space (instead of in pixel coordinates), handles panning and zooming automatically, and provides easy-to-use functions for common tasks like setting up a window, setting graphics attributes (like colour and line style) and drawing primitives (like lines and polygons). Most of VPR's drawing is performed in ezgl, and GTK functionality not exposed by ezgl can still be accessed by directly calling the relevant gtk functions.

@soheilshahrouz
Copy link
Contributor

Thanks @SamuelHo10

A couple of suggestions:

  1. The new page is added under the vtrutil library, but ezgl isn't part of vtrutil, which might be confusing.
  2. Instead of having several .rst files, it might be clearer to combine them into one. This would make the file structure easier to follow.

@vaughnbetz
Copy link
Contributor

@soheilshahrouz : good point on libvtrutil. I suggested putting ezgl under vtrutil (thinking of it as a utility) but you are right it is not part of the library we build for utilities so probably it would be better to make it another top level category; we could move it between vtrutil API and vtr internals

@AmirhosseinPoolad
Copy link
Contributor

Another suggestion @SamuelHo10, I think you should squash the commits for this PR into a single commit with a descriptive commit message. While it doesn't really matter that much for a documentation change, it's good practice to have atomic commits i.e. commits that take the code from one working state to another and do/add one thing to the code nonetheless.

You can look here to see how squashing and some other useful git tools work:
https://thoughtbot.com/blog/git-interactive-rebase-squash-amend-rewriting-history

Also if you use the GitLens extension for VSCode it gives you a nice UI which might be easier to work with. Just make that when you run git rebase -i HEAD~n, n is the number of commits you have in your branch and not more than that. If you do HEAD~1000 it would mess with the git history.

Sorry to give you extra work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation lang-python Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add ezgl Doxygen api documentation to to the VTR docs
4 participants