add Write LineOrder #45
Conversation
OIIO/WriteOIIO.cpp
Outdated
| #define kParamOutputLineOrder "lineOrder" | ||
| #define kParamOutputLineOrderLabel "Line Order" | ||
| #define kParamOutputLineOrderHint \ | ||
| "Specifies in what order the scan lines in the file are stored in the file [EXR]\n" |
There was a problem hiding this comment.
Fixed typo in what order the scan lines @devernay
|
have a good Add support LineOrder OpenEXR Writing the image. @acolwell |
There was a problem hiding this comment.
Can you help me understand why you need/want this feature? What is your use case that is motivating this change?
I have some concerns about the level of testing you are doing. This code doesn't compile which leads me to believe it isn't being tested. Selecting decreasingY also appears to cause a segmentation fault when I try to render a file. The fault appears to be in the OpenEXR library so I'm not sure what is going on. Do you experience this crash as well? How are you testing your changes?
|
lineOrder |
You have not answered my questions or addressed all of my comments. I'm also not a fan of checking in commented out code, so if the randomY functionality is not going to be active in this PR, then please remove all references to it. Until these things are addressed, I'm unlikely to support merging this change. |
approval Thanks! @acolwell |
|
lineOrder default increasingY. |
|
This does not address my previous comment. The RandomY option should not be visible in the UI if "Tile Size" is set to "Scan-Line based". That option should only be selectable when we are actually generating tiled files. Otherwise the state of the UI would be inconsistent with what actually gets generated. |
|
After thinking about this a little more, I don't think we should have the "random Y" option visible in the Natron UI. Given how Natron currently uses OpenImageIO to write out images, we would only ever write out tiles in order. The feature is intended for apps that may generate tiles in a random order. Natron does not do this so it doesn't really make sense to expose this to users right now. Even exposing the difference between "increasing Y" and "decreasing Y" seems a little questionable to me. It seems like it would only be useful for pipelines where the order of the lines mattered to some downstream component that consumes the EXR after Natron has generated it. Do you have a specific use case that requires this? I'll ask again. What is your motivation for creating this PR and exposing this feature? Also, I just wanted to let you know that I've discovered some cases where using the "decreasing Y" mode causes Natron to crash. Until I'm able to figure out why the crashes are happening and fix them, I don't believe it would be wise to merge this change. |
OpenEXR lineOrder attribute
increasingY,randomYordecreasingY. here.