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

Test Images page for website #1372

Merged
merged 14 commits into from
Apr 7, 2023

Conversation

cary-ilm
Copy link
Member

This adds a "Test Image" section to the website with a gallery of images from the test image repo https://github.com/AcademySoftwareFoundation/openexr-images, with a page for each image that shows the output of exrheader. Each image is a link to download the .exr file.

Preview it here: https://cary-ilm-openexr.readthedocs.io/en/docs-images/index.html

It's not perfect, but it's a decent start, feel free to suggest improvements to the layout or content. A few notes:

  • In lieu of setting up a way to view .exr images directly in a browser, I converted the .exr's in openexr-images to .jpg's. They are stored in the openexr-images repo and served to the website directly from github, so the OpenEXR distribution (which holds the website source) doesn't get bloated with extra image data. The download link downloads the source .exr image, not the .jpg.

  • This draft actually serves them from a branch of my fork of the openexr-images repo, but that can get fixed later.

  • The exr2rst.py script generates the rst (and embedded html) from the .exr's (by running exrheader) and the accompanying README's, so it's (relatively) easy to add new images.

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
@Vertexwahn
Copy link
Contributor

Vertexwahn commented Mar 26, 2023

Section with orientation cube looks strange:

image

@cary-ilm
Copy link
Member Author

If by "strange" you mean a text repeated pattern with rotated cross-hairs, I'm pretty sure that's what the image actually looks like, although I'm not actually familiar with how this specific image was made. What exactly looks strange?

@Vertexwahn
Copy link
Contributor

I was thinking the table layout is destroyed here. But now I realize that is text within an image. Ignore my comments.

Copy link
Contributor

@meshula meshula left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

This LGTM as far as it goes, but I wonder if rather than have a bespoke file for every test image, perhaps in the long run these should be auto-generated from the exr files themselves? This might have several advantages:

  1. Easy to add new test images with the same style page.
  2. Probably enforces that the test images are self-documenting (because to generate the page automatically, some description probably needs to be in the metadata).
  3. Serves as additional tests -- testsuite can regenerate and compare to checked-in output that has been scrutinized for correctness.

@cary-ilm
Copy link
Member Author

I think it's halfway there already, since the current workflow is to run the exr2rst.py script on my local clone of openexr-images, which generates a bunch of .rst files, which I then commit to git, so that sphinx can process them on readthedocs. Adding new images just means re-running the script, although if you remove an image, you'd have to know to remove it from git.

You're suggesting that the cmake build process just invoke this script automatically and put the intermediate .rst in the build area, bypassing git altogether, right? I'll look into that, it would be nice to avoid the intermediate files.

The script doesn't rely on any metadata in the individual .exr's but it does parse the README files in the openexr-images repo, which are useful because the provide local context if you're just looking through the repo itself.

Incidentally, I also looked into jeri.io as a way of displaying .exr's directly rather than .jpg proxies, but it only supports a limited set of exr features, so it's not going to work without modification.

@lgritz
Copy link
Contributor

lgritz commented Mar 27, 2023

Ah, ok, I see that you have already mostly done that. The .py script got lost in the sea of .rst files I was scanning, I just didn't notice it when I write my comment. I wish GitHub had an option when submitting a PR to give a custom order for presentation of the files to diff, so that they could properly "tell the story" of the change in the logical order you would use for a live code review.

@meshula
Copy link
Contributor

meshula commented Mar 27, 2023

It should be straight forward (but it would take some time to engineer with our current set up) to cross compile to wasm and just have a real viewer inline.

Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
Signed-off-by: Cary Phillips <cary@ilm.com>
@cary-ilm
Copy link
Member Author

cary-ilm commented Apr 5, 2023

Major update to this, the .rst and the proxy .jpg’s are now generated automatically, so no need to maintain .jpg’s in the openexr-images repo.

The docs/test_images.txt file lists urls for .exr’s and README.rst files that define the contents of the “Test Images” page.

The docs/scripts/test_images.py script builds the index page and the per-image pages. It downloads the files via wget, then runs convert (ImageMagick) to generate the proxy .jpg’s, then runs exrheader to generate the tables of attribute values.

The script puts the auto-generated .rst files in the docs/_test_images subdirectory. Ideally, these would go in the build directory, but sphinx seems to want all .rst input under a single source root.

There's now a separate "Docs" CI job that builds the docs, so I removed all references to docs in the other jobs. It uses Ubuntu to mirror .readthedocs.yml. There's no need for the install_doxygen.sh and install_docs_env.sh scripts, the necessary installations are simple enough to do inline in the CI job.

@cary-ilm cary-ilm merged commit 71a7141 into AcademySoftwareFoundation:main Apr 7, 2023
@cary-ilm cary-ilm added the v3.2.0 label Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants