Skip to content

Conversation

@soswow
Copy link
Contributor

@soswow soswow commented Apr 15, 2025

No description provided.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 15, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: soswow / name: Aleksandr Motsjonov (44a8466)

@soswow soswow force-pushed the update-readme-dep-instructions branch 2 times, most recently from 449a5b1 to ced5509 Compare April 15, 2025 10:30
```

Install libraw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing repeating instruction to declutter. Kept the first one. IF people following to top to bottom, they would installed homebrew already, or they can always google

cmake ..
make
sudo make install
```
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Source repo would always have more up to date instruction.

README.md Outdated

```sh
/usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
$ /usr/bin/ruby -e "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/master/install)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

consistency

README.md Outdated

## Prerequisites

⚠️ Some of the instructions below might be outdated and require overhall. The simplest way to install all the dependencies is to look into intsall script in [`build_scripts/`](./build_scripts) for your OS.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not say that and just provide the up to date instructions. I think we are pretty close.

README.md Outdated
```sh
brew install ceres-solver imath openexr libraw boost # From build_scripts
brew install cmake
brew install aces_container # Either that or follow build_scripts/install_aces_container.bash to build from source
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not give the installing aces_container from brew as an example, as it is not available on other platforms. I'd just mention the shell script.

@antond-weta
Copy link
Contributor

Looks good.
I would simplify the dependencies section even further by just mentioning the list of dependencies with the minimum required version, and then providing a one liner fetching the dependencies for each packaging system.

README.md Outdated
| ------- | -----------| -------- | -------------------------------- |
| `cmake` | `3.10` | | [CMake download](https://cmake.org/download/)|
| `ceres` | `1.14.0` | Ceres Solver is an open source library for solving Non-linear Least Squares problems with bounds constraints and unconstrained optimization problems. It processes non-linear regression for rawtoaces. | [Ceres Solver installation](http://ceres-solver.org/installation.html)|
| `imath` | `3.1.8` | Provides the half data type used for representing 16-bit floating-point values. It's used by rawtoaces for storing high dynamic range (HDR) data in a compact format. | [imath installation](https://imath.readthedocs.io/en/latest/install.html#install)|
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@antond-weta please check Purpose column for Imath

README.md Outdated
```

In case of any GFlag errors AFTER running the next step (rawtoaces installation) you should repeat the install of ceres and follow the instructions shown in terminal to switch the version.
Follow [ACES Container installation ](https://github.com/ampas/aces_container?tab=readme-ov-file#installation) instruction.
Copy link
Contributor

Choose a reason for hiding this comment

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

build_scripts/install_aces_container.bash works on windows as well

@soswow soswow force-pushed the update-readme-dep-instructions branch from f9f45fd to f0fc1a8 Compare April 16, 2025 06:55
@soswow soswow force-pushed the update-readme-dep-instructions branch from f0fc1a8 to 44a8466 Compare April 16, 2025 06:59
@antond-weta antond-weta merged commit e258943 into AcademySoftwareFoundation:master Apr 16, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants