Skip to content

Commit

Permalink
style: update our formatting standard to clang-format 17.0 and C++17 (A…
Browse files Browse the repository at this point in the history
…cademySoftwareFoundation#4096)

Bump our clang-format standard from llvm 12 to llvm 17, and our
C++ standard (for formatting) to 17.

Fixes/improvements along the way:

* Moved the clang-format CI test from an aswf container to bare Ubuntu
  to make it run faster -- it doesn't need most of the dependencies,
  so there's no point wasting 2 minutes or so downloading a container.

* build_llvm.bash: Make sure llvm installs when needed, even if not
  using clang.

* gh-installdeps.bash: Setting SKIP_SYSTEM_DEPS_INSTALL=1 skips
  installation of many non-essential dependencies.

There is some reformatting here and there as 12->17 changed some formatting
choices somewhat, but it's fairly minor. Most changes are improvements,
some are neutral, a few are inexplicable but not less readable than before.
We've long since accepted the tradeoff that using clang-format makes some
things formatted differently/worse than we would have preferred, but we deem
it worthy of the benefit of having it all 10% automatic and never fighting
over formatting in PRs.

---------

Signed-off-by: Larry Gritz <lg@larrygritz.com>
Signed-off-by: Peter Kovář <peter.kovar@reflexion.tv>
  • Loading branch information
lgritz authored and 1div0 committed Feb 24, 2024
1 parent a8c385a commit 28ddc59
Show file tree
Hide file tree
Showing 39 changed files with 141 additions and 115 deletions.
2 changes: 1 addition & 1 deletion .clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ SpacesInContainerLiterals: true
SpacesInCStyleCastParentheses: false
SpacesInParentheses: false
SpacesInSquareBrackets: false
Standard: c++14
Standard: c++17
StatementMacros:
- Q_UNUSED
- QT_REQUIRE_VERSION
Expand Down
32 changes: 17 additions & 15 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,21 +173,6 @@ jobs:
USE_OPENCV=0
depcmds: sudo rm -rf /usr/local/include/OpenEXR

# Test formatting. This test entry doesn't do a full build, it
# just runs clang-format on everything, and passes if nothing is
# misformatted. Upon failure, the build artifact will be the full
# source code with the formatting fixed (diffs will also appear in
# the console output).
- desc: "clang-format"
nametag: clang-format
runner: ubuntu-latest
container: aswf/ci-osl:2022-clang12
vfxyear: 2022
cxx_std: 17
skip_tests: 1
setenvs: export BUILDTARGET=clang-format
OIIO_CMAKE_FLAGS=-DUSE_PYTHON=0

# Test ABI stability. `abi_check` is the version or commit that we
# believe is the current standard against which we don't want to
# break the ABI. Basically, we will build that version as well as
Expand Down Expand Up @@ -389,6 +374,22 @@ jobs:
depcmds: |
sudo rm -rf /usr/local/include/OpenEXR
sudo rm -rf /usr/local/lib64/cmake/{IlmBase,OpenEXR}
# Test formatting. This test entry doesn't do a full build, it
# just runs clang-format on everything, and passes if nothing is
# misformatted. Upon failure, the build artifact will be the full
# source code with the formatting fixed (diffs will also appear in
# the console output).
- desc: "clang-format"
nametag: clang-format
runner: ubuntu-latest
cxx_std: 17
extra_artifacts: "src/*.*"
python_ver: "3.10"
skip_tests: 1
setenvs: export BUILDTARGET=clang-format
OIIO_CMAKE_FLAGS=-DUSE_PYTHON=0
LLVM_VERSION=17.0.6 LLVM_DISTRO_NAME=ubuntu-22.04
SKIP_SYSTEM_DEPS_INSTALL=1 QT_VERSION=0

runs-on: ${{ matrix.runner }}
env:
Expand Down Expand Up @@ -439,6 +440,7 @@ jobs:
path: |
build/cmake-save
build/testsuite/*/*.*
${{ matrix.extra_artifacts }}
!build/testsuite/oiio-images
!build/testsuite/openexr-images
!build/testsuite/fits-images
Expand Down
3 changes: 2 additions & 1 deletion src/build-scripts/build_llvm.bash
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,11 @@ if [[ `uname` == "Linux" ]] ; then
ls -l $LLVMTAR
tar xf $LLVMTAR
rm -f $LLVMTAR
echo "Installed ${LLVM_VERSION} in ${LLVM_INSTALL_DIR}"
echo "Installed LLVM ${LLVM_VERSION} in ${LLVM_INSTALL_DIR}"
mkdir -p $LLVM_INSTALL_DIR && true
mv clang+llvm*/* $LLVM_INSTALL_DIR
export LLVM_DIRECTORY=$LLVM_INSTALL_DIR
export PATH=${LLVM_INSTALL_DIR}/bin:$PATH
export LLVM_ROOT=${LLVM_INSTALL_DIR}
# ls -a $LLVM_DIRECTORY
fi
1 change: 1 addition & 0 deletions src/build-scripts/ci-build.bash
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ if [[ "${DEBUG_CI:=0}" != "0" ]] ; then
fi

if [[ "$BUILDTARGET" == clang-format ]] ; then
echo "Running " `which clang-format` " version " `clang-format --version`
git diff --color
THEDIFF=`git diff`
if [[ "$THEDIFF" != "" ]] ; then
Expand Down
23 changes: 13 additions & 10 deletions src/build-scripts/gh-installdeps.bash
Original file line number Diff line number Diff line change
Expand Up @@ -75,15 +75,18 @@ else
git cmake ninja-build ccache g++ \
libboost-dev libboost-thread-dev libboost-filesystem-dev \
libilmbase-dev libopenexr-dev \
libtbb-dev \
libtiff-dev libgif-dev libpng-dev libraw-dev libwebp-dev \
libavcodec-dev libavformat-dev libswscale-dev libavutil-dev \
dcmtk libopenvdb-dev \
libfreetype6-dev \
locales wget \
libopencolorio-dev \
libopencv-dev \
libhdf5-dev
libtiff-dev libgif-dev libpng-dev
if [[ "${SKIP_SYSTEM_DEPS_INSTALL}" != "1" ]] ; then
time sudo apt-get -q install -y \
libraw-dev libwebp-dev \
libavcodec-dev libavformat-dev libswscale-dev libavutil-dev \
dcmtk libopenvdb-dev \
libfreetype6-dev \
locales wget \
libopencolorio-dev \
libtbb-dev \
libopencv-dev
fi
if [[ "${QT_VERSION:-5}" == "5" ]] ; then
time sudo apt-get -q install -y \
qt5-default || /bin/true
Expand Down Expand Up @@ -152,7 +155,7 @@ cmake --version
# If we're using clang to compile on native Ubuntu, we need to install it.
# If on an ASWF CentOS docker container, it already is installed.
#
if [[ "$CXX" == "clang++" && "$ASWF_ORG" == "" ]] ; then
if [[ ("$CXX" == "clang++" && "$ASWF_ORG" == "") || "$LLVM_VERSION" != "" ]] ; then
source src/build-scripts/build_llvm.bash
fi

Expand Down
2 changes: 1 addition & 1 deletion src/ffmpeg.imageio/ffmpeginput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ FFmpegInput::open(const std::string& name, ImageSpec& spec)
}

m_spec = ImageSpec(m_codec_context->width, m_codec_context->height,
nchannels, datatype);
nchannels, datatype);
m_stride = (size_t)(m_spec.scanline_bytes());

m_rgb_buffer.resize(av_image_get_buffer_size(m_dst_pix_format,
Expand Down
6 changes: 3 additions & 3 deletions src/fits.imageio/fitsoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,9 @@ FitsOutput::create_fits_header(void)
using Strutil::stoi;
keyname = "Date";
value = Strutil::sprintf("%04u-%02u-%02uT%02u:%02u:%02u",
stoi(&value[0]), stoi(&value[5]),
stoi(&value[8]), stoi(&value[11]),
stoi(&value[14]), stoi(&value[17]));
stoi(&value[0]), stoi(&value[5]),
stoi(&value[8]), stoi(&value[11]),
stoi(&value[14]), stoi(&value[17]));
}

header += create_card(keyname, value);
Expand Down
8 changes: 4 additions & 4 deletions src/gif.imageio/gifoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,8 @@ GIFOutput::start_subimage(const ImageSpec& spec)
if (m_subimage == 0) {
m_gifwriter.f = ioproxy();
bool ok = GifBegin(&m_gifwriter, m_filename.c_str(), m_spec.width,
m_spec.height, m_delay, 8 /*bit depth*/,
true /*dither*/);
m_spec.height, m_delay, 8 /*bit depth*/,
true /*dither*/);
if (!ok) {
errorfmt("Could not open \"{}\"", m_filename);
return false;
Expand All @@ -216,8 +216,8 @@ GIFOutput::finish_subimage()
return true;

bool ok = GifWriteFrame(&m_gifwriter, &m_canvas[0], spec().width,
spec().height, m_delay, 8 /*bitdepth*/,
true /*dither*/);
spec().height, m_delay, 8 /*bitdepth*/,
true /*dither*/);
m_pending_write = false;
return ok;
}
Expand Down
4 changes: 2 additions & 2 deletions src/include/OpenImageIO/argparse.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class OIIO_UTIL_API ArgParse {
ArgParse(int argc, const char** argv);

// Disallow copy ctr and assignment
ArgParse(const ArgParse&) = delete;
ArgParse(const ArgParse&) = delete;
const ArgParse& operator=(const ArgParse&) = delete;

/// Move constructor
Expand Down Expand Up @@ -426,7 +426,7 @@ class OIIO_UTIL_API ArgParse {
{
}
// Disallow copy ctr and assignment
Arg(const Arg&) = delete;
Arg(const Arg&) = delete;
const Arg& operator=(const Arg&) = delete;

/// Set the help / description of this command line argument.
Expand Down
5 changes: 3 additions & 2 deletions src/include/OpenImageIO/color.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class OIIO_API ColorProcessor {
// in bytes, between subsequent color channels, pixels, and scanlines.
virtual void apply(float* data, int width, int height, int channels,
stride_t chanstride, stride_t xstride,
stride_t ystride) const = 0;
stride_t ystride) const
= 0;
// Convert a single 3-color
void apply(float* data)
{
Expand Down Expand Up @@ -348,7 +349,7 @@ class OIIO_API ColorConfig {
static const ColorConfig& default_colorconfig();

private:
ColorConfig(const ColorConfig&) = delete;
ColorConfig(const ColorConfig&) = delete;
ColorConfig& operator=(const ColorConfig&) = delete;

class Impl;
Expand Down
3 changes: 1 addition & 2 deletions src/include/OpenImageIO/span.h
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,5 @@ constexpr ptrdiff_t ssize(const OIIO::span_strided<T, E>& c) {
namespace fmt {
template<typename T, OIIO::oiio_span_size_type Extent>
struct formatter<OIIO::span<T, Extent>>
: OIIO::pvt::index_formatter<OIIO::span<T, Extent>> {
};
: OIIO::pvt::index_formatter<OIIO::span<T, Extent>> {};
} // namespace fmt
7 changes: 4 additions & 3 deletions src/include/OpenImageIO/ustring.h
Original file line number Diff line number Diff line change
Expand Up @@ -1032,15 +1032,16 @@ inline ustring::ustring(ustringhash hash)


/// ustring string literal operator
inline ustring operator""_us(const char* str, std::size_t len)
inline ustring
operator""_us(const char* str, std::size_t len)
{
return ustring(str, len);
}


/// ustringhash string literal operator
OIIO_DEVICE_CONSTEXPR ustringhash operator""_ush(const char* str,
std::size_t len)
OIIO_DEVICE_CONSTEXPR ustringhash
operator""_ush(const char* str, std::size_t len)
{
return ustringhash(str, len);
}
Expand Down
6 changes: 2 additions & 4 deletions src/include/OpenImageIO/vecparam.h
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,7 @@ template<typename T, typename Base, int Nelem> struct has_subscript_N {

/// C arrays of just the right length also are qualified for has_subscript_N.
template<typename Base, int Nelem>
struct has_subscript_N<Base[Nelem], Base, Nelem> : public std::true_type {
};
struct has_subscript_N<Base[Nelem], Base, Nelem> : public std::true_type {};



Expand Down Expand Up @@ -203,8 +202,7 @@ struct has_double_subscript_RC {
/// C arrays of just the right length also are qualified for has_double_subscript_RC.
template<typename Base, int Rows, int Cols>
struct has_double_subscript_RC<Base[Rows][Cols], Base, Rows, Cols>
: public std::true_type {
};
: public std::true_type {};

/// @}

Expand Down
4 changes: 2 additions & 2 deletions src/iv/ivgl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -598,10 +598,10 @@ IvGL::paintGL()
ybegin = std::max(spec.y, ybegin - (ybegin % m_texture_height));
int xend = (int)floor(real_centerx) + wincenterx;
xend = std::min(spec.x + spec.width,
xend + m_texture_width - (xend % m_texture_width));
xend + m_texture_width - (xend % m_texture_width));
int yend = (int)floor(real_centery) + wincentery;
yend = std::min(spec.y + spec.height,
yend + m_texture_height - (yend % m_texture_height));
yend + m_texture_height - (yend % m_texture_height));
//std::cerr << "(" << xbegin << ',' << ybegin << ") - (" << xend << ',' << yend << ")\n";

// Provide some feedback
Expand Down
2 changes: 1 addition & 1 deletion src/jpeg2000.imageio/jpeg2000input.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ Jpeg2000Input::open(const std::string& name, ImageSpec& p_spec)
// std::cout << "color_space=" << m_image->color_space << "\n";
TypeDesc format = (maxPrecision <= 8) ? TypeDesc::UINT8 : TypeDesc::UINT16;
m_spec = ImageSpec(datawindow.width(), datawindow.height(), channelCount,
format);
format);
m_spec.x = datawindow.xbegin;
m_spec.y = datawindow.ybegin;
m_spec.full_x = m_image->x0;
Expand Down
6 changes: 4 additions & 2 deletions src/libOpenImageIO/compute_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ static ImageBuf imgA, imgB, imgR;



static void test_arrays(ROI)
static void
test_arrays(ROI)
{
const float* a = (const float*)imgA.localpixels();
OIIO_DASSERT(a);
Expand Down Expand Up @@ -79,7 +80,8 @@ test_arrays_like_image(ROI roi)



static void test_arrays_simd4(ROI)
static void
test_arrays_simd4(ROI)
{
const float* a = (const float*)imgA.localpixels();
OIIO_DASSERT(a);
Expand Down
6 changes: 3 additions & 3 deletions src/libOpenImageIO/imagebuf.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1009,7 +1009,7 @@ ImageBufImpl::init_spec(string_view filename, int subimage, int miplevel,
if (m_nsubimages) {
m_badfile = false;
m_pixelaspect = m_spec.get_float_attribute("pixelaspectratio",
1.0f);
1.0f);
m_current_subimage = subimage;
m_current_miplevel = miplevel;
m_spec_valid = true;
Expand Down Expand Up @@ -2130,7 +2130,7 @@ interppixel_(const ImageBuf& img, float x, float y, float* pixel,
int n = img.spec().nchannels;
float* localpixel = OIIO_ALLOCA(float, n * 4);
float* p[4] = { localpixel, localpixel + n, localpixel + 2 * n,
localpixel + 3 * n };
localpixel + 3 * n };
x -= 0.5f;
y -= 0.5f;
int xtexel, ytexel;
Expand Down Expand Up @@ -2949,7 +2949,7 @@ ImageBufImpl::retile(int x, int y, int z, ImageCache::Tile*& tile,
tilezbegin = m_spec.z + ztile * td;
tilexend = tilexbegin + tw;
tile = m_imagecache->get_tile(m_name, m_current_subimage,
m_current_miplevel, x, y, z);
m_current_miplevel, x, y, z);
if (!tile) {
// Even though tile is NULL, ensure valid black pixel data
std::string e = m_imagecache->geterror();
Expand Down
4 changes: 2 additions & 2 deletions src/libOpenImageIO/imagebufalgo_draw.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -906,10 +906,10 @@ text_size_from_unicode(cspan<uint32_t> utext, FT_Face face, int fontsize)
continue; // ignore errors
size.ybegin = std::min(size.ybegin, y - slot->bitmap_top);
size.yend = std::max(size.yend, y + int(slot->bitmap.rows)
- int(slot->bitmap_top) + 1);
- int(slot->bitmap_top) + 1);
size.xbegin = std::min(size.xbegin, x + int(slot->bitmap_left));
size.xend = std::max(size.xend, x + int(slot->bitmap.width)
+ int(slot->bitmap_left) + 1);
+ int(slot->bitmap_left) + 1);
// increment pen position
x += slot->advance.x >> 6;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libOpenImageIO/imageioplugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ catalog_plugin(const std::string& format_name,

std::string version_function = format_name + "_imageio_version";
int* plugin_version = (int*)Plugin::getsym(handle,
version_function.c_str());
version_function.c_str());
if (!plugin_version || *plugin_version != OIIO_PLUGIN_VERSION) {
Plugin::close(handle);
return;
Expand Down
4 changes: 2 additions & 2 deletions src/libOpenImageIO/imageoutput.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@ ImageOutput::write_image(TypeDesc format, const void* data, stride_t xstride,
m_spec.z + m_spec.depth);
for (int y = 0; y < m_spec.height; y += m_spec.tile_height) {
int yend = std::min(y + m_spec.y + m_spec.tile_height,
m_spec.y + m_spec.height);
m_spec.y + m_spec.height);
const char* d = (const char*)data + z * zstride + y * ystride;
ok &= write_tiles(m_spec.x, m_spec.x + m_spec.width,
y + m_spec.y, yend, z + m_spec.z, zend,
Expand All @@ -521,7 +521,7 @@ ImageOutput::write_image(TypeDesc format, const void* data, stride_t xstride,
for (int z = 0; z < m_spec.depth; ++z)
for (int y = 0; y < m_spec.height && ok; y += chunk) {
int yend = std::min(y + m_spec.y + chunk,
m_spec.y + m_spec.height);
m_spec.y + m_spec.height);
const char* d = (const char*)data + z * zstride + y * ystride;
ok &= write_scanlines(y + m_spec.y, yend, z + m_spec.z, format,
d, xstride, ystride);
Expand Down
4 changes: 2 additions & 2 deletions src/libtexture/environment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ TextureSystemImpl::environment(TextureHandle* texture_handle_,
float invsamples;
if (aniso) {
aspect = anisotropic_aspect(majorlength, minorlength, options,
trueaspect);
trueaspect);
filtwidth = minorlength;
if (trueaspect > stats.max_aniso)
stats.max_aniso = trueaspect;
Expand Down Expand Up @@ -501,7 +501,7 @@ TextureSystemImpl::environment(TextureHandle* texture_handle_,
miplevel[0] = m - 1;
miplevel[1] = m;
levelblend = OIIO::clamp(2.0f * filtwidth_ras - 1.0f, 0.0f,
1.0f);
1.0f);
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/libtexture/imagecache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ ImageCacheFile::open(ImageCachePerThreadInfo* thread_info)
}
tempspec.tile_height = std::min(tempspec.height, autotile);
tempspec.tile_depth = std::min(std::max(tempspec.depth, 1),
autotile);
autotile);
} else {
// Don't auto-tile -- which really means, make it look like
// a single tile that's as big as the whole image.
Expand Down
Loading

0 comments on commit 28ddc59

Please sign in to comment.