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

Fix formatting of sample exr file in OpenEXRFileLayout.rst #1545

Open
cary-ilm opened this issue Sep 11, 2023 · 12 comments
Open

Fix formatting of sample exr file in OpenEXRFileLayout.rst #1545

cary-ilm opened this issue Sep 11, 2023 · 12 comments
Labels
Documentation Developer guide, web site, project policies, etc. good first issue Possible one-day project for somebody new

Comments

@cary-ilm
Copy link
Member

The formatting here is confusing and hard to read: https://openexr.com/en/latest/OpenEXRFileLayout.html#sample-file

The table show simply annotate each byte of the file, alongside the hex value of the byte. The formatting would probably work better as a single long column.

This task requires some basic familiarity with reStructuredText and sphinx but the concepts are easy to pick up from other examples. See Building the Website for how to test the formatting.

@cary-ilm cary-ilm added good first issue Possible one-day project for somebody new Documentation Developer guide, web site, project policies, etc. labels Sep 11, 2023
@cary-ilm
Copy link
Member Author

A helpful addition to this would be to create an image file that corresponds to the example and link it in the documentation for download.

@cary-ilm
Copy link
Member Author

Also, it would be helpful to change the floating point numbers in the example to values that have an exact correspondence to the bit patterns shown, which would avoid confusion like that described in #1433, where computing the bit pattern of the numbers shown yield slightly different values due to rounding.

@annguyen-ilm
Copy link
Contributor

assigning myself this ticket.

@annguyen-ilm
Copy link
Contributor

I've updated format in the branch of the fork.
https://github.com/annguyen-ilm/openexr/blob/1545_fix_formatting_sample_exr/website/OpenEXRFileLayout.rst
I create the actual sample file tomorrow.

@cary-ilm
Copy link
Member Author

This is looking good, thanks! An improvement over the previous formatting.

For the channel names, I wonder if it would be even more instructive to put each character of the channel name on a separate line, so the ascii character appears next to the hex byte:

63 c
68 h
61 a
6e n
6e n
65 e
6c l
73 s
00 \0

As is, it's not entirely clear what the bytes correspond do until you realize it's the characters in the name.

If you're so inclined, you can set up an account on readthedocs and create project corresponding to your fork of OpenEXR, then build the documentation there for preview. Not necessary, but that will illuminate a few more of the steps in the process.

@annguyen-ilm
Copy link
Contributor

Updated the value column to put the ascii values adjacent to the bytes rows.
https://openexr-annguyen.readthedocs.io/en/latest/OpenEXRFileLayout.html has been built using my branch.

@annguyen-ilm
Copy link
Contributor

I created the sample.exr file with some python code.


hex = "762f3101020000006368616e6e656c730063686c69737400250000004700010000000000000001000000010000005a000200000000000000010000000100000000636f6d7072657373696f6e00636f6d7072657373696f6e0001000000006461746157696e646f7700626f783269001000000000000000000000000300000002000000646973706c617957696e646f7700626f7832690010000000000000000000000003000000020000006c696e654f72646572006c696e654f72646572000100000000706978656c417370656374526174696f00666c6f617400040000000000803f73637265656e57696e646f7743656e746572007632660008000000000000000000000073637265656e57696e646f77576964746800666c6f617400040000000000803f003f010000000000005f010000000000007f01000000000000000000001800000000005429d535e82d5c28813acfe1343e8b0bbb3d8974f93e000000001800000037387633743b73387fabe83e8acf543f5b6c113f2035503d0200000018000000233a0a34023b5d3b38f39a3c4dad983e1c14083f4cf3033f"

try:
    byte_data = bytes.fromhex(hex)
except ValueError:
    print("Invalid hex string")

output_path = "sample.exr"

with open(output_path, "wb") as output_file:
    output_file.write(byte_data)

print(f"Bytes written to {output_file}")

Where should I put the sample.exr file so I can update the rst file to link to it?

@annguyen-ilm
Copy link
Contributor

I think I need to verify this sample.exr file and see if float values are readable in a DCC or the API. As well verify the float values in the table. If there is invalid float value values in the documentation.

@cary-ilm
Copy link
Member Author

Looking at the output after suggesting one ascii character per line, I now wonder if it would be further helpful to put the attribute name itself in the third column, something like:

attribute name
"displayWindow"

It's nice to see the annotation of each byte, but not immediately obvious that the column of characters spells a word.

Also, I think the start of header deserves a separate line, without an associated byte.

Also, the value of the "channels" attribute is a little confusing. It's a string list, but it isn't obvious how the bytes correspond to the value. Could that be further annotated?

The value column in for the displayWindow values for the box2i type aren't lined up properly. It would be helpful to annotate those as box.min.x, rather than a single value.

And as for the example file itself, I'd suggest putting it in the website/images subdirectory, and add a sentence to the section mentioned that the file can be downloaded, with a link to download the file.

You can validate the file by running exrcheck, one of the OpenEXR programs, on it. It checks that the input is a valid file.

@annguyen-ilm
Copy link
Contributor

annguyen-ilm commented Oct 16, 2023

Thanks for exrcheck suggestion.. Found out that I had one byte in the in the scan line 1 section wrong.. it was still pointing to scanline 0. So changing that single byte to point to scanline 1 helped it. Since scanline 1 was undefined at the time.

hex = "762f3101020000006368616e6e656c730063686c69737400250000004700010000000000000001000000010000005a000200000000000000010000000100000000636f6d7072657373696f6e00636f6d7072657373696f6e0001000000006461746157696e646f7700626f783269001000000000000000000000000300000002000000646973706c617957696e646f7700626f7832690010000000000000000000000003000000020000006c696e654f72646572006c696e654f72646572000100000000706978656c417370656374526174696f00666c6f617400040000000000803f73637265656e57696e646f7743656e746572007632660008000000000000000000000073637265656e57696e646f77576964746800666c6f617400040000000000803f003f010000000000005f010000000000007f01000000000000000000001800000000005429d535e82d5c28813acfe1343e8b0bbb3d8974f93e010000001800000037387633743b73387fabe83e8acf543f5b6c113f2035503d0200000018000000233a0a34023b5d3b38f39a3c4dad983e1c14083f4cf3033f"


try:
    byte_data = bytes.fromhex(hex)
except ValueError:
    print("Invalid hex string")

output_path = "sample.exr"

with open(output_path, "wb") as output_file:
    output_file.write(byte_data)

print(f"Bytes written to {output_file}")

should be the correct python variable to generate the correct sample.exr.
I opened the sample.exr file in Nuke and I inspected each Green value and zdepth value and it does seem to correlate correctly to the documentation float values. So there's no issue there at all. Since those bytes are copied into the string above as the same order from the documentation.

I put the sample.exr file in openexr/website/images
Linking the sample.exr to the page, should I do anything special?
i.e.

.. image:: images/sample.exr
  :alt: sample.exr image.
Download `here <_images/sample.exr>`_
  

Since, one thing I'm not sure is how sphinx is copying the files from website/images to it's website/sphinx/_images directory.
Unless I specify the sample.exr with the .. image directive for sphinx.

@annguyen-ilm
Copy link
Contributor

Okay. I somehow got the download link working with the :download: directive. so ignore me trying to understand sphinx cmake process. The latest readthedocs has the update.

I think it's ready to for a pull request.

@annguyen-ilm
Copy link
Contributor

Created pull request #1586
After signing the commits and not syncing.

cary-ilm pushed a commit that referenced this issue Dec 18, 2023
…d attempt (#1587)

* Replaced the sample exr file code blocks with a grid table formatted with bytes of the sample file in one column. The value in the second column and the third column being the description of the bytes and value.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* * Updated so the ascii values for the bytes are aligned to each byte.
* Gave the table a title and reduce the width to 50%

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added missing 4 bytes missing from the dataWindow value and displayWindow value

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added the sample.exr file

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Annotated the attribute values more for the header. Should be a bit clearer.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Adding the sample.exr file was documented.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added download link to the sample.exr file.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Move the download link above the table. Updated the paragraph before the table to describe the table

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Move the sample.exr image into the downloads directory to match the target destination naming scheme to avoid confusion where the files go when built

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Updated the table to annotate the type and literal as type.

Signed-off-by: An Nguyen <annguyen@ilm.com>

---------

Signed-off-by: An Nguyen <annguyen@ilm.com>
cary-ilm pushed a commit to cary-ilm/openexr that referenced this issue Feb 13, 2024
…penEXRFileLayout.rst - 3rd attempt (AcademySoftwareFoundation#1587)

* Replaced the sample exr file code blocks with a grid table formatted with bytes of the sample file in one column. The value in the second column and the third column being the description of the bytes and value.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* * Updated so the ascii values for the bytes are aligned to each byte.
* Gave the table a title and reduce the width to 50%

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added missing 4 bytes missing from the dataWindow value and displayWindow value

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added the sample.exr file

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Annotated the attribute values more for the header. Should be a bit clearer.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Adding the sample.exr file was documented.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added download link to the sample.exr file.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Move the download link above the table. Updated the paragraph before the table to describe the table

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Move the sample.exr image into the downloads directory to match the target destination naming scheme to avoid confusion where the files go when built

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Updated the table to annotate the type and literal as type.

Signed-off-by: An Nguyen <annguyen@ilm.com>

---------

Signed-off-by: An Nguyen <annguyen@ilm.com>
cary-ilm pushed a commit that referenced this issue Feb 16, 2024
…d attempt (#1587)

* Replaced the sample exr file code blocks with a grid table formatted with bytes of the sample file in one column. The value in the second column and the third column being the description of the bytes and value.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* * Updated so the ascii values for the bytes are aligned to each byte.
* Gave the table a title and reduce the width to 50%

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added missing 4 bytes missing from the dataWindow value and displayWindow value

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added the sample.exr file

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Annotated the attribute values more for the header. Should be a bit clearer.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Adding the sample.exr file was documented.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Added download link to the sample.exr file.

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Move the download link above the table. Updated the paragraph before the table to describe the table

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Move the sample.exr image into the downloads directory to match the target destination naming scheme to avoid confusion where the files go when built

Signed-off-by: An Nguyen <annguyen@ilm.com>

* Updated the table to annotate the type and literal as type.

Signed-off-by: An Nguyen <annguyen@ilm.com>

---------

Signed-off-by: An Nguyen <annguyen@ilm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Developer guide, web site, project policies, etc. good first issue Possible one-day project for somebody new
Projects
None yet
Development

No branches or pull requests

2 participants