Skip to content

Rendering toolkit oidn code sample PR #713

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

Merged

Conversation

MichaelRoyceCarroll
Copy link
Contributor

Adding a New Sample(s)

Description

New sample PR for oneapi rendering toolkit oidn sample. (library # 4 of 4)
See isseue: #697

Checklist

Administrative

  • Review sample design with the appropriate Domain Expert:
  • If you have any new dependencies/binaries, inform the oneAPI Code Samples Project Manager: @JoeOster

Code Development

Security and Legal

  • OSPDT Approval (see @JoeOster for assistance)
  • Compile using the following compiler flags and fix any warnings, the falgs are: "/Wall -Wformat-security -Werror=format-security"
  • Bandit Scans (Python only)
  • Virus scan

Review

  • Review DPC++ code with Paul Peterseon. (GitHub User: pmpeter1)
  • Review readme with Tom Lenth(@tomlenth) and/or Joe Oster(@JoeOster)
  • Tested using Dev Cloud when applicable

JoeOster and others added 18 commits September 29, 2021 09:45
* Update Makefile

* Update Makefile

* Update Makefile

* Update DCT.hpp

* Update intrin_ftz_sample.cpp

* Update merge_sort.cpp

* Update intrin_double_sample.cpp

* Update intrin_dot_sample.cpp

* Update DCT.cpp
Moving README.md content to individual folders
Signed-off-by: Michael R Carroll <[email protected]>
…get stuck. It is not too big.

Signed-off-by: Michael R Carroll <[email protected]>
Signed-off-by: Michael R Carroll <[email protected]>
Signed-off-by: Michael R Carroll <[email protected]>
@JoeOster JoeOster linked an issue Oct 18, 2021 that may be closed by this pull request
- Open Image Denoise

Imaging Tools:
- An image **display program** for .ppm and .pfm filetypes . Ex: [ImageMagick](https://www.imagemagick.org/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should read as "A image viewer..."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JoeOster Thanks for the review. Per grammar rules I think 'an image display program' is correct... Reference: https://www.thesaurus.com/e/grammar/a-vs-an/
However, if you believe there is a readability issue I can think about a different description.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been changed to use the mandlebrot style README file table. This is now no longer applicable.

@MichaelRoyceCarroll MichaelRoyceCarroll changed the base branch from master to development October 18, 2021 23:39
@MichaelRoyceCarroll MichaelRoyceCarroll changed the title Rendering toolkit oidn Rendering toolkit oidn code sample PR Oct 19, 2021
@JoeOster
Copy link
Contributor

OSPDT Approved per OSPDT-1015 * All Apache Licensed libraries distributed as part of oneAPI have been approved
OIDN is part of the oneAPI Rendering Toolkit

Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI purposes only

Signed-off-by: Michael R Carroll <[email protected]>
Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI purposes again

Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI

Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI

Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI

@MichaelRoyceCarroll
Copy link
Contributor Author

@Yury-B Is CMake not available for MacOS? Can it be?

@Yury-B
Copy link

Yury-B commented Oct 28, 2021

@Yury-B Is CMake not available for MacOS? Can it be?

now, cmake 3.21 is available in /usr/local/bin

@MichaelRoyceCarroll
Copy link
Contributor Author

@Yury-B Is CMake not available for MacOS? Can it be?

now, cmake 3.21 is available in /usr/local/bin

Thanks I should be able to fix the latest issue with the std toggle.

-MichaelC

@praveenkk123 praveenkk123 requested a review from pmpeter1 October 28, 2021 23:12
Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI

@MichaelRoyceCarroll
Copy link
Contributor Author

This one I still need to fix... lost track of the toggle amongst other changes... WIP

Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI

@MichaelRoyceCarroll
Copy link
Contributor Author

Hello... 0b20e73 'may' be a workaround for oidn dyld not loading issue. Can we launch it? Thanks.

Copy link
Contributor

@praveenkk123 praveenkk123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve for CI

@praveenkk123 praveenkk123 merged commit 783bd6b into oneapi-src:development Nov 3, 2021
praveenkk123 pushed a commit that referenced this pull request Dec 7, 2021
* ONSAM-1414 Broken Link in Headers (#685)

* Update Makefile

* Update Makefile

* Update Makefile

* Update DCT.hpp

* Update intrin_ftz_sample.cpp

* Update merge_sort.cpp

* Update intrin_double_sample.cpp

* Update intrin_dot_sample.cpp

* Update DCT.cpp

* fix deprecation notice (#682)

* initial commit for RenderingToolkit GSG sample proposal

* signed inital commit for RenderingToolkit GSG intro samples

Signed-off-by: Michael R Carroll <[email protected]>

* Added generated GUIDS to .repo-tools/Docs_Automation/guids.json (signed commit)

Signed-off-by: Michael R Carroll <[email protected]>

* Update README.md

Moving README.md content to individual folders

* Order samples folders

Signed-off-by: Michael R Carroll <[email protected]>

* Update README.md

* Adding per sample component README.md files

Signed-off-by: Michael R Carroll <[email protected]>

* Update README.md

* Adding percomponent LICENSE placeholder files (to be reviewed)

Signed-off-by: Michael R Carroll <[email protected]>

* Adding converted .pfm input file. This file could help users if they get stuck. It is not too big.

Signed-off-by: Michael R Carroll <[email protected]>

* Updating sample.json files per Joseph Oster guidance

Signed-off-by: Michael R Carroll <[email protected]>

* Updates to oidn README.md for linux and macos

Signed-off-by: Michael R Carroll <[email protected]>

* Update README for library requirements

Signed-off-by: Michael R Carroll <[email protected]>

* New branch for just oidn sample

Signed-off-by: Michael R Carroll <[email protected]>

* Add description to base README.md

Signed-off-by: Michael R Carroll <[email protected]>

* removing overlapping content. overlap to RenderingToolkit-ospray branch

Signed-off-by: Michael R Carroll <[email protected]>

* Updates for the oidn README.md

Signed-off-by: Michael R Carroll <[email protected]>

* clang-format for source

Signed-off-by: Michael R Carroll <[email protected]>

* README.md update

Signed-off-by: Michael R Carroll <[email protected]>

* Triage samples.json for CI process. Hopefully this unblocks!

Signed-off-by: Michael R Carroll <[email protected]>

* Escape the spaces for CI in sample.json

Signed-off-by: Michael R Carroll <[email protected]>

* Escape extra quotes for CI in sample.json

Signed-off-by: Michael R Carroll <[email protected]>

* Should be no Release folder for sample.json on macos or lin. It has been removed

Signed-off-by: Michael R Carroll <[email protected]>

* Escapes for sample.json input files and exe invoke

Signed-off-by: Michael R Carroll <[email protected]>

* README.md updates for better pathing and simplified build

Signed-off-by: Michael R Carroll <[email protected]>

* CMakeLists convenience updates

Signed-off-by: Michael R Carroll <[email protected]>

* Changing paths for better CI

Signed-off-by: Michael R Carroll <[email protected]>

* Nameing in CMakeLists for global variables build fix

Signed-off-by: Michael R Carroll <[email protected]>

* Update sample.json to take our oneapi env... check paths to input images (OK)

Signed-off-by: Michael R Carroll <[email protected]>

* fix typo/text ommission in sample.json to unblock CI

Signed-off-by: Michael R Carroll <[email protected]>

* Update sample.json

* sample.json to match for macos (env vars)

* Healthier CMakeLists for macos and c++11 on all platforms

Signed-off-by: Michael R Carroll <[email protected]>

* Update samples.json for better Macos env vars... (CI Only)

Co-authored-by: JoeOster <[email protected]>
Co-authored-by: ericlars <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Request to Add a oneAPI Sample: Initial Rendering Toolkit sample
7 participants