-
Notifications
You must be signed in to change notification settings - Fork 452
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
Update usage_examples.rst #2062
Update usage_examples.rst #2062
Conversation
Hi @shaneantrim, thanks for this! In general it looks good, but we required signed commits (git commit -s). To fix this since you've already committed it:
|
Changed Displaying an image, using the GPU (Full Viewport Pipeline) to CPU, added Python section and added Displaying an image, using the GPU with C++ example. Signed-off-by: shaneantrim <shaneantrim@gmail.com>
bc34005
to
c9c6f07
Compare
My commit has been updated and signed off on. |
// Step 4: Add custom transforms for a 'canonical' Display Pipeline | ||
|
||
// Add an fstop exposure control (in SCENE_LINEAR) | ||
float gain = powf(2.0f, exposure_in_stops); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
The processor is then queried from the LegacyViewingPipeline. | ||
#. **Convert your image, using the processor.** | ||
See :ref:`usage_applybasic` for details for using the CPU. | ||
.. code-block:: cpp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This C++ section looks like it's from the old documentation and using the old API, please remove this part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@doug-walker can you be specific about what to remove? Sorry, I'm trying to wrap my brain around this. Do you mean everything in red starting from Get the Config.?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything in red is already removed. I was asking to remove lines 280-328.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed lines 280-328 containing Legacy C++
|
||
.. code-block:: python | ||
|
||
def main(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this line and the call to main() below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @doug-walker, I removed def main(): and the main() call at the bottom but left ..code-block:: python so that the code is formatted as python code.
@doug-walker : I was wondering, do we want the C++ and Python match a bit more in what they actually implement so that it is possible to compare the difference between languages? I think that would be useful for users and maybe show that C++ is not "scary". |
Put another way, do we want the C++ and Python examples to match almost exactly? |
@KelSolaar , yes, ideally the C++ and Python should match, and that is the case for the other examples on this page where both languages are present. Shane is adding a new Python example (based on the old documentation but with the new API), since he is a Python dev. Ideally at some point a C++ dev could add the corresponding code. |
Signed-off-by: shaneantrim <shaneantrim@gmail.com>
Signed-off-by: shaneantrim <shaneantrim@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks, Shane!
Changed Displaying an image, using the GPU (Full Viewport Pipeline) to Displaying an image, using the CPU (Full Viewport Pipeline)
Added Python example under Displaying an image, using the CPU (Full Viewport Pipeline)
Added Displaying an image, using the GPU comment section.