AcademySoftwareFoundation / openexr

The OpenEXR project provides the specification and reference implementation of the EXR file format, the professional-grade image storage format of the motion picture industry.
http://www.openexr.com/
BSD 3-Clause "New" or "Revised" License
1.6k stars 606 forks source link

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

Open cary-ilm opened 10 months ago

cary-ilm commented 10 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

assigning myself this ticket.

annguyen-ilm commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

Created pull request https://github.com/AcademySoftwareFoundation/openexr/pull/1586 After signing the commits and not syncing.