search

AcademySoftwareFoundation / OpenPBR

Specification and reference implementation for the OpenPBR Surface shading model
Apache License 2.0
401 stars 17 forks source link

Initial cleanup for 1.0. #191

Closed portsmouth closed 1 month ago

portsmouth commented 1 month ago

Add direct link to parameter reference from README.

portsmouth commented 1 month ago

I strongly suggest we make these links not visible from the GitHub front page, as we don't want people navigating to those files directly, as then the markdown does not render. (Some people already mistook the un-rendered markdown source as the intended form of the spec).

That can be done by putting them in a sub-folder, named e.g. internal.

Someone with admin access is probably needed to do that, as it may require pointing the GitHub Pages link to the internal/index.html etc. See the dialog below for that.

(A better solution is to make a nicer landing page for OpenPBR, that is not the GitHub repo itself. But that would take some time to create).

image

image

portsmouth commented 1 month ago

Clarified front page:

image

portsmouth commented 1 month ago

The spec takes about 10 seconds to properly render (due to all the MathJax I assume), maybe we should mention that next to the white paper link?

portsmouth commented 1 month ago

Added a copy-pastable BibTeX citation reference, which is needed if we want OpenPBR to be referenced in papers or documentation. (The form of this can be tweaked as needed, we can check how it renders in a paper for example).

@techreport{OpenPBR,
  author = {Zap Andersson and Paul Edmondson and Julien Guertault and Adrien Herubel and Alan King and Peter Kutz and Andréa Machizaud and Jamie Portsmouth and Frédéric Servant},
  title = {OpenPBR Surface},
  institution = {Academy Software Foundation (ASWF)},
  year = {2024},
  url = {https://academysoftwarefoundation.github.io/OpenPBR/}
}
portsmouth commented 1 month ago

Added a clickable GitHub icon to the spec (just after the version info), which links back to the GitHub page.

Also removed "specification" from the title. I think that is reasonably implied (and we say it in the first sentence of the abstract), and makes a more bold and readable title.

image

portsmouth commented 1 month ago

The reference to the Gulbrandsen paper is missing, even though we cite it.

Added in 0c69b27288428c1e066161728173c9d68d002539.

portsmouth commented 1 month ago

Rewording the text on the front page in ac84bfe543effe2de257ddd8d738cc393076b940, as the sentence was long and hard to parse.

OpenPBR Surface is a specification of a surface shading model intended as a standard for computer graphics. It aims to provide a material representation capable of accurately modeling the vast majority of CG materials used in practical visual effects and feature animation productions.

OpenPBR Surface is an open standard hosted by the Academy Software Foundation (ASWF), and is organized as a subproject of MaterialX.

virtualzavie commented 1 month ago

Someone with admin access is probably needed to do that, as it may require pointing the GitHub Pages link to the internal/index.html etc. See the dialog below for that.

Internal doesn't seem like a faithful description of the nature of those files. Accessing them directly is only problematic on GitHub, but works fine for someone who clones the repository. What about specification/index.html or spec/index.html? This way they are moved away from top-level, but are not "hidden".

virtualzavie commented 1 month ago

The spec takes about 10 seconds to properly render (due to all the MathJax I assume), maybe we should mention that next to the white paper link?

On my side, it loads nearly instantaneously, both on Firefox/macOS, Firefox/Windows and Edge/Windows.

portsmouth commented 1 month ago

The spec takes about 10 seconds to properly render (due to all the MathJax I assume), maybe we should mention that next to the white paper link?

On my side, it loads nearly instantaneously, both on Firefox/macOS, Firefox/Windows and Edge/Windows.

Do the equations render correctly nearly instantaneously? On my system, they show up immediately, but change format as they are correctly rendered.

virtualzavie commented 1 month ago

[page loading time]

Do the equations render correctly nearly instantaneously? On my system, they show up immediately, but change format as they are correctly rendered.

After maybe a second or so, yes. The loading of images is what takes the most time on my side, with the scrolling being a bit all over the place while this happens.

portsmouth commented 1 month ago

Someone with admin access is probably needed to do that, as it may require pointing the GitHub Pages link to the internal/index.html etc. See the dialog below for that.

Internal doesn't seem like a faithful description of the nature of those files. Accessing them directly is only problematic on GitHub, but works fine for someone who clones the repository. What about specification/index.html or spec/index.html? This way they are moved away from top-level, but are not "hidden".

Sounds fine to me. But I can't make this change myself as it will break the GitHub pages deployment. @jstone-lucasfilm would you mind to make this change?

jstone-lucasfilm commented 1 month ago

@portsmouth For moving the specification to a new location, I'm happy to assist, but I'd recommend that we leave this as an improvement for after today's release candidate.

portsmouth commented 1 month ago

@portsmouth For moving the specification to a new location, I'm happy to assist, but I'd recommend that we leave this as an improvement for after today's release candidate.

Sure that's fine. How about we merge the other improvements in this PR, and leave that for a different PR?

jstone-lucasfilm commented 1 month ago

@portsmouth Sure, I'm fine for this PR to be the first in a series of improvements over the next two weeks, but then maybe the title of the PR should be more specific? :D

portsmouth commented 1 month ago

@portsmouth Sure, I'm fine for this PR to be the first in a series of improvements over the next two weeks, but then maybe the title of the PR should be more specific? :D

OK, changed the PR title.

How is this? The ASWF logo now doubles as the link back to the OpenPBR GitHub, killing two birds with one stone. (If you prefer another form of that logo, or a different size/placement, feel free to suggest).

image

I moved the authors to the end, and took the liberty of moving @jstone-lucasfilm to that list, as that seems fairer to me considering your contribution. The BibTeX citation is updated accordingly, with the version removed.

image

@techreport{OpenPBR,
  author = {Zap Andersson and Paul Edmondson and Julien Guertault and Adrien Herubel and Alan King and Peter Kutz and Andréa Machizaud and Jamie Portsmouth and Frédéric Servant and Jonathan Stone},
  title = {OpenPBR Surface},
  institution = {Academy Software Foundation (ASWF)},
  year = {2024},
  month = {May},
  url = {https://academysoftwarefoundation.github.io/OpenPBR/}
}
portsmouth commented 1 month ago

Let me add the shader playground image as well...

virtualzavie commented 1 month ago

I think we can thank a few more people who have made significant contributions in the last months. Chris Kulla, Eugene d'Eon, Masuo Suzuki, Andrea Weidlich, André Mazzone and Stephen Hill come to mind.

portsmouth commented 1 month ago

How's this? (Nikie is attributed in the alt-text of the images).

image

portsmouth commented 1 month ago

I think we can thank a few more people who have made significant contributions in the last months. Chris Kulla, Eugene d'Eon, Masuo Suzuki, Andrea Weidlich, André Mazzone and Stephen Hill come to mind.

Good point, thanks for the list. I updated it to:

image