Bring the three node pages up to what the nodes now do - #16
Merged
Conversation
Checked all four documentation files against today's changes rather than assuming they were current, and three had gaps. docs/NODES_VAE.md still said "`tiled` off is the default so no existing graph changes behaviour" - a plain untruth since the default flipped to ON. Corrected, and the old reasoning is kept rather than deleted: off was chosen so adding the node could not change a graph's output, which is a sound instinct that lost to a harder fact, that whole-frame is the setting which runs out of memory on a real clip and is slower even when it fits. docs/NODES_IO.md described no on-node preview at all, so the new behaviour had nowhere to be. It now has a section: a video container shows an H.264 proxy with audio, a sequence of more than one frame flips through its own written files through /ocio/thumb, a single still shows itself naively so a wrong colorspace looks visibly wrong, and the sequence path carries no audio because a frame sequence has none. docs/NODES.md gave one recipe for a review movie, through a separate OCIO Display node. It now gives both, plus the thing neither recipe said: doing both at once applies the output transform twice, and doing neither writes values above 1.0 into a container that clips them flat. docs/NODES_COLOR.md needed nothing - the six colour operators did not change today. Gate: 36 passed, 0 failed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You asked whether the four documentation pages were current. Three were not.
docs/NODES_VAE.mdstill said "tiledoff is the default so no existing graph changes behaviour" - untrue since the default flipped to ON earlier today. Corrected, keeping the old reasoning rather than deleting it: off was chosen so that adding the node could not change a graph's output, which is a sound instinct that lost to a harder fact.docs/NODES_IO.mddescribed no on-node preview at all, so today's change had nowhere to live. It now has a section covering all three cases: the H.264 proxy with audio for a video container, the frame flipbook for a sequence, and the naive still.docs/NODES.mdgave one recipe for a review movie, through a separateOCIO Display. It now gives both routes, plus what neither said: doing both at once applies the output transform twice, and doing neither writes values above 1.0 into a container that clips them flat.docs/NODES_COLOR.mdneeded nothing. The six colour operators did not change today, and I am saying so explicitly rather than leaving it unmentioned.Documentation only, no code. Gate: 36 passed, 0 failed.